Skip to content

Commit 13fc16c

Browse files
vstinnercmaloneyyihong0618brijkapadiaashm-dev
authored
[3.14] gh-157364: Fix use-after-free in io.TextIOWrapper during reentrant detach (#159600)
[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. (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) (cherry picked from commit e19dc47) --------- (cherry picked from commit ea1312b) 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 566f815 commit 13fc16c

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
@@ -191,3 +191,23 @@ def reset_worker():
191191
decoder.reset()
192192

193193
run_concurrently([decode_worker] * 2 + [reset_worker] * 2)
194+
195+
196+
class TextIOWrapperTest(TestCase):
197+
def test_buffer_detach_race(self):
198+
make = lambda: io.TextIOWrapper(io.BytesIO())
199+
slot = [make()]
200+
201+
def reader():
202+
for _ in range(1000):
203+
try:
204+
slot[0].buffer
205+
except ValueError:
206+
pass
207+
208+
def detacher():
209+
for _ in range(1000):
210+
slot[0] = make()
211+
slot[0].detach()
212+
213+
run_concurrently([reader, detacher])

‎Lib/test/test_io.py‎

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

4279+
def test_reentrant_detach_during_flush(self):
4280+
# gh-143008: Reentrant detach() during flush should not crash.
4281+
4282+
class DetachOnce(self.BufferedRandom):
4283+
wrapper = None
4284+
4285+
def detach_once(self):
4286+
original = self.wrapper
4287+
self.wrapper = None
4288+
if original is not None:
4289+
original.detach()
4290+
original.flush()
4291+
4292+
class DetachOnFlush(DetachOnce):
4293+
def flush(self):
4294+
self.detach_once()
4295+
4296+
class DetachOnWrite(DetachOnce):
4297+
def write(self, b):
4298+
self.detach_once()
4299+
return len(b)
4300+
4301+
# Separate reference for after detach_once.
4302+
wrapper = None
4303+
4304+
def make_text(buffer):
4305+
nonlocal wrapper
4306+
buffer.wrapper = self.TextIOWrapper(buffer, encoding='utf-8')
4307+
wrapper = buffer.wrapper
4308+
4309+
# Many calls could result in the same null self->buffer crash.
4310+
tests = [
4311+
('truncate', lambda: wrapper.truncate(0)),
4312+
('close', lambda: wrapper.close()),
4313+
('detach', lambda: wrapper.detach()),
4314+
('seek', lambda: wrapper.seek(0)),
4315+
('tell', lambda: wrapper.tell()),
4316+
('reconfigure', lambda: wrapper.reconfigure(line_buffering=True)),
4317+
]
4318+
for name, method in tests:
4319+
with self.subTest(name):
4320+
make_text(DetachOnFlush(self.MockRawIO()))
4321+
self.assertRaisesRegex(ValueError, "detached", method)
4322+
4323+
# Should not crash.
4324+
with self.subTest('read via writeflush'):
4325+
make_text(DetachOnWrite(self.MockRawIO()))
4326+
wrapper.write('x')
4327+
self.assertRaisesRegex(ValueError, "detached", wrapper.read)
4328+
4329+
def test_reentrant_detach_during_read(self):
4330+
# gh-157363, gh-157364: The buffer must stay alive until its active
4331+
# read operation returns.
4332+
wrapper = None
4333+
4334+
class DetachOnRead(self.RawIOBase):
4335+
detached = False
4336+
4337+
def readable(self):
4338+
return True
4339+
4340+
def readinto(self, b):
4341+
if self.detached:
4342+
return 0
4343+
self.detached = True
4344+
wrapper.detach()
4345+
b[:3] = b"ab\n"
4346+
return 3
4347+
4348+
for method_name in ("read", "readline"):
4349+
with self.subTest(method_name):
4350+
raw = DetachOnRead()
4351+
wrapper = self.TextIOWrapper(
4352+
self.BufferedReader(raw), encoding="utf-8")
4353+
method = getattr(wrapper, method_name)
4354+
self.assertEqual(method(), "ab\n")
4355+
with self.assertRaisesRegex(ValueError,
4356+
"underlying buffer has been detached"):
4357+
getattr(wrapper, 'buffer')
4358+
42794359

42804360
class PyTextIOWrapperTest(TextIOWrapperTest):
42814361
io = pyio
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)