From ceaa4c3476ac49b5b31954fec53796c7a3b40349 Mon Sep 17 00:00:00 2001 From: Prince Roshan Date: Sat, 20 May 2023 09:04:12 +0530 Subject: [PATCH] gh-103987: fix several crashes in mmap module (#103990) Co-authored-by: sunmy2019 <59365878+sunmy2019@users.noreply.github.com> Co-authored-by: Jelle Zijlstra --- Lib/test/test_mmap.py | 87 ++++++++++++++++++- ...-04-29-18-23-16.gh-issue-103987.sRgALL.rst | 2 + Modules/mmapmodule.c | 15 +++- 3 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-04-29-18-23-16.gh-issue-103987.sRgALL.rst diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py index 213a44d56f37b3..517cbe0cb115ab 100644 --- a/Lib/test/test_mmap.py +++ b/Lib/test/test_mmap.py @@ -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) @@ -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): diff --git a/Misc/NEWS.d/next/Library/2023-04-29-18-23-16.gh-issue-103987.sRgALL.rst b/Misc/NEWS.d/next/Library/2023-04-29-18-23-16.gh-issue-103987.sRgALL.rst new file mode 100644 index 00000000000000..48c6d149a8ed42 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-04-29-18-23-16.gh-issue-103987.sRgALL.rst @@ -0,0 +1,2 @@ +In :mod:`mmap`, fix several bugs that could lead to access to memory-mapped files after +they have been invalidated. diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index a470dd3c2f3bba..8e5a0bde889901 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -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; @@ -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, @@ -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); @@ -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); @@ -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; @@ -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; @@ -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; @@ -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)) { @@ -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) @@ -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]; @@ -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; } @@ -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) {