GeezerVison

All commit authors signed the Contributor License Agreement.

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

We need a test as well, but I'm more worried about the use-case as given in the issue. zapthreads will arbitrarily destroy a thread state (one created by PyGILState_Ensure in the repro), which is a recipe for crashes if the thread is still running.

We need a test as well, but I'm more worried about the use-case as given in the issue. zapthreads will arbitrarily destroy a thread state (one created by PyGILState_Ensure in the repro), which is a recipe for crashes if the thread is still running.

Sorry, the issue description isn't great. The thread wasn't alive, but Py_EndInterpreter takes a PyThreadState* as the argument (not a PyInterpreterState*), so there must always been one on the interpreter. But the fact is this code ALWAYS does a use-after-free (even if there was only ever one thread state). Here is some pseudo code of what is going on:

for (PyThreadState *tstate = interp->threads.head; 
       tstate != NULL; 
       tstate = tstate->next)
{
   free(tstate);
}

Right, I've noticed this before. In most cases, we get lucky because the initial thread is statically allocated on the interpreter state, so it doesn't actually cause UAF. It becomes an issue with something like PyGILState_Ensure because we have more than one thread state, but the problem is that's unsupported anyway.

It becomes an issue with something like PyGILState_Ensure because we have more than one thread state, but the problem is that's unsupported anyway.

It happens without the Ensure. I've updated the linked issue with a complete program (without an Ensure). So are you saying it's not supported to use a different PyThreadState* to end the interpreter than the one returned from its creation?

So are you saying it's not supported to use a different PyThreadState* to end the interpreter than the one returned from its creation?

Yes, and you can't have any additional threads when you call Py_EndInterpreter.

I'm fine with this fix, but my point is that it's not enough. Let's add a test and then I'll approve.

Yeah, a test would be nice.

Thanks @b-pass for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

GH-134182 is a backport of this pull request to the 3.14 branch.