Skip to content

Commit ea1312b

Browse files
vstinnercmaloneyyihong0618brijkapadiaashm-dev
authored
[3.15] gh-157364: Fix use-after-free in io.TextIOWrapper during reentrant detach (#159591)
* gh-143008: Fix Null pointer dereferences in TextIOWrapper underlying stream access (#145957) TextIOWrapper keeps its underlying stream in a member called `self->buffer`. That stream can be detached by user code, such as custom `.flush` implementations resulting in `self->buffer` being set to NULL. The implementation often checked at the start of functions if `self->buffer` is in a good state, but did not always recheck after other Python code was called which could modify `self->buffer`. The cases which need to be re-checked are hard to spot so rather than rely on reviewer effort create better safety by making all self->buffer access go through helper functions. Thank you yihong0618 for the test, NEWS and initial implementation in gh-143041. Co-authored-by: yihong0618 <zouzou0208@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org> (cherry picked from commit db4b194) * gh-154523: Fix data-race in `TextIOWrapper.detach()` (#154565) (cherry picked from commit 8440a6a) * gh-157364: Fix use-after-free in io.TextIOWrapper during reentrant detach (#157370) Co-authored-by: Victor Stinner <vstinner@python.org> (cherry picked from commit e19dc47) --------- Co-authored-by: Cody Maloney <cmaloney@users.noreply.github.com> Co-authored-by: yihong0618 <zouzou0208@gmail.com> Co-authored-by: Brij Kapadia <97006829+brijkapadia@users.noreply.github.com> Co-authored-by: Shamil <ashm.tech@proton.me>
1 parent db85e76 commit ea1312b

8 files changed

Lines changed: 276 additions & 39 deletions

File tree

‎Lib/test/test_free_threading/test_io.py‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,3 +198,23 @@ def reset_worker():
198198
decoder.reset()
199199

200200
run_concurrently([decode_worker] * 2 + [reset_worker] * 2)
201+
202+
203+
class TextIOWrapperTest(TestCase):
204+
def test_buffer_detach_race(self):
205+
make = lambda: io.TextIOWrapper(io.BytesIO())
206+
slot = [make()]
207+
208+
def reader():
209+
for _ in range(1000):
210+
try:
211+
slot[0].buffer
212+
except ValueError:
213+
pass
214+
215+
def detacher():
216+
for _ in range(1000):
217+
slot[0] = make()
218+
slot[0].detach()
219+
220+
run_concurrently([reader, detacher])

‎Lib/test/test_io/test_textio.py‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,6 +1626,86 @@ def closed(self):
16261626
wrapper = self.TextIOWrapper(raw)
16271627
wrapper.close() # should not crash
16281628

1629+
def test_reentrant_detach_during_flush(self):
1630+
# gh-143008: Reentrant detach() during flush should not crash.
1631+
1632+
class DetachOnce(self.BufferedRandom):
1633+
wrapper = None
1634+
1635+
def detach_once(self):
1636+
original = self.wrapper
1637+
self.wrapper = None
1638+
if original is not None:
1639+
original.detach()
1640+
original.flush()
1641+
1642+
class DetachOnFlush(DetachOnce):
1643+
def flush(self):
1644+
self.detach_once()
1645+
1646+
class DetachOnWrite(DetachOnce):
1647+
def write(self, b):
1648+
self.detach_once()
1649+
return len(b)
1650+
1651+
# Separate reference for after detach_once.
1652+
wrapper = None
1653+
1654+
def make_text(buffer):
1655+
nonlocal wrapper
1656+
buffer.wrapper = self.TextIOWrapper(buffer, encoding='utf-8')
1657+
wrapper = buffer.wrapper
1658+
1659+
# Many calls could result in the same null self->buffer crash.
1660+
tests = [
1661+
('truncate', lambda: wrapper.truncate(0)),
1662+
('close', lambda: wrapper.close()),
1663+
('detach', lambda: wrapper.detach()),
1664+
('seek', lambda: wrapper.seek(0)),
1665+
('tell', lambda: wrapper.tell()),
1666+
('reconfigure', lambda: wrapper.reconfigure(line_buffering=True)),
1667+
]
1668+
for name, method in tests:
1669+
with self.subTest(name):
1670+
make_text(DetachOnFlush(self.MockRawIO()))
1671+
self.assertRaisesRegex(ValueError, "detached", method)
1672+
1673+
# Should not crash.
1674+
with self.subTest('read via writeflush'):
1675+
make_text(DetachOnWrite(self.MockRawIO()))
1676+
wrapper.write('x')
1677+
self.assertRaisesRegex(ValueError, "detached", wrapper.read)
1678+
1679+
def test_reentrant_detach_during_read(self):
1680+
# gh-157363, gh-157364: The buffer must stay alive until its active
1681+
# read operation returns.
1682+
wrapper = None
1683+
1684+
class DetachOnRead(self.RawIOBase):
1685+
detached = False
1686+
1687+
def readable(self):
1688+
return True
1689+
1690+
def readinto(self, b):
1691+
if self.detached:
1692+
return 0
1693+
self.detached = True
1694+
wrapper.detach()
1695+
b[:3] = b"ab\n"
1696+
return 3
1697+
1698+
for method_name in ("read", "readline"):
1699+
with self.subTest(method_name):
1700+
raw = DetachOnRead()
1701+
wrapper = self.TextIOWrapper(
1702+
self.BufferedReader(raw), encoding="utf-8")
1703+
method = getattr(wrapper, method_name)
1704+
self.assertEqual(method(), "ab\n")
1705+
with self.assertRaisesRegex(ValueError,
1706+
"underlying buffer has been detached"):
1707+
getattr(wrapper, 'buffer')
1708+
16291709

16301710
class PyTextIOWrapperTest(TextIOWrapperTest, PyTestCase):
16311711
shutdown_error = "LookupError: unknown encoding: ascii"
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix crash in :class:`io.TextIOWrapper` when reentrant
2+
:meth:`io.TextIOBase.detach` is called reentrantly from the underlying buffer.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix race conditions when re-initializing a :class:`io.TextIOWrapper` object.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fixed data-race when calling :meth:`io.TextIOBase.detach` in
2+
:term:`free-threaded build`.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a use-after-free in :class:`io.TextIOWrapper` when a call to the
2+
underlying buffer reentrantly detaches it. Patched by Shamil Abdulaev.

‎Modules/_io/clinic/textio.c.h‎

Lines changed: 26 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)