[Python-checkins] gh-103987: fix several crashes in mmap module (#103990)
JelleZijlstra
webhook-mailer at python.org
Fri May 19 23:34:19 EDT 2023
https://github.com/python/cpython/commit/ceaa4c3476ac49b5b31954fec53796c7a3b40349
commit: ceaa4c3476ac49b5b31954fec53796c7a3b40349
branch: main
author: Prince Roshan <princekrroshan01 at gmail.com>
committer: JelleZijlstra <jelle.zijlstra at gmail.com>
date: 2023-05-19T20:34:12-07:00
summary:
gh-103987: fix several crashes in mmap module (#103990)
Co-authored-by: sunmy2019 <59365878+sunmy2019 at users.noreply.github.com>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra at gmail.com>
files:
A Misc/NEWS.d/next/Library/2023-04-29-18-23-16.gh-issue-103987.sRgALL.rst
M Lib/test/test_mmap.py
M Modules/mmapmodule.c
diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py
index 213a44d56f37..517cbe0cb115 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 000000000000..48c6d149a8ed
--- /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 a470dd3c2f3b..8e5a0bde8899 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) {
More information about the Python-checkins
mailing list