Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-92888: memory_ass_sub() calls CHECK_RELEASED_INT() twice #93127

Closed
wants to merge 1 commit into from
Closed

gh-92888: memory_ass_sub() calls CHECK_RELEASED_INT() twice #93127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions Lib/test/test_memoryview.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,51 @@ def test_issue22668(self):
self.assertEqual(c.format, "H")
self.assertEqual(d.format, "H")

def test_index_release(self):
# gh-92888: If a memory view is released while setting an item, an
# exception must be raised before writing into memory.

def create_index(view):
class IndexRelease:
def __index__(self):
view.release()
return 0
return IndexRelease()

def set_item(view, index):
view[index] = 0

def delete_item(view, index):
del view[index]

def set_first_items(view, index):
view[:index] = [0]

def set_last_items(view, index):
view[index:] = [0]

if not self.rw_type:
self.skipTest("no writable type to test")
tp = self.rw_type
b = self.rw_type(self._source)
view = self._view(b)

regex = 'operation forbidden on released memoryview object'

for setter in (
set_item,
delete_item,
set_first_items,
set_last_items,
):
with self.subTest(setter=setter):
index = create_index(view)
with self.assertRaisesRegex(ValueError, regex):
setter(view, index)

# Check that the read-write object was not modified
self.assertEqual(b, self.rw_type(self._source))


# Variations on source objects for the buffer: bytes-like objects, then arrays
# with itemsize > 1.
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2022-05-23-17-53-54.gh-issue-92888.aLR9yl.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
If a memoryview is released while setting an item, raise an exception. For
example, converting an object to an index in C can execute arbitrary Python
code which can indirectly release the memoryview. Patch by Victor Stinner.
192 changes: 130 additions & 62 deletions Objects/memoryobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,17 +179,19 @@ PyTypeObject _PyManagedBuffer_Type = {
(((PyMemoryViewObject *)mv)->flags&_Py_MEMORYVIEW_RELEASED || \
((PyMemoryViewObject *)mv)->mbuf->flags&_Py_MANAGED_BUFFER_RELEASED)

#define RELEASED_ERROR() \
PyErr_SetString(PyExc_ValueError, \
"operation forbidden on released memoryview object")

#define CHECK_RELEASED(mv) \
if (BASE_INACCESSIBLE(mv)) { \
PyErr_SetString(PyExc_ValueError, \
"operation forbidden on released memoryview object"); \
RELEASED_ERROR(); \
return NULL; \
}

#define CHECK_RELEASED_INT(mv) \
if (BASE_INACCESSIBLE(mv)) { \
PyErr_SetString(PyExc_ValueError, \
"operation forbidden on released memoryview object"); \
RELEASED_ERROR(); \
return -1; \
}

Expand Down Expand Up @@ -1767,23 +1769,23 @@ unpack_single(const char *ptr, const char *fmt)
/* Pack a single item. 'fmt' can be any native format character in
struct module syntax. */
static int
pack_single(char *ptr, PyObject *item, const char *fmt)
pack_single(PyMemoryViewObject *self, char *ptr, PyObject *item,
const char *fmt)
{
unsigned long long llu;
unsigned long lu;
size_t zu;
long long lld;
long ld;
Py_ssize_t zd;
double d;
void *p;
// gh-92888: CHECK_RELEASED_INT() is called before the final write and
// after function calls, like _PyNumber_Index(), which can execute
// arbitrary Python code which can release the memoryview indirectly.

switch (fmt[0]) {
/* signed integers */
case 'b': case 'h': case 'i': case 'l':
ld = pylong_as_ld(item);
if (ld == -1 && PyErr_Occurred())
case 'b': case 'h': case 'i': case 'l': {
long ld = pylong_as_ld(item);
if (ld == -1 && PyErr_Occurred()) {
goto err_occurred;
}

CHECK_RELEASED_INT(self);

switch (fmt[0]) {
case 'b':
if (ld < SCHAR_MIN || ld > SCHAR_MAX) goto err_range;
Expand All @@ -1798,12 +1800,17 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
PACK_SINGLE(ptr, ld, long); break;
}
break;
}

/* unsigned integers */
case 'B': case 'H': case 'I': case 'L':
lu = pylong_as_lu(item);
if (lu == (unsigned long)-1 && PyErr_Occurred())
case 'B': case 'H': case 'I': case 'L': {
unsigned long lu = pylong_as_lu(item);
if (lu == (unsigned long)-1 && PyErr_Occurred()) {
goto err_occurred;
}

CHECK_RELEASED_INT(self);

switch (fmt[0]) {
case 'B':
if (lu > UCHAR_MAX) goto err_range;
Expand All @@ -1818,72 +1825,118 @@ pack_single(char *ptr, PyObject *item, const char *fmt)
PACK_SINGLE(ptr, lu, unsigned long); break;
}
break;
}

/* native 64-bit */
case 'q':
lld = pylong_as_lld(item);
if (lld == -1 && PyErr_Occurred())
case 'q': {
long long lld = pylong_as_lld(item);
if (lld == -1 && PyErr_Occurred()) {
goto err_occurred;
}

CHECK_RELEASED_INT(self);

PACK_SINGLE(ptr, lld, long long);
break;
case 'Q':
llu = pylong_as_llu(item);
if (llu == (unsigned long long)-1 && PyErr_Occurred())
}

case 'Q': {
unsigned long long llu = pylong_as_llu(item);
if (llu == (unsigned long long)-1 && PyErr_Occurred()) {
goto err_occurred;
}

CHECK_RELEASED_INT(self);

PACK_SINGLE(ptr, llu, unsigned long long);
break;
}

/* ssize_t and size_t */
case 'n':
zd = pylong_as_zd(item);
if (zd == -1 && PyErr_Occurred())
case 'n': {
Py_ssize_t zd = pylong_as_zd(item);
if (zd == -1 && PyErr_Occurred()) {
goto err_occurred;
}

CHECK_RELEASED_INT(self);

PACK_SINGLE(ptr, zd, Py_ssize_t);
break;
case 'N':
zu = pylong_as_zu(item);
if (zu == (size_t)-1 && PyErr_Occurred())
}

case 'N': {
size_t zu = pylong_as_zu(item);
if (zu == (size_t)-1 && PyErr_Occurred()) {
goto err_occurred;
}

CHECK_RELEASED_INT(self);

PACK_SINGLE(ptr, zu, size_t);
break;
}

/* floats */
case 'f': case 'd':
d = PyFloat_AsDouble(item);
if (d == -1.0 && PyErr_Occurred())
case 'f': case 'd': {
double d = PyFloat_AsDouble(item);
if (d == -1.0 && PyErr_Occurred()) {
goto err_occurred;
}

CHECK_RELEASED_INT(self);

if (fmt[0] == 'f') {
PACK_SINGLE(ptr, d, float);
}
else {
PACK_SINGLE(ptr, d, double);
}
break;
}

/* bool */
case '?':
ld = PyObject_IsTrue(item);
if (ld < 0)
case '?': {
int res = PyObject_IsTrue(item);
if (res < 0) {
return -1; /* preserve original error */
PACK_SINGLE(ptr, ld, _Bool);
break;
}

CHECK_RELEASED_INT(self);

PACK_SINGLE(ptr, res, _Bool);
break;
}

/* bytes object */
case 'c':
if (!PyBytes_Check(item))
case 'c': {
if (!PyBytes_Check(item)) {
return type_error_int(fmt);
if (PyBytes_GET_SIZE(item) != 1)
}
if (PyBytes_GET_SIZE(item) != 1) {
return value_error_int(fmt);
*ptr = PyBytes_AS_STRING(item)[0];
}
char ch = PyBytes_AS_STRING(item)[0];

// No need to check CHECK_RELEASED_INT(): PyBytes_GET_SIZE() and
// PyBytes_AS_STRING() cannot execute arbitrary Python.

*ptr = ch;
break;
}

/* pointer */
case 'P':
p = PyLong_AsVoidPtr(item);
if (p == NULL && PyErr_Occurred())
case 'P': {
void *p = PyLong_AsVoidPtr(item);
if (p == NULL && PyErr_Occurred()) {
goto err_occurred;
}

CHECK_RELEASED_INT(self);

PACK_SINGLE(ptr, p, void *);
break;
}

/* default */
default: goto err_format;
Expand Down Expand Up @@ -2516,15 +2569,13 @@ static int
memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
{
Py_buffer *view = &(self->view);
Py_buffer src;
const char *fmt;
char *ptr;

CHECK_RELEASED_INT(self);

fmt = adjust_fmt(view);
if (fmt == NULL)
const char *fmt = adjust_fmt(view);
if (fmt == NULL) {
return -1;
}

if (view->readonly) {
PyErr_SetString(PyExc_TypeError, "cannot modify read-only memory");
Expand All @@ -2534,11 +2585,12 @@ memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
PyErr_SetString(PyExc_TypeError, "cannot delete memory");
return -1;
}

if (view->ndim == 0) {
if (key == Py_Ellipsis ||
(PyTuple_Check(key) && PyTuple_GET_SIZE(key)==0)) {
ptr = (char *)view->buf;
return pack_single(ptr, value, fmt);
char *ptr = (char *)view->buf;
return pack_single(self, ptr, value, fmt);
}
else {
PyErr_SetString(PyExc_TypeError,
Expand All @@ -2557,20 +2609,24 @@ memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
index = PyNumber_AsSsize_t(key, PyExc_IndexError);
if (index == -1 && PyErr_Occurred())
return -1;
ptr = ptr_from_index(view, index);
if (ptr == NULL)
char *ptr = ptr_from_index(view, index);
if (ptr == NULL) {
return -1;
return pack_single(ptr, value, fmt);
}
return pack_single(self, ptr, value, fmt);
}

/* one-dimensional: fast path */
if (PySlice_Check(key) && view->ndim == 1) {
Py_buffer dest; /* sliced view */
Py_ssize_t arrays[3];
int ret = -1;

/* rvalue must be an exporter */
if (PyObject_GetBuffer(value, &src, PyBUF_FULL_RO) < 0)
Py_buffer src;
if (PyObject_GetBuffer(value, &src, PyBUF_FULL_RO) < 0) {
return ret;
}

dest = *view;
dest.shape = &arrays[0]; dest.shape[0] = view->shape[0];
Expand All @@ -2579,28 +2635,40 @@ memory_ass_sub(PyMemoryViewObject *self, PyObject *key, PyObject *value)
dest.suboffsets = &arrays[2]; dest.suboffsets[0] = view->suboffsets[0];
}

if (init_slice(&dest, key, 0) < 0)
if (init_slice(&dest, key, 0) < 0) {
goto end_block;
}
dest.len = dest.shape[0] * dest.itemsize;

// gh-92888: PyObject_GetBuffer() and PySlice_Unpack() can execute
// arbitrary Python code which can release the memoryview indirectly
if (BASE_INACCESSIBLE(self)) {
RELEASED_ERROR();
goto end_block;
}

ret = copy_single(&dest, &src);

end_block:
PyBuffer_Release(&src);
return ret;
}

if (is_multiindex(key)) {
char *ptr;
if (PyTuple_GET_SIZE(key) < view->ndim) {
PyErr_SetString(PyExc_NotImplementedError,
"sub-views are not implemented");
return -1;
}
ptr = ptr_from_tuple(view, key);
if (ptr == NULL)

char *ptr = ptr_from_tuple(view, key);
if (ptr == NULL) {
return -1;
return pack_single(ptr, value, fmt);
}

return pack_single(self, ptr, value, fmt);
}

if (PySlice_Check(key) || is_multislice(key)) {
/* Call memory_subscript() to produce a sliced lvalue, then copy
rvalue into lvalue. This is already implemented in _testbuffer.c. */
Expand Down