Skip to content

Commit ebf6fa2

Browse files
committed
gh-154997: Guard stale raw access after reentrant detach
1 parent 6a139d6 commit ebf6fa2

2 files changed

Lines changed: 296 additions & 9 deletions

File tree

Lib/test/test_io/test_bufferedio.py

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,75 @@ def test_bad_readinto_type(self):
623623
bufio.readline()
624624
self.assertIsInstance(cm.exception.__cause__, TypeError)
625625

626+
def test_reentrant_detach_during_read_all(self):
627+
# gh-154997: Reentrant detach() during read_all() should not crash.
628+
# Use a duck-typed raw stream so _bufferedreader_read_all()
629+
# dispatches through raw.read(). Return EOF to terminate the loop.
630+
class Duck:
631+
closed = False
632+
633+
def __init__(self):
634+
self.n = 0
635+
636+
def readable(self):
637+
return True
638+
639+
def writable(self):
640+
return False
641+
642+
def seekable(self):
643+
return False
644+
645+
def close(self):
646+
pass
647+
648+
def flush(self):
649+
pass
650+
651+
def read(self, *args):
652+
self.n += 1
653+
if self.n == 1:
654+
self.buf.detach()
655+
return b"abc" if self.n < 3 else b""
656+
657+
def readinto(self, b):
658+
data = self.read()
659+
b[0:len(data)] = data
660+
return len(data)
661+
662+
raw = Duck()
663+
buf = self.tp(raw)
664+
raw.buf = buf
665+
666+
with self.assertRaisesRegex(ValueError, "detached"):
667+
buf.read()
668+
669+
def test_reentrant_detach_during_raw_read(self):
670+
# gh-154997: Reentrant detach() during read() should not crash.
671+
# detach() fires from inside raw.readinto(), which
672+
# _bufferedreader_raw_read() calls directly.
673+
class Raw(io.RawIOBase):
674+
def __init__(self):
675+
super().__init__()
676+
self.fired = False
677+
678+
def readable(self):
679+
return True
680+
681+
def readinto(self, b):
682+
if not self.fired:
683+
self.fired = True
684+
self.buf.detach()
685+
b[0:1] = b"a"
686+
return 1
687+
688+
raw = Raw()
689+
buf = self.tp(raw, buffer_size=4)
690+
raw.buf = buf
691+
692+
with self.assertRaisesRegex(ValueError, "detached"):
693+
buf.read(64)
694+
626695
@unittest.skipUnless(sys.maxsize > 2**32, 'requires 64bit platform')
627696
@unittest.skipIf(check_sanitizer(thread=True),
628697
'ThreadSanitizer aborts on huge allocations (exit code 66).')
@@ -1002,6 +1071,155 @@ def closed(self):
10021071
self.assertRaisesRegex(ValueError, "test", bufio.flush)
10031072
self.assertRaisesRegex(ValueError, "test", bufio.close)
10041073

1074+
def test_reentrant_detach_during_close(self):
1075+
# gh-154997: Reentrant detach() during close() should not crash.
1076+
1077+
class B(self.tp):
1078+
armed = True
1079+
1080+
def flush(self):
1081+
if self.armed:
1082+
self.armed = False
1083+
super().detach()
1084+
1085+
buf = B(self.BytesIO())
1086+
with self.assertRaisesRegex(ValueError, "detached"):
1087+
buf.close()
1088+
1089+
def test_reentrant_detach_during_raw_write(self):
1090+
# gh-154997: Reentrant detach() during write() should not crash.
1091+
# Use a small buffer and partial writes so write() reaches
1092+
# _bufferedwriter_raw_write(). Override flush() to avoid
1093+
# re-entering the buffered lock during detach().
1094+
class B(self.tp):
1095+
def flush(self):
1096+
return None
1097+
1098+
class Raw(io.RawIOBase):
1099+
def __init__(self):
1100+
super().__init__()
1101+
self.fired = False
1102+
1103+
def writable(self):
1104+
return True
1105+
1106+
def write(self, b):
1107+
if not self.fired:
1108+
self.fired = True
1109+
self.buf.detach()
1110+
return 1 # partial write -> flush loop iterates again
1111+
1112+
raw = Raw()
1113+
buf = B(raw, buffer_size=4)
1114+
raw.buf = buf
1115+
1116+
with self.assertRaisesRegex(ValueError, "detached"):
1117+
buf.write(b"0123456789abcdef")
1118+
1119+
def test_reentrant_detach_during_truncate(self):
1120+
# gh-154997: Reentrant detach() during truncate() should not crash.
1121+
# Avoid the seek path and make flush() a no-op so detach()
1122+
# exercises the guarded truncate path without re-entering
1123+
# the buffered lock.
1124+
class B(self.tp):
1125+
def flush(self):
1126+
return None
1127+
1128+
class Raw(io.RawIOBase):
1129+
def __init__(self):
1130+
super().__init__()
1131+
self.fired = False
1132+
1133+
def readable(self):
1134+
return False
1135+
1136+
def writable(self):
1137+
return True
1138+
1139+
def seekable(self):
1140+
return True
1141+
1142+
def tell(self):
1143+
return 0
1144+
1145+
def seek(self, pos, whence=0):
1146+
return 0
1147+
1148+
def truncate(self, pos=None):
1149+
return 0
1150+
1151+
def write(self, b):
1152+
if not self.fired:
1153+
self.fired = True
1154+
self.buf.detach()
1155+
return len(b)
1156+
1157+
raw = Raw()
1158+
buf = B(raw, buffer_size=64)
1159+
raw.buf = buf
1160+
1161+
buf.write(b"012")
1162+
with self.assertRaisesRegex(ValueError, "detached"):
1163+
buf.truncate(1)
1164+
1165+
def test_reentrant_detach_during_raw_tell(self):
1166+
# gh-154997: After detach(), truncate() calls _buffered_raw_tell()
1167+
# to refresh the cached position. That ValueError is intentionally
1168+
# swallowed, so verify the guarded path by checking raw.tell() is
1169+
# not called again after detach().
1170+
class B(self.tp):
1171+
def flush(self):
1172+
return None
1173+
1174+
class Raw(io.RawIOBase):
1175+
def __init__(self):
1176+
super().__init__()
1177+
self.fired = False
1178+
self.tell_calls = 0
1179+
1180+
def readable(self):
1181+
return False
1182+
1183+
def writable(self):
1184+
return True
1185+
1186+
def seekable(self):
1187+
return True
1188+
1189+
def tell(self):
1190+
self.tell_calls += 1
1191+
return 0
1192+
1193+
def seek(self, pos, whence=0):
1194+
return 0
1195+
1196+
def write(self, b):
1197+
return len(b)
1198+
1199+
def truncate(self, pos=None):
1200+
if not self.fired:
1201+
self.fired = True
1202+
self.buf.detach()
1203+
return 0
1204+
1205+
raw = Raw()
1206+
buf = B(raw, buffer_size=64)
1207+
raw.buf = buf
1208+
1209+
# _buffered_init() calls _buffered_raw_tell() once at construction.
1210+
self.assertEqual(raw.tell_calls, 1)
1211+
1212+
buf.write(b"012")
1213+
# Must not crash; the swallowed ValueError means truncate() itself
1214+
# still reports success, unchanged from pre-detach behavior.
1215+
self.assertEqual(buf.truncate(1), 0)
1216+
1217+
# The critical assertion: raw_access_safe() short-circuited the
1218+
# post-truncate tell() call -- tell_calls stayed at 1, it did NOT
1219+
# increment to 2. Before the fix this dispatched through a NULL
1220+
# self->raw and crashed with SIGSEGV.
1221+
self.assertEqual(raw.tell_calls, 1)
1222+
10051223

10061224
class PyBufferedWriterTest(BufferedWriterTest, PyTestCase):
10071225
tp = pyio.BufferedWriter

0 commit comments

Comments
 (0)