Repository navigation
memoryview to freed memory can cause segfault #60198
Description
Activity
sbt commented
on Sep 20, 2012 sbtmannequinMannequinAuthorA memoryview which does not own a reference to its base object can point to freed or reallocated memory. For instance the following segfaults for me on Windows and Linux.
import io class File(io.RawIOBase): def readinto(self, buf): global view view = buf def readable(self): return True f = io.BufferedReader(File()) f.read(1) # get view of buffer used by BufferedReader del f # deallocate buffer view = view.cast('P') L = [None] * len(view) # create list whose array has same size # (this will probably coincide with view) view[0] = 0 # overwrite first item with NULL print(L[0]) # segfault: dereferencing NULL
I realize there are easier ways to make Python segfault, so maybe this should not be considered a serious issue. But I think there should be some way of guaranteeing that a memoryview will not try to access memory which has already been freed.
In bpo-15903 skrah proposed exposing memory_release() as PyBuffer_Release(). However, I don't think that would necessarily invalidate all exports of the buffer.
Alternatively, one could incref the buffered reader object and set mview->mbuf->obj to it. Maybe we could have
PyMemoryView_FromMemoryEx(char *mem, Py_ssize_t size, int flags, PyObject *obj)
which guarantees that if obj is non-NULL then it will not be garbage collected before the memoryview. This should *not* expose obj as an attribute of the memoryview.
BreamoreBoy commented
on Jul 16, 2014 BreamoreBoymannequinMannequinCan someone follow this up noting it refers to bpo-15903.
skrah commented
on Jul 18, 2014 skrahmannequinMannequinWe deal with it when we have time. IMO there is little value in
bumping up issues this way.Saving a reference to the overall buffered reader is probably not good enough. I think the buffer is freed when close() is called, so a reference to this internal buffer would need to be saved instead (or as well). Unless we stop having close() free the buffer.
skrah commented
on Oct 31, 2015 skrahmannequinMannequinBut I think there should be some way of guaranteeing that a memoryview will not try to access memory which has already been freed.
Unless the "buffered *self" parameter in _buffered_raw_read() is
made a full PEP-3118 exporter, I'm not sure how it would be possible
to prevent all examples of this kind.I recently created bpo-26720 about a similar situation with BufferedWriter. However I am starting to believe that the problem cannot be solved the way I originally wanted. Instead, the best solution there would be similar what I would suggest here: we need to avoid the user accessing the memoryview after readinto() or write() has returned. Some ideas, not all perfect:
-
Call memoryview.release() after the method returns. This would be simple and practical, but could be cheated by making a second memoryview of the original. Also, memoryview.release() is not guaranteed to succeed; there is code that raises BufferError("memoryview has exported buffers").
-
Raise an exception (BufferError or RuntimeError?) if readinto() or write() returns and the memoryview(s) are not all released. This doesn’t prevent a determined programmer from overwriting memory or losing written data, but it might let them know about the problem when it happens.
-
Try to force the memoryview.release() state on all views into the original memory. Is this practical? I guess this would be a bit inconsistent if some other thread was in the middle of a system call using the buffer at the time that we want to do the release.
-
I think idea 2 (error if memoryview not released) would not work. It would rely on garbage collection, which is not always immediate. E.g. if the memoryview were kept alive by a reference cycle, it may not immediately be released. I don’t think idea 3 is worthwhile, because the memoryview API does not seem designed for this use case, and it would not be 100% consistent anyway.
So that leaves my first idea, to call view.release() when readinto(), etc, return. This should avoid the crash and data loss problems in some common cases. It is implemented in release-view.patch, along with some notes.
Just stumbled upon https://pwn.win/2022/05/11/python-buffered-reader.html which is an “exploit” for this //cc @vstinner
9 remaining items
As a fourth option to add to @vadmium's three options above, could it also be possible to replace the raw
self->bufferallocation with an internalbytearray? In that case, if the caller smuggles out amemoryviewlike we're talking about here, the result would be that theBufferedReaderis in a bad state where any resizing operations on its internalbytearraywill fail. As long as those failures get propagated to the caller correctly and don't lead to any other memory corruption, then all a pathological caller can do is cause more exceptions for themselves.Idea 1 has the same problem as idea 2. If the memoryview were kept alive by a reference cycle, release() will fail. It will leave us in the same state, except that the user code maybe has a chance to report an error before crash.
The problem in the buffered reader can be solved by using bytearray as a buffer (the code will be complicated by necessary to support multiple buffers if they are not yet released). But
PyUnicode_Decode()is more hopeless case. It takes a pointer to raw buffer and have no control on its lifespan. The only reliable way is to copy it to a nyw bytes or bytearray object, but it would be grossy inefficient.- moved this to Todo in Struct, memoryview and array issues 🏗️
on Jul 13, 2026
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs