Commit62ec3e1

michaelpq

and

Maxim Orlov

committed

Fix possible double-release of spinlock in procsignal.c

9d9b9d4 has added spinlocks to protect the fields in ProcSignalflags, introducing a code path in ProcSignalInit() where a spinlockcould be released twice if the pss_pid field of a ProcSignalSlot isfound as already set. Multiple spinlock releases have no effect withmost spinlock implementations, but this could cause the code to run intoissues when the spinlock is acquired concurrently by a differentprocess.This sanity check on pss_pid generates a LOG that can be delayed untilafter the spinlock is released as, like older versions up to v17, thecode expects the initialization of the ProcSignalSlot to happen even ifpss_pid is found incorrect. The code is changed so as the old pss_pidis read while holding the slot's spinlock, with the LOG from the sanitycheck generated after releasing the spinlock, preventing the doublerelease.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

1 parent15df9d7 commit62ec3e1Copy full SHA for 62ec3e1

File tree

1 file changed

-7

lines changed

src/backend/storage/ipc
- procsignal.c

1 file changed

-7

lines changed

`‎src/backend/storage/ipc/procsignal.c`

Lines changed: 9 additions & 7 deletions

Original file line number	Diff line number	Diff line change
`@@ -167,21 +167,18 @@ ProcSignalInit(bool cancel_key_valid, int32 cancel_key)`
`167`	`167`	`{`
`168`	`168`	`ProcSignalSlot*slot;`
`169`	`169`	`uint64barrier_generation;`
	`170`	`+uint32old_pss_pid;`
`170`	`171`
`171`	`172`	`if (MyProcNumber<0)`
`172`	`173`	`elog(ERROR,"MyProcNumber not set");`
`173`	`174`	`if (MyProcNumber >=NumProcSignalSlots)`
`174`	`175`	`elog(ERROR,"unexpected MyProcNumber %d in ProcSignalInit (max %d)",MyProcNumber,NumProcSignalSlots);`
`175`	`176`	`slot=&ProcSignal->psh_slot[MyProcNumber];`
`176`	`177`
`177`		`-/* sanity check */`
`178`	`178`	`SpinLockAcquire(&slot->pss_mutex);`
`179`		`-if (pg_atomic_read_u32(&slot->pss_pid)!=0)`
`180`		`-{`
`181`		`-SpinLockRelease(&slot->pss_mutex);`
`182`		`-elog(LOG,"process %d taking over ProcSignal slot %d, but it's not empty",`
`183`		`-MyProcPid,MyProcNumber);`
`184`		`-}`
	`179`	`+`
	`180`	`+/* Value used for sanity check below */`
	`181`	`+old_pss_pid=pg_atomic_read_u32(&slot->pss_pid);`
`185`	`182`
`186`	`183`	`/* Clear out any leftover signal reasons */`
`187`	`184`	`MemSet(slot->pss_signalFlags,0,NUM_PROCSIGNALS*sizeof(sig_atomic_t));`
`@@ -208,6 +205,11 @@ ProcSignalInit(bool cancel_key_valid, int32 cancel_key)`
`208`	`205`
`209`	`206`	`SpinLockRelease(&slot->pss_mutex);`
`210`	`207`
	`208`	`+/* Spinlock is released, do the check */`
	`209`	`+if (old_pss_pid!=0)`
	`210`	`+elog(LOG,"process %d taking over ProcSignal slot %d, but it's not empty",`
	`211`	`+MyProcPid,MyProcNumber);`
	`212`	`+`
`211`	`213`	`/* Remember slot location for CheckProcSignal */`
`212`	`214`	`MyProcSignalSlot=slot;`
`213`	`215`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit62ec3e1

File tree

1 file changed

1 file changed

`‎src/backend/storage/ipc/procsignal.c`

0 commit comments