From 16628e86aed6fa3a9c7a95101130b846c0f0c9ef Mon Sep 17 00:00:00 2001 From: b-pass Date: Sat, 17 May 2025 10:55:14 -0400 Subject: [PATCH 1/5] Fix use-after-free in zapthreads Can't use the _Py_FOR_EACH_TSTATE_UNLOCKED macro because it will reference the just-deleted tstate in order to get the next value. --- Python/pystate.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index 1ac134400856d4..a6e42c636e01d3 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1908,9 +1908,14 @@ tstate_delete_common(PyThreadState *tstate, int release_gil) static void zapthreads(PyInterpreterState *interp) { + PyThreadState *tstate; /* No need to lock the mutex here because this should only happen - when the threads are all really dead (XXX famous last words). */ - _Py_FOR_EACH_TSTATE_UNLOCKED(interp, tstate) { + when the threads are all really dead (XXX famous last words). + + Cannot use _Py_FOR_EACH_TSTATE_UNLOCKED because we are freeing + the thread states here. + */ + while ((tstate = interp->threads.head) != NULL) { tstate_verify_not_active(tstate); tstate_delete_common(tstate, 0); free_threadstate((_PyThreadStateImpl *)tstate); From 1c8531710e86552e0505033642cc2ef81d3137f7 Mon Sep 17 00:00:00 2001 From: b-pass Date: Sat, 17 May 2025 11:04:35 -0400 Subject: [PATCH 2/5] Fix trailing whitespace --- Python/pystate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index a6e42c636e01d3..14ae2748b0bc99 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1910,8 +1910,8 @@ zapthreads(PyInterpreterState *interp) { PyThreadState *tstate; /* No need to lock the mutex here because this should only happen - when the threads are all really dead (XXX famous last words). - + when the threads are all really dead (XXX famous last words). + Cannot use _Py_FOR_EACH_TSTATE_UNLOCKED because we are freeing the thread states here. */ From 89082fa14a973343ffdf78b0cb117f910cd575fa Mon Sep 17 00:00:00 2001 From: b-pass Date: Sat, 17 May 2025 14:41:42 -0400 Subject: [PATCH 3/5] Add NEWS blurb --- .../next/C_API/2025-05-17-14-41-21.gh-issue-134144.xVpZik.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/C_API/2025-05-17-14-41-21.gh-issue-134144.xVpZik.rst diff --git a/Misc/NEWS.d/next/C_API/2025-05-17-14-41-21.gh-issue-134144.xVpZik.rst b/Misc/NEWS.d/next/C_API/2025-05-17-14-41-21.gh-issue-134144.xVpZik.rst new file mode 100644 index 00000000000000..f85f42bcc7cc6d --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2025-05-17-14-41-21.gh-issue-134144.xVpZik.rst @@ -0,0 +1 @@ +Fix use-after-free during ``Py_EndInterpreter``. From 985eeda660232edb30fbee118b01cb327c12c5f8 Mon Sep 17 00:00:00 2001 From: b-pass Date: Sat, 17 May 2025 17:47:02 -0400 Subject: [PATCH 4/5] Update Misc/NEWS.d/next/C_API/2025-05-17-14-41-21.gh-issue-134144.xVpZik.rst Co-authored-by: Peter Bierma --- .../next/C_API/2025-05-17-14-41-21.gh-issue-134144.xVpZik.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/C_API/2025-05-17-14-41-21.gh-issue-134144.xVpZik.rst b/Misc/NEWS.d/next/C_API/2025-05-17-14-41-21.gh-issue-134144.xVpZik.rst index f85f42bcc7cc6d..11c7bd59a4dd1d 100644 --- a/Misc/NEWS.d/next/C_API/2025-05-17-14-41-21.gh-issue-134144.xVpZik.rst +++ b/Misc/NEWS.d/next/C_API/2025-05-17-14-41-21.gh-issue-134144.xVpZik.rst @@ -1 +1 @@ -Fix use-after-free during ``Py_EndInterpreter``. +Fix crash when calling :c:func:`Py_EndInterpreter` with a :term:`thread state` that isn't the initial thread for the interpreter. From 6db6f3d1b638d1e3bef96fee149ea42ce469be6a Mon Sep 17 00:00:00 2001 From: b-pass Date: Sat, 17 May 2025 20:44:47 -0400 Subject: [PATCH 5/5] Add a test which ends an interpreter with a different PyThreadState than normal --- Lib/test/test_interpreters/test_api.py | 8 +++++++ Modules/_testinternalcapi.c | 29 ++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_interpreters/test_api.py b/Lib/test/test_interpreters/test_api.py index 66c7afce88f0d8..1e2d572b1cbb81 100644 --- a/Lib/test/test_interpreters/test_api.py +++ b/Lib/test/test_interpreters/test_api.py @@ -1452,6 +1452,14 @@ def test_destroy(self): self.assertFalse( self.interp_exists(interpid)) + with self.subTest('basic C-API'): + interpid = _testinternalcapi.create_interpreter() + self.assertTrue( + self.interp_exists(interpid)) + _testinternalcapi.destroy_interpreter(interpid, basic=True) + self.assertFalse( + self.interp_exists(interpid)) + def test_get_config(self): # This test overlaps with # test.test_capi.test_misc.InterpreterConfigTests. diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 92f744c5a5fc70..76bd76cc6b2490 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -1690,11 +1690,12 @@ create_interpreter(PyObject *self, PyObject *args, PyObject *kwargs) static PyObject * destroy_interpreter(PyObject *self, PyObject *args, PyObject *kwargs) { - static char *kwlist[] = {"id", NULL}; + static char *kwlist[] = {"id", "basic", NULL}; PyObject *idobj = NULL; + int basic = 0; if (!PyArg_ParseTupleAndKeywords(args, kwargs, - "O:destroy_interpreter", kwlist, - &idobj)) + "O|p:destroy_interpreter", kwlist, + &idobj, &basic)) { return NULL; } @@ -1704,7 +1705,27 @@ destroy_interpreter(PyObject *self, PyObject *args, PyObject *kwargs) return NULL; } - _PyXI_EndInterpreter(interp, NULL, NULL); + if (basic) + { + // Test the basic Py_EndInterpreter with weird out of order thread states + PyThreadState *t1, *t2; + PyThreadState *prev; + t1 = interp->threads.head; + if (t1 == NULL) { + t1 = PyThreadState_New(interp); + } + t2 = PyThreadState_New(interp); + prev = PyThreadState_Swap(t2); + PyThreadState_Clear(t1); + PyThreadState_Delete(t1); + Py_EndInterpreter(t2); + PyThreadState_Swap(prev); + } + else + { + // use the cross interpreter _PyXI_EndInterpreter normally + _PyXI_EndInterpreter(interp, NULL, NULL); + } Py_RETURN_NONE; }