pythongh-103987: fix several crashes in mmap module (python#103990)

Co-authored-by: sunmy2019 <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>

Author: 3 people

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):

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.

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) {