Skip to content

Commit 5c17ae6

Browse files
[3.13] gh-151295: Fix use-after-free in bytes.join()/bytearray.join() via re-entrant __buffer__ (GH-151296) (GH-151306)
(cherry picked from commit 84a322a) Co-authored-by: tonghuaroot (童话) <tonghuaroot@gmail.com>
1 parent d5468ba commit 5c17ae6

3 files changed

Lines changed: 35 additions & 0 deletions

File tree

Lib/test/test_bytes.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,32 @@ def test_join(self):
580580
with self.assertRaises(TypeError):
581581
dot_join([memoryview(b"ab"), "cd", b"ef"])
582582

583+
def test_join_concurrent_buffer_mutation(self):
584+
# __buffer__() can release the GIL, letting another thread concurrently
585+
# mutate the joined sequence (simulated here by mutating in __buffer__).
586+
# See: https://github.com/python/cpython/issues/151295
587+
def make_seq(mutate):
588+
# Item is only referenced from the list slot, so mutate() frees it.
589+
class Item:
590+
def __buffer__(self, flags):
591+
mutate(seq)
592+
return memoryview(b'x')
593+
seq = [b'a', Item(), b'c']
594+
return seq
595+
596+
for sep in (self.type2test(b''), self.type2test(b'::')):
597+
with self.subTest(sep=sep):
598+
# Changing the list length is reported as a RuntimeError.
599+
seq = make_seq(lambda seq: seq.clear())
600+
self.assertRaises(RuntimeError, sep.join, seq)
601+
602+
# The list length is unchanged, so the size-change recheck
603+
# cannot fire: only keeping the item alive avoids the crash.
604+
def replace(seq):
605+
seq[1] = b'z'
606+
seq = make_seq(replace)
607+
self.assertEqual(sep.join(seq), sep.join([b'a', b'x', b'c']))
608+
583609
def test_count(self):
584610
b = self.type2test(b'mississippi')
585611
i = 105
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fixed a crash (use-after-free) in :meth:`bytes.join` and
2+
:meth:`bytearray.join` that could occur if an item's
3+
:meth:`~object.__buffer__` concurrently mutates the sequence being joined.
4+
The mutation is now reported as a :exc:`RuntimeError` instead.

Objects/stringlib/join.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,18 @@ STRINGLIB(bytes_join)(PyObject *sep, PyObject *iterable)
6868
buffers[i].len = PyBytes_GET_SIZE(item);
6969
}
7070
else {
71+
/* item is only borrowed; its __buffer__() may run Python that
72+
drops the sequence's last reference to it. */
73+
Py_INCREF(item);
7174
if (PyObject_GetBuffer(item, &buffers[i], PyBUF_SIMPLE) != 0) {
75+
Py_DECREF(item);
7276
PyErr_Format(PyExc_TypeError,
7377
"sequence item %zd: expected a bytes-like object, "
7478
"%.80s found",
7579
i, Py_TYPE(item)->tp_name);
7680
goto error;
7781
}
82+
Py_DECREF(item);
7883
/* If the backing objects are mutable, then dropping the GIL
7984
* opens up race conditions where another thread tries to modify
8085
* the object which we hold a buffer on it. Such code has data

0 commit comments

Comments
 (0)