Fix mislabeling of PROC_QUEUE->links as PGPROC, fixing UBSan on 32bit

ProcSleep() used a PGPROC* variable to point to PROC_QUEUE->links.next,
because that does "the right thing" with SHMQueueInsertBefore(). While that
largely works, it's certainly not correct and unnecessary - we can just use
SHM_QUEUE* to point to the insertion point.

Noticed when testing a 32bit of postgres with undefined behavior
sanitizer. UBSan noticed that sometimes the supposed PGPROC wasn't
sufficiently aligned (required since 46d6e5f5679, ensured indirectly, via
ShmemAllocRaw() guaranteeing cacheline alignment).

For now fix this by using a SHM_QUEUE* for the insertion point. Subsequently
we should replace all the use of PROC_QUEUE and SHM_QUEUE with ilist.h, but
that's a larger change that we don't want to backpatch.

Backpatch to all supported versions - it's useful to be able to run postgres
under UBSan.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20221117014230.op5kmgypdv2dtqsf@awork3.anarazel.de
Backpatch: 11-

diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 13fa07b0ff7a283afcc60dd6b639cf8addb61110..0aa304c835c6a30978f91823ebbf939809b51050 100644 (file)
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -1050,13 +1050,13 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
    uint32      hashcode = locallock->hashcode;
    LWLock     *partitionLock = LockHashPartitionLock(hashcode);
    PROC_QUEUE *waitQueue = &(lock->waitProcs);
+   SHM_QUEUE  *waitQueuePos;
    LOCKMASK    myHeldLocks = MyProc->heldLocks;
    TimestampTz standbyWaitStart = 0;
    bool        early_deadlock = false;
    bool        allow_autovacuum_cancel = true;
    bool        logged_recovery_conflict = false;
    ProcWaitStatus myWaitStatus;
-   PGPROC     *proc;
    PGPROC     *leader = MyProc->lockGroupLeader;
    int         i;
 
@@ -1104,13 +1104,16 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
     * we are only considering the part of the wait queue before my insertion
     * point.
     */
-   if (myHeldLocks != 0)
+   if (myHeldLocks != 0 && waitQueue->size > 0)
    {
        LOCKMASK    aheadRequests = 0;
+       SHM_QUEUE  *proc_node;
 
-       proc = (PGPROC *) waitQueue->links.next;
+       proc_node = waitQueue->links.next;
        for (i = 0; i < waitQueue->size; i++)
        {
+           PGPROC     *proc = (PGPROC *) proc_node;
+
            /*
             * If we're part of the same locking group as this waiter, its
             * locks neither conflict with ours nor contribute to
@@ -1118,7 +1121,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
             */
            if (leader != NULL && leader == proc->lockGroupLeader)
            {
-               proc = (PGPROC *) proc->links.next;
+               proc_node = proc->links.next;
                continue;
            }
            /* Must he wait for me? */
@@ -1153,24 +1156,25 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
            }
            /* Nope, so advance to next waiter */
            aheadRequests |= LOCKBIT_ON(proc->waitLockMode);
-           proc = (PGPROC *) proc->links.next;
+           proc_node = proc->links.next;
        }
 
        /*
-        * If we fall out of loop normally, proc points to waitQueue head, so
-        * we will insert at tail of queue as desired.
+        * If we iterated through the whole queue, cur points to the waitQueue
+        * head, so we will insert at tail of queue as desired.
         */
+       waitQueuePos = proc_node;
    }
    else
    {
        /* I hold no locks, so I can't push in front of anyone. */
-       proc = (PGPROC *) &(waitQueue->links);
+       waitQueuePos = &waitQueue->links;
    }
 
    /*
-    * Insert self into queue, ahead of the given proc (or at tail of queue).
+    * Insert self into queue, at the position determined above.
     */
-   SHMQueueInsertBefore(&(proc->links), &(MyProc->links));
+   SHMQueueInsertBefore(waitQueuePos, &MyProc->links);
    waitQueue->size++;
 
    lock->waitMask |= LOCKBIT_ON(lockmode);