From 5c85181feaf40ca9bfbcf33d01800d68363e3589 Mon Sep 17 00:00:00 2001 From: ZeroIntensity Date: Thu, 8 Aug 2024 14:12:45 -0400 Subject: [PATCH 1/3] Store an extra strong reference to prevent a double free in FutureIter_dealloc() --- Lib/test/test_asyncio/test_futures.py | 8 ++++++++ Modules/_asynciomodule.c | 10 ++++++++++ 2 files changed, 18 insertions(+) diff --git a/Lib/test/test_asyncio/test_futures.py b/Lib/test/test_asyncio/test_futures.py index 2184b2091f84eed..cfcd43dbe725aef 100644 --- a/Lib/test/test_asyncio/test_futures.py +++ b/Lib/test/test_asyncio/test_futures.py @@ -641,6 +641,14 @@ def test_future_del_segfault(self): with self.assertRaises(AttributeError): del fut._log_traceback + def test_future_iter_get_referents_segfault(self): + # See https://github.com/python/cpython/issues/122695 + import _asyncio + it = iter(self._new_future(loop=self.loop)) + del it + evil = gc.get_referents(_asyncio) + + @unittest.skipUnless(hasattr(futures, '_CFuture'), 'requires the C _asyncio module') diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 6b969edca298046..10e1ca285a2556c 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -1610,8 +1610,14 @@ FutureIter_dealloc(futureiterobject *it) if (state && state->fi_freelist_len < FI_FREELIST_MAXLEN) { state->fi_freelist_len++; + assert(state->fi_freelist != it); it->future = (FutureObj*) state->fi_freelist; state->fi_freelist = it; + + // GH-122695: Since the freelist is traversed, gc.get_referents() + // can return this object, causing a double free when the object + // is deallocated for a second time. + Py_INCREF(it); } else { PyObject_GC_Del(it); @@ -3573,6 +3579,10 @@ module_free_freelists(asyncio_state *state) current = next; next = (PyObject*) ((futureiterobject*) current)->future; + + // GH-122695: We need to own a reference to this to prevent + // a double free. + assert(Py_REFCNT(current) == 1); PyObject_GC_Del(current); } assert(state->fi_freelist_len == 0); From 45c086e0849a2a9f01d763cc7353e9b55b29d98c Mon Sep 17 00:00:00 2001 From: ZeroIntensity Date: Thu, 8 Aug 2024 14:16:48 -0400 Subject: [PATCH 2/3] Add NEWS entry. --- .../next/Library/2024-08-08-14-16-41.gh-issue-122695.f7pwBv.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2024-08-08-14-16-41.gh-issue-122695.f7pwBv.rst diff --git a/Misc/NEWS.d/next/Library/2024-08-08-14-16-41.gh-issue-122695.f7pwBv.rst b/Misc/NEWS.d/next/Library/2024-08-08-14-16-41.gh-issue-122695.f7pwBv.rst new file mode 100644 index 000000000000000..cc6bc38f419462c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-08-08-14-16-41.gh-issue-122695.f7pwBv.rst @@ -0,0 +1,2 @@ +Fixed double-free when using :func:`gc.get_referents` with a freed +:class:`asyncio.Future` iterator. From 64fb5eb83e09895f5e4f3a37bb55a1352d557b88 Mon Sep 17 00:00:00 2001 From: ZeroIntensity Date: Thu, 8 Aug 2024 14:30:11 -0400 Subject: [PATCH 3/3] Remove traversal of the freelist in module_traverse() --- Modules/_asynciomodule.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 10e1ca285a2556c..cf6257834cdeb33 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -1610,14 +1610,8 @@ FutureIter_dealloc(futureiterobject *it) if (state && state->fi_freelist_len < FI_FREELIST_MAXLEN) { state->fi_freelist_len++; - assert(state->fi_freelist != it); it->future = (FutureObj*) state->fi_freelist; state->fi_freelist = it; - - // GH-122695: Since the freelist is traversed, gc.get_referents() - // can return this object, causing a double free when the object - // is deallocated for a second time. - Py_INCREF(it); } else { PyObject_GC_Del(it); @@ -3579,10 +3573,6 @@ module_free_freelists(asyncio_state *state) current = next; next = (PyObject*) ((futureiterobject*) current)->future; - - // GH-122695: We need to own a reference to this to prevent - // a double free. - assert(Py_REFCNT(current) == 1); PyObject_GC_Del(current); } assert(state->fi_freelist_len == 0); @@ -3617,13 +3607,8 @@ module_traverse(PyObject *mod, visitproc visit, void *arg) Py_VISIT(state->context_kwname); - // Visit freelist. - PyObject *next = (PyObject*) state->fi_freelist; - while (next != NULL) { - PyObject *current = next; - Py_VISIT(current); - next = (PyObject*) ((futureiterobject*) current)->future; - } + // GH-122695: Do not traverse the freelist here, as that can cause problems + // with gc.get_referents() return 0; }