[PATCH] Fix SIGSEGV in GrantLockLocal when OOM leaves LOCALLOCK.lockOwners NULL
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253354psql -h localhost -U postgresBuilt from patchset v1 (message #1), August 11, 2026 at 01:15 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253354_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253354_1 && git checkout t253354_1Patchset v1 (message #1) is on t253354_1
Greetings,
An out-of-memory error during the first LockAcquire for a lock tag leaves a
LOCALLOCK that crashes the next acquire of the same tag. The initial
lockOwners allocation is done with maxLockOwners already set to 8 and
lockOwners still NULL; if that allocation throws, the entry survives in that
state. The next acquire takes the existing-entry path, where the only check
is numLockOwners >= maxLockOwners (0 >= 8, false), so it skips the
allocation and GrantLockLocal() dereferences the NULL pointer.
This is not a hypothetical state. Tom Lane hardened RemoveLocalLock()
against exactly it in ba51774d87 (2015, "per low-memory testing by Andreas
Seltenreich"): "RemoveLocalLock() must consider the possibility that
LockAcquireExtended() failed to palloc the initial space for a locallock's
lockOwners array." That fix covered the cleanup path; the re-acquire path
still assumes lockOwners is allocated. This closes that gap, the same way:
if lockOwners is NULL, allocate it before use.
One subtlety worth stating: a plain top-level OOM here aborts the
transaction, and RemoveLocalLock() then discards the partial entry, so no
re-acquire hits it. The crash needs the OOM caught without a full lock
release, e.g. a PL/pgSQL EXCEPTION handler, where the subtransaction abort
does not run LockReleaseAll and does not touch an entry that never got a
resource owner. The entry then survives to the next acquire.
The fix is one branch in lock.c. I confirmed the crash and the fix by
forcing the allocation to fail for advisory locks; the regression suite
passes.
The original diagnosis is Mark Dilger's; I reproduced it on master and
prepared it for submission.
--
Bryan Green
EDB: https://www.enterprisedb.com
On Sat, Aug 08, 2026 at 10:26:40PM -0500, Bryan Green wrote:
An out-of-memory error during the first LockAcquire for a lock tag leaves a
LOCALLOCK that crashes the next acquire of the same tag. The initial
lockOwners allocation is done with maxLockOwners already set to 8 and
lockOwners still NULL; if that allocation throws, the entry survives in that
state. The next acquire takes the existing-entry path, where the only check
is numLockOwners >= maxLockOwners (0 >= 8, false), so it skips the
allocation and GrantLockLocal() dereferences the NULL pointer.
else
{
/* Make sure there will be room to remember the lock */
- if (locallock->numLockOwners >= locallock->maxLockOwners)
+ if (locallock->lockOwners == NULL)
+ {
+ /* A prior acquisition left the array unallocated after OOM. */
+ locallock->maxLockOwners = 8;
+ locallock->lockOwners = (LOCALLOCKOWNER *)
+ MemoryContextAlloc(TopMemoryContext,
+ locallock->maxLockOwners * sizeof(LOCALLOCKOWNER));
+ }
+ else if (locallock->numLockOwners >= locallock->maxLockOwners)
{
int newsize = locallock->maxLockOwners * 2;I can buy that. The allocation failing would leave lockOwners NULL
while there is an entry in LockMethodLocalHash that we may try to
re-access later. Just doing an extra allocation if we find out that
lockOwners is not set seems like a solution good enough.
Thanks for the report. Will look again at it later.
--
Michael