Skip to content

Commit b3fbc16

Browse files
committed
gh-149816: Fix UAF in Modules/_pickle.c
Get a strong reference atomically for list item instead of 2 operations.
1 parent 5775aa8 commit b3fbc16

3 files changed

Lines changed: 45 additions & 5 deletions

File tree

Lib/test/test_free_threading/test_pickle.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,39 @@ def mutator():
4040
with threading_helper.start_threads(threads):
4141
pass
4242

43+
def test_pickle_dumps_with_concurrent_list_mutations(self):
44+
# gh-149816: Pickling a list while another thread mutates it
45+
# used to be a UAF in free-threaded mode. batch_list_exact()
46+
# used PyList_GET_ITEM (borrowed) followed by Py_INCREF, and a
47+
# concurrent replace/pop could free the item between those two
48+
# operations.
49+
shared = [list(range(20)) for _ in range(50)]
50+
51+
def dumper():
52+
for _ in range(1000):
53+
try:
54+
pickle.dumps(shared)
55+
except (RuntimeError, IndexError):
56+
pass
57+
58+
def mutator():
59+
for i in range(1000):
60+
idx = i % 50
61+
shared[idx] = list(range(i % 20))
62+
if i % 10 == 0:
63+
try:
64+
shared.pop()
65+
except IndexError:
66+
pass
67+
shared.append([i])
68+
69+
threads = []
70+
for _ in range(10):
71+
threads.append(threading.Thread(target=dumper))
72+
threads.append(threading.Thread(target=mutator))
73+
74+
with threading_helper.start_threads(threads):
75+
pass
76+
4377
if __name__ == "__main__":
4478
unittest.main()
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix a race condition in ``pickle.dumps`` method in free-threaded mode.

Modules/_pickle.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3188,14 +3188,16 @@ batch_list_exact(PickleState *state, PicklerObject *self, PyObject *obj)
31883188
assert(obj != NULL);
31893189
assert(self->proto > 0);
31903190
assert(PyList_CheckExact(obj));
3191-
assert(PyList_GET_SIZE(obj));
31923191

31933192
/* Write in batches of BATCHSIZE. */
31943193
total = 0;
31953194
do {
31963195
if (PyList_GET_SIZE(obj) - total == 1) {
3197-
item = PyList_GET_ITEM(obj, total);
3198-
Py_INCREF(item);
3196+
item = PyList_GetItemRef(obj, total);
3197+
if (item == NULL) {
3198+
_PyErr_FormatNote("when serializing %T item %zd", obj, total);
3199+
return -1;
3200+
}
31993201
int err = save(state, self, item, 0);
32003202
Py_DECREF(item);
32013203
if (err < 0) {
@@ -3210,8 +3212,11 @@ batch_list_exact(PickleState *state, PicklerObject *self, PyObject *obj)
32103212
if (_Pickler_Write(self, &mark_op, 1) < 0)
32113213
return -1;
32123214
while (total < PyList_GET_SIZE(obj)) {
3213-
item = PyList_GET_ITEM(obj, total);
3214-
Py_INCREF(item);
3215+
item = PyList_GetItemRef(obj, total);
3216+
if (item == NULL) {
3217+
_PyErr_FormatNote("when serializing %T item %zd", obj, total);
3218+
return -1;
3219+
}
32153220
int err = save(state, self, item, 0);
32163221
Py_DECREF(item);
32173222
if (err < 0) {

0 commit comments

Comments
 (0)