Skip to content

Commit f2de1e6

Browse files
authored
gh-134144: Fix use-after-free in zapthreads() (#134145)
1 parent 0a160bf commit f2de1e6

File tree

4 files changed

+41
-6
lines changed

4 files changed

+41
-6
lines changed

Lib/test/test_interpreters/test_api.py

+8
Original file line numberDiff line numberDiff line change
@@ -1452,6 +1452,14 @@ def test_destroy(self):
14521452
self.assertFalse(
14531453
self.interp_exists(interpid))
14541454

1455+
with self.subTest('basic C-API'):
1456+
interpid = _testinternalcapi.create_interpreter()
1457+
self.assertTrue(
1458+
self.interp_exists(interpid))
1459+
_testinternalcapi.destroy_interpreter(interpid, basic=True)
1460+
self.assertFalse(
1461+
self.interp_exists(interpid))
1462+
14551463
def test_get_config(self):
14561464
# This test overlaps with
14571465
# test.test_capi.test_misc.InterpreterConfigTests.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix crash when calling :c:func:`Py_EndInterpreter` with a :term:`thread state` that isn't the initial thread for the interpreter.

Modules/_testinternalcapi.c

+25-4
Original file line numberDiff line numberDiff line change
@@ -1690,11 +1690,12 @@ create_interpreter(PyObject *self, PyObject *args, PyObject *kwargs)
16901690
static PyObject *
16911691
destroy_interpreter(PyObject *self, PyObject *args, PyObject *kwargs)
16921692
{
1693-
static char *kwlist[] = {"id", NULL};
1693+
static char *kwlist[] = {"id", "basic", NULL};
16941694
PyObject *idobj = NULL;
1695+
int basic = 0;
16951696
if (!PyArg_ParseTupleAndKeywords(args, kwargs,
1696-
"O:destroy_interpreter", kwlist,
1697-
&idobj))
1697+
"O|p:destroy_interpreter", kwlist,
1698+
&idobj, &basic))
16981699
{
16991700
return NULL;
17001701
}
@@ -1704,7 +1705,27 @@ destroy_interpreter(PyObject *self, PyObject *args, PyObject *kwargs)
17041705
return NULL;
17051706
}
17061707

1707-
_PyXI_EndInterpreter(interp, NULL, NULL);
1708+
if (basic)
1709+
{
1710+
// Test the basic Py_EndInterpreter with weird out of order thread states
1711+
PyThreadState *t1, *t2;
1712+
PyThreadState *prev;
1713+
t1 = interp->threads.head;
1714+
if (t1 == NULL) {
1715+
t1 = PyThreadState_New(interp);
1716+
}
1717+
t2 = PyThreadState_New(interp);
1718+
prev = PyThreadState_Swap(t2);
1719+
PyThreadState_Clear(t1);
1720+
PyThreadState_Delete(t1);
1721+
Py_EndInterpreter(t2);
1722+
PyThreadState_Swap(prev);
1723+
}
1724+
else
1725+
{
1726+
// use the cross interpreter _PyXI_EndInterpreter normally
1727+
_PyXI_EndInterpreter(interp, NULL, NULL);
1728+
}
17081729
Py_RETURN_NONE;
17091730
}
17101731

Python/pystate.c

+7-2
Original file line numberDiff line numberDiff line change
@@ -1908,9 +1908,14 @@ tstate_delete_common(PyThreadState *tstate, int release_gil)
19081908
static void
19091909
zapthreads(PyInterpreterState *interp)
19101910
{
1911+
PyThreadState *tstate;
19111912
/* No need to lock the mutex here because this should only happen
1912-
when the threads are all really dead (XXX famous last words). */
1913-
_Py_FOR_EACH_TSTATE_UNLOCKED(interp, tstate) {
1913+
when the threads are all really dead (XXX famous last words).
1914+
1915+
Cannot use _Py_FOR_EACH_TSTATE_UNLOCKED because we are freeing
1916+
the thread states here.
1917+
*/
1918+
while ((tstate = interp->threads.head) != NULL) {
19141919
tstate_verify_not_active(tstate);
19151920
tstate_delete_common(tstate, 0);
19161921
free_threadstate((_PyThreadStateImpl *)tstate);

0 commit comments

Comments
 (0)