AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix race condition in futex-based QSemaphore

Browse Source 
Add one and reset the wakeAll bit atomically.

This avoids a race in a case where an acquiring thread
is owning the semaphore, and deleting it after a
set number of releases (one for each thread referencing the semaphore).

Two releasing threads could enter the if statement under
futexNeedsWake(prevValue), the counter been incremented at
this point and reached the value being acquired, meaning the thread
acquiring can be awakened by just one of the two releasers,
delete the semaphore, and then the second releaser would access
the now deleted semaphore.

The patch avoids that by unsetting and reading the wakeAll bit
atomically, so only one thread will try to wake all threads.

Pick-to: 6.3 6.2 5.15
Fixes: QTBUG-102484
Change-Id: I32172ed44d74378c627918e19b9e1aaadb5c6d1d
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Allan Sandfeld Jensen 2022-04-12 16:25:08 +02:00
parent 8b3f2dd994
commit 830b1550de
1 changed files with 6 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/corelib/thread/qsemaphore.cpp
View File

@ -357,7 +357,12 @@ void QSemaphore::release(int n)
quintptr nn = unsigned(n);
if (futexHasWaiterCount)
nn |= quint64(nn) << 32; // token count replicated in high word
quintptr prevValue = u.fetchAndAddRelease(nn);
quintptr prevValue = u.loadRelaxed();
quintptr newValue;
do { // loop just to ensure the operations are done atomically
newValue = prevValue + nn;
newValue &= (futexNeedsWakeAllBit - 1);
} while (!u.testAndSetRelease(prevValue, newValue, prevValue));
if (futexNeedsWake(prevValue)) {
#ifdef FUTEX_OP
if (futexHasWaiterCount) {
@ -379,7 +384,6 @@ void QSemaphore::release(int n)
quint32 oparg = 0;
quint32 cmp = FUTEX_OP_CMP_NE;
quint32 cmparg = 0;
u.fetchAndAndRelease(futexNeedsWakeAllBit - 1);
futexWakeOp(*futexLow32(&u), n, INT_MAX, *futexHigh32(&u), FUTEX_OP(op, oparg, cmp, cmparg));
return;
}
@ -391,7 +395,6 @@ void QSemaphore::release(int n)
// its acquisition anyway, so it has to wait;
// 2) it did not see the new counter value, in which case its
// futexWait will fail.
u.fetchAndAndRelease(futexNeedsWakeAllBit - 1);
if (futexHasWaiterCount) {
futexWakeAll(*futexLow32(&u));
futexWakeAll(*futexHigh32(&u));