Skip to content

Commit 4f17349

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

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
@@ -614,6 +614,32 @@ def test_join(self):
614614
with self.assertRaises(TypeError):
615615
dot_join([memoryview(b"ab"), "cd", b"ef"])
616616

617+
def test_join_concurrent_buffer_mutation(self):
618+
# __buffer__() can release the GIL, letting another thread concurrently
619+
# mutate the joined sequence (simulated here by mutating in __buffer__).
620+
# See: https://github.com/python/cpython/issues/151295
621+
def make_seq(mutate):
622+
# Item is only referenced from the list slot, so mutate() frees it.
623+
class Item:
624+
def __buffer__(self, flags):
625+
mutate(seq)
626+
return memoryview(b'x')
627+
seq = [b'a', Item(), b'c']
628+
return seq
629+
630+
for sep in (self.type2test(b''), self.type2test(b'::')):
631+
with self.subTest(sep=sep):
632+
# Changing the list length is reported as a RuntimeError.
633+
seq = make_seq(lambda seq: seq.clear())
634+
self.assertRaises(RuntimeError, sep.join, seq)
635+
636+
# The list length is unchanged, so the size-change recheck
637+
# cannot fire: only keeping the item alive avoids the crash.
638+
def replace(seq):
639+
seq[1] = b'z'
640+
seq = make_seq(replace)
641+
self.assertEqual(sep.join(seq), sep.join([b'a', b'x', b'c']))
642+
617643
def test_count(self):
618644
b = self.type2test(b'mississippi')
619645
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)