Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 44 additions & 1 deletion Lib/test/test_mmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def test_basic(self):

# Shouldn't crash on boundary (Issue #5292)
self.assertRaises(IndexError, m.__getitem__, len(m))
self.assertRaises(IndexError, m.__setitem__, len(m), b'\0')
self.assertRaises(IndexError, m.__setitem__, len(m), 0)

# Modify the file's content
m[0] = b'3'[0]
Expand Down Expand Up @@ -953,6 +953,49 @@ def test_resize_down_anonymous_mapping(self):
with self.assertRaises(ValueError):
m.resize(start_size)

@unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize')
def test_setitem_resize_reentrancy(self):
Comment thread
vstinner marked this conversation as resolved.
"""Resizing the mmap from inside __index__ while assigning to a
single item must not access memory past the new bounds (gh-157335).
"""
size = 2 * PAGESIZE
new_size = PAGESIZE

class ResizeOnIndex:
def __init__(self, m):
self.m = m
def __index__(self):
self.m.resize(new_size)
return 0

with mmap.mmap(-1, size) as m:
with self.assertRaises(IndexError):
m[size - 1] = ResizeOnIndex(m)
self.assertEqual(len(m), new_size)

@unittest.skipUnless(hasattr(mmap.mmap, 'resize'), 'requires mmap.resize')
def test_setitem_slice_resize_reentrancy(self):
"""Resizing the mmap from inside a value's buffer-protocol
callback while assigning to a slice must not access memory past
the new bounds (gh-157335).
"""
size = 2 * PAGESIZE
new_size = PAGESIZE

class ResizeOnBuffer:
def __init__(self, m, data):
self.m = m
self.data = data
def __buffer__(self, flags):
self.m.resize(new_size)
return memoryview(self.data)

with mmap.mmap(-1, size) as m:
value = ResizeOnBuffer(m, bytes(size))
with self.assertRaises(IndexError):
m[0:size] = value
self.assertEqual(len(m), new_size)

@unittest.skipUnless(os.name == 'nt', 'requires Windows')
def test_resize_fails_if_mapping_held_elsewhere(self):
"""If more than one mapping is held against a named file on Windows, neither
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fix out-of-bounds write in ``mmap.mmap.__setitem__`` that could occur
when converting the index or the assigned value (via :meth:`~object.__index__`
for a single item, or via the buffer protocol for a slice) resized or closed the mmap
object during the assignment.
32 changes: 19 additions & 13 deletions Modules/mmapmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1649,24 +1649,15 @@ static int
mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
{
mmap_object *self = mmap_object_CAST(op);
CHECK_VALID(-1);

if (!is_writable(self))
return -1;

if (PyIndex_Check(item)) {
Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError);
Py_ssize_t v;

if (i == -1 && PyErr_Occurred())
return -1;
if (i < 0)
i += self->size;
if (i < 0 || i >= self->size) {
PyErr_SetString(PyExc_IndexError,
"mmap index out of range");
return -1;
}

if (value == NULL) {
PyErr_SetString(PyExc_TypeError,
"mmap doesn't support item deletion");
Expand All @@ -1677,7 +1668,7 @@ mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
"mmap item value must be an int");
return -1;
}
v = PyNumber_AsSsize_t(value, PyExc_TypeError);
Py_ssize_t v = PyNumber_AsSsize_t(value, PyExc_TypeError);
if (v == -1 && PyErr_Occurred())
return -1;
if (v < 0 || v > 255) {
Expand All @@ -1686,7 +1677,18 @@ mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
"in range(0, 256)");
return -1;
}

/* Converting item or value above may have run arbitrary code
* (e.g. __index__) that resized or closed the mmap, so bounds
* are only checked now, against the current size. */
CHECK_VALID(-1);
if (i < 0)
i += self->size;
if (i < 0 || i >= self->size) {
PyErr_SetString(PyExc_IndexError,
"mmap index out of range");
return -1;
}

char v_char = (char) v;
if (safe_byte_copy(self->data + i, &v_char) < 0) {
Expand All @@ -1701,22 +1703,26 @@ mmap_ass_subscript_lock_held(PyObject *op, PyObject *item, PyObject *value)
if (PySlice_Unpack(item, &start, &stop, &step) < 0) {
return -1;
}
slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);
if (value == NULL) {
PyErr_SetString(PyExc_TypeError,
"mmap object doesn't support slice deletion");
return -1;
}
if (PyObject_GetBuffer(value, &vbuf, PyBUF_SIMPLE) < 0)
return -1;

/* Acquiring the buffer above may have run arbitrary code (e.g. a
* __buffer__ method) that resized or closed this mmap, so the slice bounds
* are only computed now, against the current size. */
CHECK_VALID_OR_RELEASE(-1, vbuf);
slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);
if (vbuf.len != slicelen) {
PyErr_SetString(PyExc_IndexError,
"mmap slice assignment is wrong size");
PyBuffer_Release(&vbuf);
return -1;
}

CHECK_VALID_OR_RELEASE(-1, vbuf);
int result = 0;
if (slicelen == 0) {
}
Expand Down
Loading