diff --git a/Lib/test/test_io/test_bufferedio.py b/Lib/test/test_io/test_bufferedio.py index e83dd0d4e28d006..3b3b16f0da7d46e 100644 --- a/Lib/test/test_io/test_bufferedio.py +++ b/Lib/test/test_io/test_bufferedio.py @@ -623,6 +623,75 @@ def test_bad_readinto_type(self): bufio.readline() self.assertIsInstance(cm.exception.__cause__, TypeError) + def test_reentrant_detach_during_read_all(self): + # gh-154997: Reentrant detach() during read_all() should not crash. + # Use a duck-typed raw stream so _bufferedreader_read_all() + # dispatches through raw.read(). Return EOF to terminate the loop. + class Duck: + closed = False + + def __init__(self): + self.n = 0 + + def readable(self): + return True + + def writable(self): + return False + + def seekable(self): + return False + + def close(self): + pass + + def flush(self): + pass + + def read(self, *args): + self.n += 1 + if self.n == 1: + self.buf.detach() + return b"abc" if self.n < 3 else b"" + + def readinto(self, b): + data = self.read() + b[0:len(data)] = data + return len(data) + + raw = Duck() + buf = self.tp(raw) + raw.buf = buf + + with self.assertRaisesRegex(ValueError, "detached"): + buf.read() + + def test_reentrant_detach_during_raw_read(self): + # gh-154997: Reentrant detach() during read() should not crash. + # detach() fires from inside raw.readinto(), which + # _bufferedreader_raw_read() calls directly. + class Raw(io.RawIOBase): + def __init__(self): + super().__init__() + self.fired = False + + def readable(self): + return True + + def readinto(self, b): + if not self.fired: + self.fired = True + self.buf.detach() + b[0:1] = b"a" + return 1 + + raw = Raw() + buf = self.tp(raw, buffer_size=4) + raw.buf = buf + + with self.assertRaisesRegex(ValueError, "detached"): + buf.read(64) + @unittest.skipUnless(sys.maxsize > 2**32, 'requires 64bit platform') @unittest.skipIf(check_sanitizer(thread=True), 'ThreadSanitizer aborts on huge allocations (exit code 66).') @@ -1002,6 +1071,209 @@ def closed(self): self.assertRaisesRegex(ValueError, "test", bufio.flush) self.assertRaisesRegex(ValueError, "test", bufio.close) + def test_reentrant_detach_during_close(self): + # gh-154997: Reentrant detach() during close() should not crash. + + class B(self.tp): + armed = True + + def flush(self): + if self.armed: + self.armed = False + super().detach() + + buf = B(self.BytesIO()) + with self.assertRaisesRegex(ValueError, "detached"): + buf.close() + + def test_reentrant_detach_during_raw_write(self): + # gh-154997: Reentrant detach() during write() should not crash. + # Use a small buffer and partial writes so write() reaches + # _bufferedwriter_raw_write(). Override flush() to avoid + # re-entering the buffered lock during detach(). + class B(self.tp): + def flush(self): + return None + + class Raw(io.RawIOBase): + def __init__(self): + super().__init__() + self.fired = False + + def writable(self): + return True + + def write(self, b): + if not self.fired: + self.fired = True + self.buf.detach() + return 1 # partial write -> flush loop iterates again + + raw = Raw() + buf = B(raw, buffer_size=4) + raw.buf = buf + + with self.assertRaisesRegex(ValueError, "detached"): + buf.write(b"0123456789abcdef") + + def test_reentrant_detach_during_truncate(self): + # gh-154997: Reentrant detach() during truncate() should not crash. + # Avoid the seek path and make flush() a no-op so detach() + # exercises the guarded truncate path without re-entering + # the buffered lock. + class B(self.tp): + def flush(self): + return None + + class Raw(io.RawIOBase): + def __init__(self): + super().__init__() + self.fired = False + + def readable(self): + return False + + def writable(self): + return True + + def seekable(self): + return True + + def tell(self): + return 0 + + def seek(self, pos, whence=0): + return 0 + + def truncate(self, pos=None): + return 0 + + def write(self, b): + if not self.fired: + self.fired = True + self.buf.detach() + return len(b) + + raw = Raw() + buf = B(raw, buffer_size=64) + raw.buf = buf + + buf.write(b"012") + with self.assertRaisesRegex(ValueError, "detached"): + buf.truncate(1) + + def test_reentrant_detach_during_raw_tell(self): + # gh-154997: After detach(), truncate() calls _buffered_raw_tell() + # to refresh the cached position. That ValueError is intentionally + # swallowed, so verify the guarded path by checking raw.tell() is + # not called again after detach(). + class B(self.tp): + def flush(self): + return None + + class Raw(io.RawIOBase): + def __init__(self): + super().__init__() + self.fired = False + self.tell_calls = 0 + + def readable(self): + return False + + def writable(self): + return True + + def seekable(self): + return True + + def tell(self): + self.tell_calls += 1 + return 0 + + def seek(self, pos, whence=0): + return 0 + + def write(self, b): + return len(b) + + def truncate(self, pos=None): + if not self.fired: + self.fired = True + self.buf.detach() + return 0 + + raw = Raw() + buf = B(raw, buffer_size=64) + raw.buf = buf + + # _buffered_init() calls _buffered_raw_tell() once at construction. + self.assertEqual(raw.tell_calls, 1) + + buf.write(b"012") + # Must not crash; the swallowed ValueError means truncate() itself + # still reports success, unchanged from pre-detach behavior. + self.assertEqual(buf.truncate(1), 0) + + # The critical assertion: raw_access_safe() short-circuited the + # post-truncate tell() call -- tell_calls stayed at 1, it did NOT + # increment to 2. Before the fix this dispatched through a NULL + # self->raw and crashed with SIGSEGV. + self.assertEqual(raw.tell_calls, 1) + + def test_reentrant_detach_during_raw_seek(self): + # gh-154997: After detach(), seek() calls _buffered_raw_seek(). + # Verify the guarded path by checking raw.seek() is not called + # again after detach(). + + class B(self.tp): + def flush(self): + return None + + class Raw(io.RawIOBase): + def __init__(self): + super().__init__() + self.fired = False + self.seek_calls = 0 + + def readable(self): + return False + + def writable(self): + return True + + def seekable(self): + return True + + def tell(self): + return 0 + + def seek(self, pos, whence=0): + self.seek_calls += 1 + return 0 + + def write(self, b): + return len(b) + + def truncate(self, pos=None): + if not self.fired: + self.fired = True + self.buf.detach() + return 0 + + raw = Raw() + buf = B(raw, buffer_size=64) + raw.buf = buf + + # _buffered_init() performs one seek() during initialization. + initial_seek_calls = raw.seek_calls + + buf.write(b"012") + self.assertEqual(buf.truncate(1), 0) + + # _buffered_raw_seek() should not dispatch through a detached raw + # object, so no additional seek() should have occurred. + self.assertEqual(raw.seek_calls, initial_seek_calls) + class PyBufferedWriterTest(BufferedWriterTest, PyTestCase): tp = pyio.BufferedWriter diff --git a/Misc/NEWS.d/next/Library/2026-08-01-09-52-23.gh-issue-154997.wcYpao.rst b/Misc/NEWS.d/next/Library/2026-08-01-09-52-23.gh-issue-154997.wcYpao.rst new file mode 100644 index 000000000000000..61c74a8d27af25e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-08-01-09-52-23.gh-issue-154997.wcYpao.rst @@ -0,0 +1,3 @@ +Fixed a crash in :mod:`io` buffered streams when ``detach()`` is called +reentrantly during buffered operations. The affected operations now raise +``ValueError`` instead of dereferencing a stale raw stream pointer. diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 5537947f6a51c11..42b54d245819437 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -471,6 +471,35 @@ buffered_traverse(PyObject *op, visitproc visit, void *arg) return 0; } +static PyObject * +raw_access_safe(buffered *self) +{ + /* Similar to textio.c's buffer_access_safe(), but preserves + _Buffered's detached and uninitialized error semantics. + + Unlike buffer_access_safe(), we do not assert that a critical + section is held. This helper protects against same-thread + reentrant detach(), and it is also used during __init__, + where no critical section is held. + + Return a new reference so callers safely hold the raw object + across callbacks that may detach it. + */ + if (self->raw == NULL) { + if (self->detached) { + PyErr_SetString(PyExc_ValueError, + "raw stream has been detached"); + } + else { + PyErr_SetString(PyExc_ValueError, + "I/O operation on uninitialized object"); + } + return NULL; + } + + return Py_NewRef(self->raw); +} + /* Because this can call arbitrary code, it shouldn't be called when the refcount is 0 (that is, not directly from tp_dealloc unless the refcount has been temporarily re-incremented). */ @@ -588,7 +617,11 @@ _io__Buffered_close_impl(buffered *self) exc = PyErr_GetRaisedException(); } - res = PyObject_CallMethodNoArgs(self->raw, &_Py_ID(close)); + PyObject *raw = raw_access_safe(self); + if (raw != NULL) { + res = PyObject_CallMethodNoArgs(raw, &_Py_ID(close)); + Py_DECREF(raw); + } if (self->buffer) { PyMem_Free(self->buffer); @@ -730,6 +763,7 @@ _io__Buffered_isatty_impl(buffered *self) /* Forward decls */ static PyObject * _bufferedwriter_flush_unlocked(buffered *); + static Py_ssize_t _bufferedreader_fill_buffer(buffered *self); static void @@ -785,16 +819,24 @@ _buffered_raw_tell(buffered *self) { Py_off_t n; PyObject *res; - res = PyObject_CallMethodNoArgs(self->raw, &_Py_ID(tell)); + PyObject *raw = raw_access_safe(self); + if (raw == NULL) { + return -1; + } + + res = PyObject_CallMethodNoArgs(raw, &_Py_ID(tell)); + Py_DECREF(raw); if (res == NULL) return -1; + n = PyNumber_AsOff_t(res, PyExc_ValueError); Py_DECREF(res); if (n < 0) { - if (!PyErr_Occurred()) + if (!PyErr_Occurred()) { PyErr_Format(PyExc_OSError, "Raw stream returned invalid position %" PY_PRIdOFF, (PY_OFF_T_COMPAT)n); + } return -1; } self->abs_pos = n; @@ -804,32 +846,46 @@ _buffered_raw_tell(buffered *self) static Py_off_t _buffered_raw_seek(buffered *self, Py_off_t target, int whence) { + PyObject *raw; PyObject *res, *posobj, *whenceobj; Py_off_t n; + raw = raw_access_safe(self); + if (raw == NULL) { + return -1; + } + posobj = PyLong_FromOff_t(target); if (posobj == NULL) return -1; + whenceobj = PyLong_FromLong(whence); if (whenceobj == NULL) { Py_DECREF(posobj); return -1; } - res = PyObject_CallMethodObjArgs(self->raw, &_Py_ID(seek), + + res = PyObject_CallMethodObjArgs(raw, &_Py_ID(seek), posobj, whenceobj, NULL); + Py_DECREF(raw); Py_DECREF(posobj); Py_DECREF(whenceobj); + if (res == NULL) return -1; + n = PyNumber_AsOff_t(res, PyExc_ValueError); Py_DECREF(res); + if (n < 0) { - if (!PyErr_Occurred()) + if (!PyErr_Occurred()) { PyErr_Format(PyExc_OSError, "Raw stream returned invalid position %" PY_PRIdOFF, (PY_OFF_T_COMPAT)n); + } return -1; } + self->abs_pos = n; return n; } @@ -1482,7 +1538,13 @@ _io__Buffered_truncate_impl(buffered *self, PyTypeObject *cls, PyObject *pos) } Py_CLEAR(res); - res = PyObject_CallMethodOneArg(self->raw, &_Py_ID(truncate), pos); + PyObject *raw = raw_access_safe(self); + if (raw == NULL) { + goto end; + } + + res = PyObject_CallMethodOneArg(raw, &_Py_ID(truncate), pos); + Py_DECREF(raw); if (res == NULL) goto end; /* Reset cached position */ @@ -1637,7 +1699,14 @@ _bufferedreader_raw_read(buffered *self, char *start, Py_ssize_t len) raised (see issue #10956). */ do { - res = PyObject_CallMethodOneArg(self->raw, &_Py_ID(readinto), memobj); + PyObject *raw = raw_access_safe(self); + if (raw == NULL) { + Py_DECREF(memobj); + return -1; + } + + res = PyObject_CallMethodOneArg(raw, &_Py_ID(readinto), memobj); + Py_DECREF(raw); } while (res == NULL && _PyIO_trap_eintr()); Py_DECREF(memobj); if (res == NULL) @@ -1745,7 +1814,13 @@ _bufferedreader_read_all(buffered *self) } /* Read until EOF or until read() would block. */ - data = PyObject_CallMethodNoArgs(self->raw, &_Py_ID(read)); + PyObject *raw = raw_access_safe(self); + if (raw == NULL) { + goto cleanup; + } + + data = PyObject_CallMethodNoArgs(raw, &_Py_ID(read)); + Py_DECREF(raw); if (data == NULL) goto cleanup; if (data != Py_None && !PyBytes_Check(data)) { @@ -1992,9 +2067,16 @@ _bufferedwriter_raw_write(buffered *self, char *start, Py_ssize_t len) raised (see issue #10956). */ do { + PyObject *raw = raw_access_safe(self); + if (raw == NULL) { + Py_DECREF(memobj); + return -1; + } + errno = 0; - res = PyObject_CallMethodOneArg(self->raw, &_Py_ID(write), memobj); + res = PyObject_CallMethodOneArg(raw, &_Py_ID(write), memobj); errnum = errno; + Py_DECREF(raw); } while (res == NULL && _PyIO_trap_eintr()); Py_DECREF(memobj); if (res == NULL)