Skip to content

Commit

Permalink
pythongh-103987: fix several crashes in mmap module (python#103990)
Browse files Browse the repository at this point in the history
Co-authored-by: sunmy2019 <59365878+sunmy2019@users.noreply.github.com>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
  • Loading branch information
3 people authored May 20, 2023
1 parent 3bc94e6 commit ceaa4c3
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 3 deletions.
87 changes: 86 additions & 1 deletion Lib/test/test_mmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,6 @@ def test_move(self):
m.move(0, 0, 1)
m.move(0, 0, 0)


def test_anonymous(self):
# anonymous mmap.mmap(-1, PAGE)
m = mmap.mmap(-1, PAGESIZE)
Expand Down Expand Up @@ -887,6 +886,92 @@ def test_resize_succeeds_with_error_for_second_named_mapping(self):
self.assertEqual(m1[:data_length], data)
self.assertEqual(m2[:data_length], data)

def test_mmap_closed_by_int_scenarios(self):
"""
gh-103987: Test that mmap objects raise ValueError
for closed mmap files
"""

class MmapClosedByIntContext:
def __init__(self, access) -> None:
self.access = access

def __enter__(self):
self.f = open(TESTFN, "w+b")
self.f.write(random.randbytes(100))
self.f.flush()

m = mmap.mmap(self.f.fileno(), 100, access=self.access)

class X:
def __index__(self):
m.close()
return 10

return (m, X)

def __exit__(self, exc_type, exc_value, traceback):
self.f.close()

read_access_modes = [
mmap.ACCESS_READ,
mmap.ACCESS_WRITE,
mmap.ACCESS_COPY,
mmap.ACCESS_DEFAULT,
]

write_access_modes = [
mmap.ACCESS_WRITE,
mmap.ACCESS_COPY,
mmap.ACCESS_DEFAULT,
]

for access in read_access_modes:
with MmapClosedByIntContext(access) as (m, X):
with self.assertRaisesRegex(ValueError, "mmap closed or invalid"):
m[X()]

with MmapClosedByIntContext(access) as (m, X):
with self.assertRaisesRegex(ValueError, "mmap closed or invalid"):
m[X() : 20]

with MmapClosedByIntContext(access) as (m, X):
with self.assertRaisesRegex(ValueError, "mmap closed or invalid"):
m[X() : 20 : 2]

with MmapClosedByIntContext(access) as (m, X):
with self.assertRaisesRegex(ValueError, "mmap closed or invalid"):
m[20 : X() : -2]

with MmapClosedByIntContext(access) as (m, X):
with self.assertRaisesRegex(ValueError, "mmap closed or invalid"):
m.read(X())

with MmapClosedByIntContext(access) as (m, X):
with self.assertRaisesRegex(ValueError, "mmap closed or invalid"):
m.find(b"1", 1, X())

for access in write_access_modes:
with MmapClosedByIntContext(access) as (m, X):
with self.assertRaisesRegex(ValueError, "mmap closed or invalid"):
m[X() : 20] = b"1" * 10

with MmapClosedByIntContext(access) as (m, X):
with self.assertRaisesRegex(ValueError, "mmap closed or invalid"):
m[X() : 20 : 2] = b"1" * 5

with MmapClosedByIntContext(access) as (m, X):
with self.assertRaisesRegex(ValueError, "mmap closed or invalid"):
m[20 : X() : -2] = b"1" * 5

with MmapClosedByIntContext(access) as (m, X):
with self.assertRaisesRegex(ValueError, "mmap closed or invalid"):
m.move(1, 2, X())

with MmapClosedByIntContext(access) as (m, X):
with self.assertRaisesRegex(ValueError, "mmap closed or invalid"):
m.write_byte(X())

class LargeMmapTests(unittest.TestCase):

def setUp(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
In :mod:`mmap`, fix several bugs that could lead to access to memory-mapped files after
they have been invalidated.
15 changes: 13 additions & 2 deletions Modules/mmapmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,8 @@ mmap_read_method(mmap_object *self,

CHECK_VALID(NULL);
if (!PyArg_ParseTuple(args, "|O&:read", _Py_convert_optional_to_ssize_t, &num_bytes))
return(NULL);
return NULL;
CHECK_VALID(NULL);

/* silently 'adjust' out-of-range requests */
remaining = (self->pos < self->size) ? self->size - self->pos : 0;
Expand Down Expand Up @@ -325,6 +326,7 @@ mmap_gfind(mmap_object *self,
end = self->size;

Py_ssize_t res;
CHECK_VALID(NULL);
if (reverse) {
res = _PyBytes_ReverseFind(
self->data + start, end - start,
Expand Down Expand Up @@ -388,7 +390,7 @@ mmap_write_method(mmap_object *self,

CHECK_VALID(NULL);
if (!PyArg_ParseTuple(args, "y*:write", &data))
return(NULL);
return NULL;

if (!is_writable(self)) {
PyBuffer_Release(&data);
Expand All @@ -401,6 +403,7 @@ mmap_write_method(mmap_object *self,
return NULL;
}

CHECK_VALID(NULL);
memcpy(&self->data[self->pos], data.buf, data.len);
self->pos += data.len;
PyBuffer_Release(&data);
Expand All @@ -420,6 +423,7 @@ mmap_write_byte_method(mmap_object *self,
if (!is_writable(self))
return NULL;

CHECK_VALID(NULL);
if (self->pos < self->size) {
self->data[self->pos++] = value;
Py_RETURN_NONE;
Expand Down Expand Up @@ -724,6 +728,7 @@ mmap_move_method(mmap_object *self, PyObject *args)
if (self->size - dest < cnt || self->size - src < cnt)
goto bounds;

CHECK_VALID(NULL);
memmove(&self->data[dest], &self->data[src], cnt);

Py_RETURN_NONE;
Expand Down Expand Up @@ -847,6 +852,7 @@ mmap_madvise_method(mmap_object *self, PyObject *args)
length = self->size - start;
}

CHECK_VALID(NULL);
if (madvise(self->data + start, length, option) != 0) {
PyErr_SetFromErrno(PyExc_OSError);
return NULL;
Expand Down Expand Up @@ -945,6 +951,7 @@ mmap_subscript(mmap_object *self, PyObject *item)
"mmap index out of range");
return NULL;
}
CHECK_VALID(NULL);
return PyLong_FromLong(Py_CHARMASK(self->data[i]));
}
else if (PySlice_Check(item)) {
Expand All @@ -955,6 +962,7 @@ mmap_subscript(mmap_object *self, PyObject *item)
}
slicelen = PySlice_AdjustIndices(self->size, &start, &stop, step);

CHECK_VALID(NULL);
if (slicelen <= 0)
return PyBytes_FromStringAndSize("", 0);
else if (step == 1)
Expand All @@ -968,6 +976,7 @@ mmap_subscript(mmap_object *self, PyObject *item)

if (result_buf == NULL)
return PyErr_NoMemory();

for (cur = start, i = 0; i < slicelen;
cur += step, i++) {
result_buf[i] = self->data[cur];
Expand Down Expand Up @@ -1052,6 +1061,7 @@ mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value)
"in range(0, 256)");
return -1;
}
CHECK_VALID(-1);
self->data[i] = (char) v;
return 0;
}
Expand All @@ -1077,6 +1087,7 @@ mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value)
return -1;
}

CHECK_VALID(-1);
if (slicelen == 0) {
}
else if (step == 1) {
Expand Down

0 comments on commit ceaa4c3

Please sign in to comment.