gh-142884: Fix use-after-free in array.array.tofile() with reentrant writer#144925
Closed
raminfp wants to merge 1 commit intopython:mainfrom
Closed
gh-142884: Fix use-after-free in array.array.tofile() with reentrant writer#144925raminfp wants to merge 1 commit intopython:mainfrom
raminfp wants to merge 1 commit intopython:mainfrom
Conversation
…trant writer array_array_tofile_impl() pre-computed nbytes and nblocks once at the start of the function. If the file-like object's write() callback mutated the array (e.g. by clearing it or replacing its contents), the cached values became stale and subsequent iterations read from freed or invalid memory. Fix by re-checking Py_SIZE(self) on every loop iteration so the loop terminates safely when the array is modified during the write callback.
Member
|
Do not open a PR if there is already one for the issue please. And do not just rely on LLM-generated PRs. In particular, read the devguide: https://devguide.python.org/getting-started/generative-ai/ |
Contributor
Author
|
@picnixz Thank you. I already had the patch prepared and had seen your suggestion as well. I was rewriting the code when you pointed out that a new PR should not be submitted while the previous one is still open. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
array_array_tofile_impl()pre-computesnbytesandnblocksonce at the start of the function then iterates in 64 KB blocks, callingf.write()for each block. Sincef.write()can execute arbitrary Python code, a reentrant writer callback can mutate the array (clear, shrink, or reallocate), leaving the cached values stale. On the next iteration the loop reads from freed or invalid memory, causing a use-after-free, NULL pointer dereference, or heap-buffer-overflow.Fix
Temporarily increment
self->ob_exportsbefore the write loop to mark the array buffer as exported. This prevents any resize operation during thewrite()callback — any attempt to mutate the array raisesBufferError. This is consistent with how the buffer protocol already protects array buffers (e.g. viamemoryview).Tests
Added three regression tests covering the three mutation variants:
test_tofile_reentrant_write_clear—write()tries to clear the arraytest_tofile_reentrant_write_shrink—write()tries to replace contents with a smaller arraytest_tofile_reentrant_write_reallocate—write()tries to clear and append (smaller reallocation)All three now raise
BufferErrorinstead of crashing with a SEGV.array.array.tofilevia reentrant writer #142884