Skip to content

Commit 1268235

Browse files
naschemealexkatsgpshead
authored
[3.15] gh-149816: Fix UAF in Modules/_pickle.c (GH-150024) (#153718)
(cherry picked from commit d095ceb) Co-authored-by: Alexey Katsman <alex2007508@gmail.com> Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
1 parent accfc34 commit 1268235

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
@@ -3191,7 +3191,7 @@ static int
31913191
batch_list_exact(PickleState *state, PicklerObject *self, PyObject *obj)
31923192
{
31933193
PyObject *item = NULL;
3194-
Py_ssize_t this_batch, total;
3194+
Py_ssize_t this_batch, total, list_size;
31953195

31963196
const char append_op = APPEND;
31973197
const char appends_op = APPENDS;
@@ -3200,14 +3200,18 @@ batch_list_exact(PickleState *state, PicklerObject *self, PyObject *obj)
32003200
assert(obj != NULL);
32013201
assert(self->proto > 0);
32023202
assert(PyList_CheckExact(obj));
3203-
assert(PyList_GET_SIZE(obj));
3203+
3204+
list_size = PyList_GET_SIZE(obj);
32043205

32053206
/* Write in batches of BATCHSIZE. */
32063207
total = 0;
32073208
do {
3208-
if (PyList_GET_SIZE(obj) - total == 1) {
3209-
item = PyList_GET_ITEM(obj, total);
3210-
Py_INCREF(item);
3209+
if (list_size - total == 1) {
3210+
item = PyList_GetItemRef(obj, total);
3211+
if (item == NULL) {
3212+
_PyErr_FormatNote("when serializing %T item %zd", obj, total);
3213+
return -1;
3214+
}
32113215
int err = save(state, self, item, 0);
32123216
Py_DECREF(item);
32133217
if (err < 0) {
@@ -3222,8 +3226,11 @@ batch_list_exact(PickleState *state, PicklerObject *self, PyObject *obj)
32223226
if (_Pickler_Write(self, &mark_op, 1) < 0)
32233227
return -1;
32243228
while (total < PyList_GET_SIZE(obj)) {
3225-
item = PyList_GET_ITEM(obj, total);
3226-
Py_INCREF(item);
3229+
item = PyList_GetItemRef(obj, total);
3230+
if (item == NULL) {
3231+
_PyErr_FormatNote("when serializing %T item %zd", obj, total);
3232+
return -1;
3233+
}
32273234
int err = save(state, self, item, 0);
32283235
Py_DECREF(item);
32293236
if (err < 0) {
@@ -3236,8 +3243,14 @@ batch_list_exact(PickleState *state, PicklerObject *self, PyObject *obj)
32363243
}
32373244
if (_Pickler_Write(self, &appends_op, 1) < 0)
32383245
return -1;
3246+
if (PyList_GET_SIZE(obj) != list_size) {
3247+
PyErr_Format(
3248+
PyExc_RuntimeError,
3249+
"list changed size during iteration");
3250+
return -1;
3251+
}
32393252

3240-
} while (total < PyList_GET_SIZE(obj));
3253+
} while (total < list_size);
32413254

32423255
return 0;
32433256
}

0 commit comments

Comments
 (0)