-
-
Notifications
You must be signed in to change notification settings - Fork 33.8k
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.
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
base: main
Are you sure you want to change the base?
Conversation
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`.
| reshaped during a mixed format comparison loop. */ | ||
| // See https://github.com/python/cpython/issues/142663. | ||
| ((PyMemoryViewObject *)v)->exports++; | ||
| if (PyMemoryView_Check(w)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can cache this check as we're already using it above.
| :class:`memoryview`: Fix a use-after-free crash during comparison when an | ||
| overridden :meth:`struct.Struct.unpack_from` releases and resizes the | ||
| underlying buffer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| :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. |
| # Regression test for https://github.com/python/cpython/issues/142663. | ||
|
|
||
| class ST(struct.Struct): | ||
| # Context set by the subtests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid useless or trivial comments. You can just use variables in the function directly instead of attributes.
| self.assertRaises(TypeError, lambda: m >= c) | ||
| self.assertRaises(TypeError, lambda: c > m) | ||
|
|
||
| def test_compare_use_after_free(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rewrite the test as follows:
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):
self.assertRaises(BufferError, mv1.__eq__, mv2)and use them as:
def test_compare_1d_concurrent_mutation(self):
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):
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)(put the helper function after the tests that use it)
When comparing two memoryview objects with different formats,
memory_richcompareuses thestructmodule to unpack elements. A customstruct.Struct.unpack_fromimplementation 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
exportscount of the memoryview objects before performing the comparison, effectively locking the buffers. This mirrors the protection already provided for non-memoryview objects viaPyObject_GetBuffer.memoryviewcomparison via re-entrantstruct.Struct.unpack_from#142663