Skip to content

gh-142663: Fix use-after-free in memoryview comparison #143305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
superboy-zjc wants to merge 3 commits into
base: main
Choose a base branch
from
+42 −1
Open

gh-142663: Fix use-after-free in memoryview comparison #143305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
edd566c
gh-142663: Fix use-after-free in memoryview comparison
superboy-zjc Dec 31, 2025
06846a9
gh-142663: resolving comments by refactoring unit tests and cache typ…
superboy-zjc Jan 3, 2026
0694b20
Merge branch 'main' of https://github.com/python/cpython into issue-1…
superboy-zjc Jan 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions Lib/test/test_memoryview.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,32 @@ def test_compare(self):
self.assertRaises(TypeError, lambda: m >= c)
self.assertRaises(TypeError, lambda: c > m)

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)

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):
mv1.release()
src1.append(3.14)
return (1,)
with support.swap_attr(struct, "Struct", S):
Copy link
Member

@sobolevn sobolevn Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line puzzles me a little bit. When you swap classes in the stdlib, you can get unexpected results. Can we replicate this without monkeypatching?

Copy link
Author

@superboy-zjc superboy-zjc Jan 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sobolevn I doubt the possibility of replicating this without monkeypatching. The issue stems from, during a mixed format buffer comparison, memory_richcompare function calls a re-entrant struct.Struct.unpack_from that is loaded by a PyImport_ImportModuleAttrString("struct", "Struct") call in the struct_get_unpacker. Is there anyway you would recommend to work around it ?

/* Return a new unpacker for the given format. */
static struct unpacker *
struct_get_unpacker(const char *fmt, Py_ssize_t itemsize)
{
    ...

    // struct.Struct Importation
    Struct = PyImport_ImportModuleAttrString("struct", "Struct");
    if (Struct == NULL)
        return NULL;

    x = unpacker_new();

    ...

    x->unpack_from = PyObject_GetAttrString(structobj, "unpack_from");
    if (x->unpack_from == NULL)
        goto error;
    ...

}

Copy link
Member

@sobolevn sobolevn Jan 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are countless potential issues if we replace stdlib types with something else. Let's please first discuss in the issue whether this is even a valid bug report.

Copy link
Member

@picnixz picnixz Jan 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did consider it as a crash because we also fixed similar occurrences elsewhere. If we just had an exception it would have been ok, but here we have a crash. We do not want this.

Copy link
Author

@superboy-zjc superboy-zjc Jan 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see there are many tests having monkey patch. Why does this case make difference? Is there any update regarding this pr or should we @ more core dev to take a look?

self.assertRaises(BufferError, mv1.__eq__, mv2)

def check_attributes_with_type(self, tp):
m = self._view(tp(self._source))
self.assertEqual(m.format, self.format)
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2025-12-30-22-12-27.gh-issue-142663.gq7iIf.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix use-after-free crashes when a :class:`memoryview` is mutated
during a comparison with another object.
15 changes: 14 additions & 1 deletion Objects/memoryobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -3165,6 +3166,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 (w_is_mv) {
((PyMemoryViewObject *)w)->exports++;
}

if (vv->ndim == 0) {
equal = unpack_cmp(vv->buf, ww->buf,
Expand All @@ -3183,6 +3191,11 @@ memory_richcompare(PyObject *v, PyObject *w, int op)
vfmt, unpack_v, unpack_w);
}

((PyMemoryViewObject *)v)->exports--;
if (w_is_mv) {
((PyMemoryViewObject *)w)->exports--;
}

result:
if (equal < 0) {
if (equal == MV_COMPARE_NOT_IMPL)
Expand Down
Loading