Skip to content

Commit ad3c5b7

Browse files
authored
gh-148820: Fix _PyRawMutex use-after-free on spurious semaphore wakeup (gh-148852)
_PyRawMutex_UnlockSlow CAS-removes the waiter from the list and then calls _PySemaphore_Wakeup, with no handshake. If _PySemaphore_Wait returns Py_PARK_INTR, the waiter can destroy its stack-allocated semaphore before the unlocker's Wakeup runs, causing a fatal error from ReleaseSemaphore / sem_post. Loop in _PyRawMutex_LockSlow until _PySemaphore_Wait returns Py_PARK_OK, which is only signalled when a matching Wakeup has been observed. Also include GetLastError() and the handle in the Windows fatal messages in _PySemaphore_Init, _PySemaphore_Wait, and _PySemaphore_Wakeup to make similar races easier to diagnose in the future.
1 parent 59b41c8 commit ad3c5b7

3 files changed

Lines changed: 23 additions & 5 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix a race in :c:type:`!_PyRawMutex` on the free-threaded build where a
2+
``Py_PARK_INTR`` return from ``_PySemaphore_Wait`` could let the waiter
3+
destroy its semaphore before the unlocking thread's
4+
``_PySemaphore_Wakeup`` completed, causing a fatal ``ReleaseSemaphore``
5+
error.

Python/lock.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,16 @@ _PyRawMutex_LockSlow(_PyRawMutex *m)
248248

249249
// Wait for us to be woken up. Note that we still have to lock the
250250
// mutex ourselves: it is NOT handed off to us.
251-
_PySemaphore_Wait(&waiter.sema, -1);
251+
//
252+
// Loop until we observe an actual wakeup. A return of Py_PARK_INTR
253+
// could otherwise let us exit _PySemaphore_Wait and destroy
254+
// `waiter.sema` while _PyRawMutex_UnlockSlow's matching
255+
// _PySemaphore_Wakeup is still pending, since the unlocker has
256+
// already CAS-removed us from the waiter list without any handshake.
257+
int res;
258+
do {
259+
res = _PySemaphore_Wait(&waiter.sema, -1);
260+
} while (res != Py_PARK_OK);
252261
}
253262

254263
_PySemaphore_Destroy(&waiter.sema);

Python/parking_lot.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ _PySemaphore_Init(_PySemaphore *sema)
6161
NULL // unnamed
6262
);
6363
if (!sema->platform_sem) {
64-
Py_FatalError("parking_lot: CreateSemaphore failed");
64+
_Py_FatalErrorFormat(__func__,
65+
"parking_lot: CreateSemaphore failed (error: %u)",
66+
GetLastError());
6567
}
6668
#elif defined(_Py_USE_SEMAPHORES)
6769
if (sem_init(&sema->platform_sem, /*pshared=*/0, /*value=*/0) < 0) {
@@ -141,8 +143,8 @@ _PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout)
141143
}
142144
else {
143145
_Py_FatalErrorFormat(__func__,
144-
"unexpected error from semaphore: %u (error: %u)",
145-
wait, GetLastError());
146+
"unexpected error from semaphore: %u (error: %u, handle: %p)",
147+
wait, GetLastError(), sema->platform_sem);
146148
}
147149
#elif defined(_Py_USE_SEMAPHORES)
148150
int err;
@@ -230,7 +232,9 @@ _PySemaphore_Wakeup(_PySemaphore *sema)
230232
{
231233
#if defined(MS_WINDOWS)
232234
if (!ReleaseSemaphore(sema->platform_sem, 1, NULL)) {
233-
Py_FatalError("parking_lot: ReleaseSemaphore failed");
235+
_Py_FatalErrorFormat(__func__,
236+
"parking_lot: ReleaseSemaphore failed (error: %u, handle: %p)",
237+
GetLastError(), sema->platform_sem);
234238
}
235239
#elif defined(_Py_USE_SEMAPHORES)
236240
int err = sem_post(&sema->platform_sem);

0 commit comments

Comments
 (0)