gh-143375: Fix a crash in BufferedWriter.seek when passing an object with a specially crafted __index__#143577
gh-143375: Fix a crash in BufferedWriter.seek when passing an object with a specially crafted __index__#143577tomasr8 wants to merge 14 commits intopython:mainfrom
BufferedWriter.seek when passing an object with a specially crafted __index__#143577Conversation
Misc/NEWS.d/next/Core_and_Builtins/2026-01-08-19-56-40.gh-issue-143375.QCzA_8.rst
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,2 @@ | |||
| Fix a crash in the ``seek`` method of :class:`~io.BufferedWriter` when | |||
There was a problem hiding this comment.
Is it only BufferedWriter or other buffered streams?
There was a problem hiding this comment.
It's also BufferedReader and BufferedRandom since they share the same implementation. I updated the news entry and added tests!
Modules/_io/bufferedio.c
Outdated
| // PyNumber_AsOff_t calls user code via __index__, which | ||
| // could have closed the file. | ||
| CHECK_CLOSED(self, "seek of closed file") |
There was a problem hiding this comment.
I do not think this comment is needed. It is trivial if we do all right, and we do not what to add such comments for each second line.
Remove CHECK_CLOSED above, it is redundant. Also, I think it is better to move PyNumber_AsOff_t immediately after the whence check.
There was a problem hiding this comment.
Oh, and CHECK_INITIALIZED also should be after PyNumber_AsOff_t. Add a test for concurrent detach().
There was a problem hiding this comment.
I do not think this comment is needed.
Removed
Remove CHECK_CLOSED above, it is redundant.
Also removed
Also, I think it is better to move PyNumber_AsOff_t immediately after the whence check.
Done
Add a test for concurrent detach().
Added!
| @@ -0,0 +1,2 @@ | |||
| Fix a crash in the ``seek`` method of :class:`~io.BufferedWriter` when | |||
| passing an object with a specially crafted :meth:`~object.__index__`. | |||
There was a problem hiding this comment.
No, a specially crafted __index__() is not a cause, it is only a mean of reproducing the issue. The root issue is that the buffered stream can be closed (or detached?) concurrently with seek(). This can happen in other thread (or in the garbage collector), when the GIL is released in not so special custom __index__().
There was a problem hiding this comment.
True, I updated the news entry with the root cause. Let me know if it's ok!
|
And I think TextIOWrapper has the same issue. See #143007. |
3 participants
Adds a check after the call to
PyNumber_AsOff_twhich could have closed the file as a side-effect.BufferedWriter.seekduring re-entrant close #143375