gh-60198: Prevent memoryview pointing to freed heap memory

[furkanonder]

• Issue: memoryview to freed memory can cause segfault #60198

Co-authored-by: Martin Panter vadmium+py@gmail.com

[ambv]

This makes sense to me. The performance impact should be minimal as it's only a cleanup method call after the buffer's been entirely read.

However, since mv.release() can potentially raise, this is a change in behavior so we have to decide whether the risk of existing code breakage is worth backporting the patch to older releases.

I see @gpshead decided this isn't a security issue so we won't be bringing it to 3.8, 3.9, 3.10. Question is if we want it in 3.11 and 3.12.

[oconnor663]

Edit: Ah, my comment below is just an example of point number 1 here: #60198 (comment)

I'm not very familiar with the Python C APIs, so I might be mistaken, but I don't think this change is enough to categorically prevent use-after-free here. This PR is releasing the memoryview that was given to the caller as an argument to readinto, and that does prevent the caller from corrupting memory by holding on to that view, but it doesn't prevent the caller from creating additional views that the BufferedReader doesn't know about before the first one is released. I can recreate the segfault like this:

diff --git a/Lib/test/test_memoryview.py b/Lib/test/test_memoryview.py index 0eac7bc06a..922104d55c 100644 --- a/Lib/test/test_memoryview.py+++ b/Lib/test/test_memoryview.py@@ -660,13 +660,13 @@ def __bool__(self): def test_gh60198(self): global view class File(io.RawIOBase): def readinto(self, buf): global view - view = buf+ view = memoryview(buf) def readable(self): return True f = io.BufferedReader(File()) # get view of buffer used by BufferedReader

$ make test TESTOPTS="-v test_memoryview" ... test_gh60198 (test.test_memoryview.OtherTest.test_gh60198) ... Fatal Python error: Segmentation fault Current thread 0x00007fc1b0b1e740 (most recent call first): File "/home/joconnor/cpython/Lib/test/test_memoryview.py", line 682 in test_gh60198 File "/home/joconnor/cpython/Lib/unittest/case.py", line 589 in _callTestMethod File "/home/joconnor/cpython/Lib/unittest/case.py", line 634 in run File "/home/joconnor/cpython/Lib/unittest/case.py", line 690 in __call__ File "/home/joconnor/cpython/Lib/unittest/suite.py", line 122 in run File "/home/joconnor/cpython/Lib/unittest/suite.py", line 84 in __call__ File "/home/joconnor/cpython/Lib/unittest/suite.py", line 122 in run File "/home/joconnor/cpython/Lib/unittest/suite.py", line 84 in __call__ File "/home/joconnor/cpython/Lib/unittest/suite.py", line 122 in run File "/home/joconnor/cpython/Lib/unittest/suite.py", line 84 in __call__ File "/home/joconnor/cpython/Lib/unittest/runner.py", line 240 in run File "/home/joconnor/cpython/Lib/test/support/__init__.py", line 1115 in _run_suite File "/home/joconnor/cpython/Lib/test/support/__init__.py", line 1241 in run_unittest File "/home/joconnor/cpython/Lib/test/libregrtest/runtest.py", line 294 in _test_module File "/home/joconnor/cpython/Lib/test/libregrtest/runtest.py", line 330 in _runtest_inner2 File "/home/joconnor/cpython/Lib/test/libregrtest/runtest.py", line 373 in _runtest_inner File "/home/joconnor/cpython/Lib/test/libregrtest/runtest.py", line 248 in _runtest File "/home/joconnor/cpython/Lib/test/libregrtest/runtest.py", line 278 in runtest File "/home/joconnor/cpython/Lib/test/libregrtest/main.py", line 365 in rerun_failed_tests File "/home/joconnor/cpython/Lib/test/libregrtest/main.py", line 803 in _main File "/home/joconnor/cpython/Lib/test/libregrtest/main.py", line 758 in main File "/home/joconnor/cpython/Lib/test/libregrtest/main.py", line 822 in main File "/home/joconnor/cpython/Lib/test/__main__.py", line 2 in <module> File "<frozen runpy>", line 88 in _run_code File "<frozen runpy>", line 198 in _run_module_as_main Extension modules: _testcapi (total: 1) make: *** [Makefile:1983: test] Segmentation fault (core dumped)