From 25692b4ac0f07aff68292b48feef2104f4f3ffa8 Mon Sep 17 00:00:00 2001 From: jg Date: Sun, 25 Oct 2026 13:58:59 +0000 Subject: [PATCH 1/3] Fix UAF in `visit_generation` Callback can free an object resulting in UAF if we have relied on the object for visiting the next item in the chain. Fix is to use `finalize_garbage` approach of walking the object gc generation list directly maintaing and moving it into a tempoary list before calling callback --- ...-10-25-12-00-00.gh-issue-160663.vIsDrp.rst | 2 + Modules/_testcapi/gc.c | 39 +++++++++++++++++++ Python/gc.c | 23 +++++++---- 3 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/C_API/2026-10-25-12-00-00.gh-issue-160663.vIsDrp.rst diff --git a/Misc/NEWS.d/next/C_API/2026-10-25-12-00-00.gh-issue-160663.vIsDrp.rst b/Misc/NEWS.d/next/C_API/2026-10-25-12-00-00.gh-issue-160663.vIsDrp.rst new file mode 100644 index 00000000000000..73ff2c23213c45 --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2026-10-25-12-00-00.gh-issue-160663.vIsDrp.rst @@ -0,0 +1,2 @@ +Fix a crash in :c:func:`PyUnstable_GC_VisitObjects` on the default build +when the callback releases the last reference to the object it is visiting. diff --git a/Modules/_testcapi/gc.c b/Modules/_testcapi/gc.c index dac7066565f7ff..73f9edd47905b0 100644 --- a/Modules/_testcapi/gc.c +++ b/Modules/_testcapi/gc.c @@ -271,6 +271,42 @@ test_gc_visit_objects_exit_early(PyObject *Py_UNUSED(self), Py_RETURN_NONE; } +#ifndef Py_GIL_DISABLED +static int +gc_visit_callback_drop_current(PyObject *obj, void *arg) +{ + struct gc_visit_state_basic *state = (struct gc_visit_state_basic *)arg; + if (obj == state->target) { + state->found = 1; + Py_CLEAR(state->target); + } + return 1; +} + +static PyObject * +test_gc_visit_objects_drop_current(PyObject *Py_UNUSED(self), + PyObject *Py_UNUSED(ignored)) +{ + struct gc_visit_state_basic state; + + state.target = PySet_New(NULL); + if (state.target == NULL) { + return NULL; + } + state.found = 0; + + PyUnstable_GC_VisitObjects(gc_visit_callback_drop_current, &state); + if (!state.found) { + Py_DECREF(state.target); + PyErr_SetString( + PyExc_AssertionError, + "test_gc_visit_objects_drop_current: Didn't find live set"); + return NULL; + } + Py_RETURN_NONE; +} +#endif + typedef struct { PyObject_HEAD } ObjExtraData; @@ -369,6 +405,9 @@ static PyMethodDef test_methods[] = { {"test_gc_visit_objects_basic", test_gc_visit_objects_basic, METH_NOARGS, NULL}, {"test_gc_visit_objects_frozen", test_gc_visit_objects_frozen, METH_NOARGS, NULL}, {"test_gc_visit_objects_exit_early", test_gc_visit_objects_exit_early, METH_NOARGS, NULL}, +#ifndef Py_GIL_DISABLED + {"test_gc_visit_objects_drop_current", test_gc_visit_objects_drop_current, METH_NOARGS, NULL}, +#endif {"without_gc", without_gc, METH_O, NULL}, {"with_tp_del", with_tp_del, METH_VARARGS, NULL}, {NULL} diff --git a/Python/gc.c b/Python/gc.c index 5dc79e20191ebc..8ef5d6867d9bdf 100644 --- a/Python/gc.c +++ b/Python/gc.c @@ -2164,18 +2164,25 @@ PyObject_GC_IsFinalized(PyObject *obj) static int visit_generation(gcvisitobjects_t callback, void *arg, struct gc_generation *gen) { - PyGC_Head *gc_list, *gc; - gc_list = &gen->head; - for (gc = GC_NEXT(gc_list); gc != gc_list; gc = GC_NEXT(gc)) { + PyGC_Head *gc_list = &gen->head; + PyGC_Head seen; + int res = 1; + + /* The callback may free or untrack the object it is given, so always + * take the first object and move it to `seen` before calling it. */ + gc_list_init(&seen); + while (res && !gc_list_is_empty(gc_list)) { + PyGC_Head *gc = GC_NEXT(gc_list); PyObject *op = FROM_GC(gc); + gc_list_move(gc, &seen); Py_INCREF(op); - int res = callback(op, arg); + res = callback(op, arg); Py_DECREF(op); - if (!res) { - return -1; - } } - return 0; + /* Put the visited objects back in front of the unvisited ones. */ + gc_list_merge(gc_list, &seen); + gc_list_merge(&seen, gc_list); + return res ? 0 : -1; } void From 335012156a9b29c1aa27e72829a83d4136c81ed9 Mon Sep 17 00:00:00 2001 From: jg Date: Sun, 25 Oct 2026 20:28:32 +0000 Subject: [PATCH 2/3] Revert added test. Failure case is crashing the whole test suite, also may not be reliable --- Modules/_testcapi/gc.c | 39 --------------------------------------- 1 file changed, 39 deletions(-) diff --git a/Modules/_testcapi/gc.c b/Modules/_testcapi/gc.c index 73f9edd47905b0..dac7066565f7ff 100644 --- a/Modules/_testcapi/gc.c +++ b/Modules/_testcapi/gc.c @@ -271,42 +271,6 @@ test_gc_visit_objects_exit_early(PyObject *Py_UNUSED(self), Py_RETURN_NONE; } -#ifndef Py_GIL_DISABLED -static int -gc_visit_callback_drop_current(PyObject *obj, void *arg) -{ - struct gc_visit_state_basic *state = (struct gc_visit_state_basic *)arg; - if (obj == state->target) { - state->found = 1; - Py_CLEAR(state->target); - } - return 1; -} - -static PyObject * -test_gc_visit_objects_drop_current(PyObject *Py_UNUSED(self), - PyObject *Py_UNUSED(ignored)) -{ - struct gc_visit_state_basic state; - - state.target = PySet_New(NULL); - if (state.target == NULL) { - return NULL; - } - state.found = 0; - - PyUnstable_GC_VisitObjects(gc_visit_callback_drop_current, &state); - if (!state.found) { - Py_DECREF(state.target); - PyErr_SetString( - PyExc_AssertionError, - "test_gc_visit_objects_drop_current: Didn't find live set"); - return NULL; - } - Py_RETURN_NONE; -} -#endif - typedef struct { PyObject_HEAD } ObjExtraData; @@ -405,9 +369,6 @@ static PyMethodDef test_methods[] = { {"test_gc_visit_objects_basic", test_gc_visit_objects_basic, METH_NOARGS, NULL}, {"test_gc_visit_objects_frozen", test_gc_visit_objects_frozen, METH_NOARGS, NULL}, {"test_gc_visit_objects_exit_early", test_gc_visit_objects_exit_early, METH_NOARGS, NULL}, -#ifndef Py_GIL_DISABLED - {"test_gc_visit_objects_drop_current", test_gc_visit_objects_drop_current, METH_NOARGS, NULL}, -#endif {"without_gc", without_gc, METH_O, NULL}, {"with_tp_del", with_tp_del, METH_VARARGS, NULL}, {NULL} From ffeb81f37529deefb1aa45f800bafb8b672f742a Mon Sep 17 00:00:00 2001 From: jg Date: Sun, 25 Oct 2026 21:27:05 +0000 Subject: [PATCH 3/3] Update comment --- Python/gc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Python/gc.c b/Python/gc.c index 8ef5d6867d9bdf..a2dd4e90e73c70 100644 --- a/Python/gc.c +++ b/Python/gc.c @@ -2168,8 +2168,10 @@ visit_generation(gcvisitobjects_t callback, void *arg, struct gc_generation *gen PyGC_Head seen; int res = 1; - /* The callback may free or untrack the object it is given, so always - * take the first object and move it to `seen` before calling it. */ + /* Iterate directly on the gc head to ensure the iteration is safe + * from a callback freeing its object (which can also cause + * a cascading free). Visited objects are moved to `seen` and + * merged back at the end. */ gc_list_init(&seen); while (res && !gc_list_is_empty(gc_list)) { PyGC_Head *gc = GC_NEXT(gc_list);