Skip to content

Commit d095ceb

Browse files
alexkatsgpshead
andauthored
gh-149816: Fix UAF in Modules/_pickle.c (GH-150024)
Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
1 parent 61f1221 commit d095ceb

3 files changed

Lines changed: 57 additions & 8 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: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a potential use after free condition in :func:`pickle.dumps` in free-threaded
2+
mode when serializing lists.

Modules/_pickle.c

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3179,7 +3179,7 @@ static int
31793179
batch_list_exact(PickleState *state, PicklerObject *self, PyObject *obj)
31803180
{
31813181
PyObject *item = NULL;
3182-
Py_ssize_t this_batch, total;
3182+
Py_ssize_t this_batch, total, list_size;
31833183

31843184
const char append_op = APPEND;
31853185
const char appends_op = APPENDS;
@@ -3188,14 +3188,18 @@ 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));
3191+
3192+
list_size = PyList_GET_SIZE(obj);
31923193

31933194
/* Write in batches of BATCHSIZE. */
31943195
total = 0;
31953196
do {
3196-
if (PyList_GET_SIZE(obj) - total == 1) {
3197-
item = PyList_GET_ITEM(obj, total);
3198-
Py_INCREF(item);
3197+
if (list_size - total == 1) {
3198+
item = PyList_GetItemRef(obj, total);
3199+
if (item == NULL) {
3200+
_PyErr_FormatNote("when serializing %T item %zd", obj, total);
3201+
return -1;
3202+
}
31993203
int err = save(state, self, item, 0);
32003204
Py_DECREF(item);
32013205
if (err < 0) {
@@ -3210,8 +3214,11 @@ batch_list_exact(PickleState *state, PicklerObject *self, PyObject *obj)
32103214
if (_Pickler_Write(self, &mark_op, 1) < 0)
32113215
return -1;
32123216
while (total < PyList_GET_SIZE(obj)) {
3213-
item = PyList_GET_ITEM(obj, total);
3214-
Py_INCREF(item);
3217+
item = PyList_GetItemRef(obj, total);
3218+
if (item == NULL) {
3219+
_PyErr_FormatNote("when serializing %T item %zd", obj, total);
3220+
return -1;
3221+
}
32153222
int err = save(state, self, item, 0);
32163223
Py_DECREF(item);
32173224
if (err < 0) {
@@ -3224,8 +3231,14 @@ batch_list_exact(PickleState *state, PicklerObject *self, PyObject *obj)
32243231
}
32253232
if (_Pickler_Write(self, &appends_op, 1) < 0)
32263233
return -1;
3234+
if (PyList_GET_SIZE(obj) != list_size) {
3235+
PyErr_Format(
3236+
PyExc_RuntimeError,
3237+
"list changed size during iteration");
3238+
return -1;
3239+
}
32273240

3228-
} while (total < PyList_GET_SIZE(obj));
3241+
} while (total < list_size);
32293242

32303243
return 0;
32313244
}

0 commit comments

Comments
 (0)