From edd566cbd6b2ce10d397051a0fc6e021eb10d0f8 Mon Sep 17 00:00:00 2001 From: superboy-zjc <1826599908@qq.com> Date: Tue, 30 Dec 2025 22:43:26 -0800 Subject: [PATCH 1/2] gh-142663: Fix use-after-free in memoryview comparison When comparing two memoryview objects with different formats, `memory_richcompare` uses the `struct` module to unpack elements. A custom `struct.Struct.unpack_from` implementation could releases and resizes underlying buffer, which invalidates the buffer pointer, during iteration. This leads to a use-after-free when the comparison loop continued accessing the freed memory. The fix increments the `exports` count of the memoryview objects before performing the comparison, effectively locking the buffers. This mirrors the protection already provided for non-memoryview objects via `PyObject_GetBuffer`. --- Lib/test/test_memoryview.py | 61 +++++++++++++++++++ ...-12-30-22-12-27.gh-issue-142663.gq7iIf.rst | 3 + Objects/memoryobject.c | 12 ++++ 3 files changed, 76 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-12-30-22-12-27.gh-issue-142663.gq7iIf.rst diff --git a/Lib/test/test_memoryview.py b/Lib/test/test_memoryview.py index 656318668e6d6e..6b5f12554ce181 100644 --- a/Lib/test/test_memoryview.py +++ b/Lib/test/test_memoryview.py @@ -228,6 +228,67 @@ def test_compare(self): self.assertRaises(TypeError, lambda: m >= c) self.assertRaises(TypeError, lambda: c > m) + def test_compare_use_after_free(self): + # Prevent crash in comparisons of memoryview objs with re-entrant struct.unpack_from. + # Regression test for https://github.com/python/cpython/issues/142663. + + class ST(struct.Struct): + # Context set by the subtests + view = None + source = None + + def unpack_from(self, buf, /, offset=0): + # Attempt to release the buffer while it's being used in comparison loop. + if self.view is not None: + self.view.release() + + # array resize invalidates the buffer pointer used by the comparison loop. + if self.source is not None: + self.source.append(3.14) + + return (1,) + + with support.swap_attr(struct, "Struct", ST): + # Case 1: 1-D comparison (uses cmp_base optimized loop) + # Use mixed types ('d' vs 'l') to force struct unpacking path. + with self.subTest(ndim=1): + a = array.array("d", [1.0, 2.0]) + b = array.array("l", [1, 2]) + mv_a = memoryview(a) + mv_b = memoryview(b) + + ST.view = mv_a + ST.source = a + try: + with self.assertRaises(BufferError): + # Expect BufferError because the memoryview is locked during comparison + mv_a == mv_b + finally: + ST.view = None + ST.source = None + mv_a.release() + mv_b.release() + + # Case 2: N-D comparison (uses cmp_rec recursive function) + # Use mixed types ('d' vs 'l') to force struct unpacking path. + with self.subTest(ndim=2): + a = array.array("d", [1.0, 2.0]) + b = array.array("l", [1, 2]) + mv_a = memoryview(a).cast("B").cast("d", shape=(1, 2)) + mv_b = memoryview(b).cast("B").cast("l", shape=(1, 2)) + + ST.view = mv_a + ST.source = a + try: + with self.assertRaises(BufferError): + # Expect BufferError because the memoryview is locked during comparison + mv_a == mv_b + finally: + ST.view = None + ST.source = None + mv_a.release() + mv_b.release() + def check_attributes_with_type(self, tp): m = self._view(tp(self._source)) self.assertEqual(m.format, self.format) diff --git a/Misc/NEWS.d/next/Library/2025-12-30-22-12-27.gh-issue-142663.gq7iIf.rst b/Misc/NEWS.d/next/Library/2025-12-30-22-12-27.gh-issue-142663.gq7iIf.rst new file mode 100644 index 00000000000000..8bb651c8bf615e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-12-30-22-12-27.gh-issue-142663.gq7iIf.rst @@ -0,0 +1,3 @@ +:class:`memoryview`: Fix a use-after-free crash during comparison when an +overridden :meth:`struct.Struct.unpack_from` releases and resizes the +underlying buffer. diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index f3b7e4a396b4a1..69a93708633fb0 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -3165,6 +3165,13 @@ memory_richcompare(PyObject *v, PyObject *w, int op) goto result; } } + /* Prevent memoryview object from being released and its underlying buffer + reshaped during a mixed format comparison loop. */ + // See https://github.com/python/cpython/issues/142663. + ((PyMemoryViewObject *)v)->exports++; + if (PyMemoryView_Check(w)) { + ((PyMemoryViewObject *)w)->exports++; + } if (vv->ndim == 0) { equal = unpack_cmp(vv->buf, ww->buf, @@ -3183,6 +3190,11 @@ memory_richcompare(PyObject *v, PyObject *w, int op) vfmt, unpack_v, unpack_w); } + ((PyMemoryViewObject *)v)->exports--; + if (PyMemoryView_Check(w)) { + ((PyMemoryViewObject *)w)->exports--; + } + result: if (equal < 0) { if (equal == MV_COMPARE_NOT_IMPL) From 06846a967c36f14e31482f2cd474cb30878fec95 Mon Sep 17 00:00:00 2001 From: superboy-zjc <1826599908@qq.com> Date: Sat, 3 Jan 2026 12:01:11 -0800 Subject: [PATCH 2/2] gh-142663: resolving comments by refactoring unit tests and cache type check result --- Lib/test/test_memoryview.py | 77 +++++-------------- ...-12-30-22-12-27.gh-issue-142663.gq7iIf.rst | 5 +- Objects/memoryobject.c | 7 +- 3 files changed, 27 insertions(+), 62 deletions(-) diff --git a/Lib/test/test_memoryview.py b/Lib/test/test_memoryview.py index 6b5f12554ce181..0203779d994a3d 100644 --- a/Lib/test/test_memoryview.py +++ b/Lib/test/test_memoryview.py @@ -228,66 +228,31 @@ def test_compare(self): self.assertRaises(TypeError, lambda: m >= c) self.assertRaises(TypeError, lambda: c > m) - def test_compare_use_after_free(self): - # Prevent crash in comparisons of memoryview objs with re-entrant struct.unpack_from. + def test_compare_1d_concurrent_mutation(self): + # Prevent crashes during a mixed format 1-D comparison loop. # Regression test for https://github.com/python/cpython/issues/142663. + src1 = array.array("d", [1.0, 2.0]) + src2 = array.array("l", [1, 2]) + mv1, mv2 = memoryview(src1), memoryview(src2) + self.do_test_compare_concurrent_mutation(src1, mv1, mv2) - class ST(struct.Struct): - # Context set by the subtests - view = None - source = None - + def test_compare_2d_concurrent_mutation(self): + # Prevent crashes during a mixed format 2-D comparison loop. + # Regression test for https://github.com/python/cpython/issues/142663. + src1 = array.array("d", [1.0, 2.0]) + src2 = array.array("l", [1, 2]) + mv1 = memoryview(src1).cast("B").cast("d", shape=(1, 2)) + mv2 = memoryview(src2).cast("B").cast("l", shape=(1, 2)) + self.do_test_compare_concurrent_mutation(src1, mv1, mv2) + + def do_test_compare_concurrent_mutation(self, src1, mv1, mv2): + class S(struct.Struct): def unpack_from(self, buf, /, offset=0): - # Attempt to release the buffer while it's being used in comparison loop. - if self.view is not None: - self.view.release() - - # array resize invalidates the buffer pointer used by the comparison loop. - if self.source is not None: - self.source.append(3.14) - + mv1.release() + src1.append(3.14) return (1,) - - with support.swap_attr(struct, "Struct", ST): - # Case 1: 1-D comparison (uses cmp_base optimized loop) - # Use mixed types ('d' vs 'l') to force struct unpacking path. - with self.subTest(ndim=1): - a = array.array("d", [1.0, 2.0]) - b = array.array("l", [1, 2]) - mv_a = memoryview(a) - mv_b = memoryview(b) - - ST.view = mv_a - ST.source = a - try: - with self.assertRaises(BufferError): - # Expect BufferError because the memoryview is locked during comparison - mv_a == mv_b - finally: - ST.view = None - ST.source = None - mv_a.release() - mv_b.release() - - # Case 2: N-D comparison (uses cmp_rec recursive function) - # Use mixed types ('d' vs 'l') to force struct unpacking path. - with self.subTest(ndim=2): - a = array.array("d", [1.0, 2.0]) - b = array.array("l", [1, 2]) - mv_a = memoryview(a).cast("B").cast("d", shape=(1, 2)) - mv_b = memoryview(b).cast("B").cast("l", shape=(1, 2)) - - ST.view = mv_a - ST.source = a - try: - with self.assertRaises(BufferError): - # Expect BufferError because the memoryview is locked during comparison - mv_a == mv_b - finally: - ST.view = None - ST.source = None - mv_a.release() - mv_b.release() + with support.swap_attr(struct, "Struct", S): + self.assertRaises(BufferError, mv1.__eq__, mv2) def check_attributes_with_type(self, tp): m = self._view(tp(self._source)) diff --git a/Misc/NEWS.d/next/Library/2025-12-30-22-12-27.gh-issue-142663.gq7iIf.rst b/Misc/NEWS.d/next/Library/2025-12-30-22-12-27.gh-issue-142663.gq7iIf.rst index 8bb651c8bf615e..39ee1ef29267a7 100644 --- a/Misc/NEWS.d/next/Library/2025-12-30-22-12-27.gh-issue-142663.gq7iIf.rst +++ b/Misc/NEWS.d/next/Library/2025-12-30-22-12-27.gh-issue-142663.gq7iIf.rst @@ -1,3 +1,2 @@ -:class:`memoryview`: Fix a use-after-free crash during comparison when an -overridden :meth:`struct.Struct.unpack_from` releases and resizes the -underlying buffer. +Fix use-after-free crashes when a :class:`memoryview` is mutated +during a comparison with another object. diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index 69a93708633fb0..16b8495659c0d8 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -3122,7 +3122,8 @@ memory_richcompare(PyObject *v, PyObject *w, int op) } vv = VIEW_ADDR(v); - if (PyMemoryView_Check(w)) { + int w_is_mv = PyMemoryView_Check(w); + if (w_is_mv) { if (BASE_INACCESSIBLE(w)) { equal = (v == w); goto result; @@ -3169,7 +3170,7 @@ memory_richcompare(PyObject *v, PyObject *w, int op) reshaped during a mixed format comparison loop. */ // See https://github.com/python/cpython/issues/142663. ((PyMemoryViewObject *)v)->exports++; - if (PyMemoryView_Check(w)) { + if (w_is_mv) { ((PyMemoryViewObject *)w)->exports++; } @@ -3191,7 +3192,7 @@ memory_richcompare(PyObject *v, PyObject *w, int op) } ((PyMemoryViewObject *)v)->exports--; - if (PyMemoryView_Check(w)) { + if (w_is_mv) { ((PyMemoryViewObject *)w)->exports--; }