1berry/postgres
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix possible double-release of spinlock in procsignal.c

Browse Source 
9d9b9d46f3c5 has added spinlocks to protect the fields in ProcSignal
flags, introducing a code path in ProcSignalInit() where a spinlock
could be released twice if the pss_pid field of a ProcSignalSlot is
found as already set.  Multiple spinlock releases have no effect with
most spinlock implementations, but this could cause the code to run into
issues when the spinlock is acquired concurrently by a different
process.

This sanity check on pss_pid generates a LOG that can be delayed until
after the spinlock is released as, like older versions up to v17, the
code expects the initialization of the ProcSignalSlot to happen even if
pss_pid is found incorrect.  The code is changed so as the old pss_pid
is read while holding the slot's spinlock, with the LOG from the sanity
check generated after releasing the spinlock, preventing the double
release.

Author: Maksim Melnikov <m.melnikov@postgrespro.ru>
Co-authored-by: Maxim Orlov <orlovmg@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/dca47527-2d8b-4e3b-b5a0-e2deb73371a4@postgrespro.ru
This commit is contained in:
Michael Paquier 2025-02-27 09:43:06 +09:00
parent 15df9d7b51
commit 62ec3e1f67
1 changed files with 9 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/backend/storage/ipc/procsignal.c
View File

@ -167,6 +167,7 @@ ProcSignalInit(bool cancel_key_valid, int32 cancel_key)
{ {
ProcSignalSlot *slot; ProcSignalSlot *slot;
uint64 barrier_generation; uint64 barrier_generation;
uint32 old_pss_pid;
if (MyProcNumber < 0) if (MyProcNumber < 0)
elog(ERROR, "MyProcNumber not set"); elog(ERROR, "MyProcNumber not set");
@ -174,14 +175,10 @@ ProcSignalInit(bool cancel_key_valid, int32 cancel_key)
elog(ERROR, "unexpected MyProcNumber %d in ProcSignalInit (max %d)", MyProcNumber, NumProcSignalSlots); elog(ERROR, "unexpected MyProcNumber %d in ProcSignalInit (max %d)", MyProcNumber, NumProcSignalSlots);
slot = &ProcSignal->psh_slot[MyProcNumber]; slot = &ProcSignal->psh_slot[MyProcNumber];
/* sanity check */
SpinLockAcquire(&slot->pss_mutex); SpinLockAcquire(&slot->pss_mutex);
if (pg_atomic_read_u32(&slot->pss_pid) != 0)
{ /* Value used for sanity check below */
SpinLockRelease(&slot->pss_mutex); old_pss_pid = pg_atomic_read_u32(&slot->pss_pid);
elog(LOG, "process %d taking over ProcSignal slot %d, but it's not empty",
MyProcPid, MyProcNumber);
}
/* Clear out any leftover signal reasons */ /* Clear out any leftover signal reasons */
MemSet(slot->pss_signalFlags, 0, NUM_PROCSIGNALS * sizeof(sig_atomic_t)); MemSet(slot->pss_signalFlags, 0, NUM_PROCSIGNALS * sizeof(sig_atomic_t));
@ -208,6 +205,11 @@ ProcSignalInit(bool cancel_key_valid, int32 cancel_key)
SpinLockRelease(&slot->pss_mutex); SpinLockRelease(&slot->pss_mutex);
/* Spinlock is released, do the check */
if (old_pss_pid != 0)
elog(LOG, "process %d taking over ProcSignal slot %d, but it's not empty",
MyProcPid, MyProcNumber);
/* Remember slot location for CheckProcSignal */ /* Remember slot location for CheckProcSignal */
MyProcSignalSlot = slot; MyProcSignalSlot = slot;