Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 8c95416

Browse files
committed
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 46d6e5f, 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-
1 parent 3efc82e commit 8c95416

File tree

1 file changed

+14
-10
lines changed
  • src/backend/storage/lmgr

1 file changed

+14
-10
lines changed

src/backend/storage/lmgr/proc.c

+14-10
Original file line numberDiff line numberDiff line change
@@ -1050,13 +1050,13 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
10501050
uint32 hashcode = locallock->hashcode;
10511051
LWLock *partitionLock = LockHashPartitionLock(hashcode);
10521052
PROC_QUEUE *waitQueue = &(lock->waitProcs);
1053+
SHM_QUEUE *waitQueuePos;
10531054
LOCKMASK myHeldLocks = MyProc->heldLocks;
10541055
TimestampTz standbyWaitStart = 0;
10551056
bool early_deadlock = false;
10561057
bool allow_autovacuum_cancel = true;
10571058
bool logged_recovery_conflict = false;
10581059
ProcWaitStatus myWaitStatus;
1059-
PGPROC *proc;
10601060
PGPROC *leader = MyProc->lockGroupLeader;
10611061
int i;
10621062

@@ -1104,21 +1104,24 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
11041104
* we are only considering the part of the wait queue before my insertion
11051105
* point.
11061106
*/
1107-
if (myHeldLocks != 0)
1107+
if (myHeldLocks != 0 && waitQueue->size > 0)
11081108
{
11091109
LOCKMASK aheadRequests = 0;
1110+
SHM_QUEUE *proc_node;
11101111

1111-
proc = (PGPROC *) waitQueue->links.next;
1112+
proc_node = waitQueue->links.next;
11121113
for (i = 0; i < waitQueue->size; i++)
11131114
{
1115+
PGPROC *proc = (PGPROC *) proc_node;
1116+
11141117
/*
11151118
* If we're part of the same locking group as this waiter, its
11161119
* locks neither conflict with ours nor contribute to
11171120
* aheadRequests.
11181121
*/
11191122
if (leader != NULL && leader == proc->lockGroupLeader)
11201123
{
1121-
proc = (PGPROC *) proc->links.next;
1124+
proc_node = proc->links.next;
11221125
continue;
11231126
}
11241127
/* Must he wait for me? */
@@ -1153,24 +1156,25 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
11531156
}
11541157
/* Nope, so advance to next waiter */
11551158
aheadRequests |= LOCKBIT_ON(proc->waitLockMode);
1156-
proc = (PGPROC *) proc->links.next;
1159+
proc_node = proc->links.next;
11571160
}
11581161

11591162
/*
1160-
* If we fall out of loop normally, proc points to waitQueue head, so
1161-
* we will insert at tail of queue as desired.
1163+
* If we iterated through the whole queue, cur points to the waitQueue
1164+
* head, so we will insert at tail of queue as desired.
11621165
*/
1166+
waitQueuePos = proc_node;
11631167
}
11641168
else
11651169
{
11661170
/* I hold no locks, so I can't push in front of anyone. */
1167-
proc = (PGPROC *) &(waitQueue->links);
1171+
waitQueuePos = &waitQueue->links;
11681172
}
11691173

11701174
/*
1171-
* Insert self into queue, ahead of the given proc (or at tail of queue).
1175+
* Insert self into queue, at the position determined above.
11721176
*/
1173-
SHMQueueInsertBefore(&(proc->links), &(MyProc->links));
1177+
SHMQueueInsertBefore(waitQueuePos, &MyProc->links);
11741178
waitQueue->size++;
11751179

11761180
lock->waitMask |= LOCKBIT_ON(lockmode);

0 commit comments

Comments
 (0)