From 901b48c2e8ea05dcc632379873f854ad8b714c3e Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Mon, 12 Jun 2023 22:04:49 -0400 Subject: [PATCH] user_mutex: Fix potential race in switch_lock. We need to set the "to" mutex as locked+waiting before performing the unlock of the first mutex, otherwise something in userland could unset the "locked" flag but never call the kernel because "waiting" had not yet been set. In practice, the one consumer of this API (pthread_cond) could not, at present, wind up in that situation, as far as I can tell, so this race was entirely theoretical. --- src/system/kernel/locks/user_mutex.cpp | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/system/kernel/locks/user_mutex.cpp b/src/system/kernel/locks/user_mutex.cpp index 70d5f5694e..d6fb65b8e7 100644 --- a/src/system/kernel/locks/user_mutex.cpp +++ b/src/system/kernel/locks/user_mutex.cpp @@ -176,9 +176,8 @@ user_mutex_wait_locked(int32* mutex, phys_addr_t physicalAddress, const char* na } -static status_t -user_mutex_lock_locked(int32* mutex, phys_addr_t physicalAddress, - const char* name, uint32 flags, bigtime_t timeout, MutexLocker& locker) +static bool +user_mutex_prepare_to_lock(int32* mutex) { int32 oldValue = user_atomic_or(mutex, B_USER_MUTEX_LOCKED | B_USER_MUTEX_WAITING); @@ -187,9 +186,20 @@ user_mutex_lock_locked(int32* mutex, phys_addr_t physicalAddress, // clear the waiting flag and be done if ((oldValue & B_USER_MUTEX_WAITING) == 0) user_atomic_and(mutex, ~(int32)B_USER_MUTEX_WAITING); - return B_OK; + return true; } + return false; +} + + +static status_t +user_mutex_lock_locked(int32* mutex, phys_addr_t physicalAddress, + const char* name, uint32 flags, bigtime_t timeout, MutexLocker& locker) +{ + if (user_mutex_prepare_to_lock(mutex)) + return B_OK; + bool lastWaiter; status_t error = user_mutex_wait_locked(mutex, physicalAddress, name, flags, timeout, locker, lastWaiter); @@ -338,12 +348,16 @@ user_mutex_switch_lock(int32* fromMutex, int32* toMutex, const char* name, // unlock the first mutex and lock the second one { MutexLocker locker(sUserMutexTableLock); + + const bool alreadyLocked = user_mutex_prepare_to_lock(toMutex); user_atomic_and(fromMutex, ~(int32)B_USER_MUTEX_LOCKED); user_mutex_unblock_locked(fromMutex, fromWiringInfo.physicalAddress, flags); - error = user_mutex_lock_locked(toMutex, toWiringInfo.physicalAddress, - name, flags, timeout, locker); + if (!alreadyLocked) { + error = user_mutex_lock_locked(toMutex, toWiringInfo.physicalAddress, + name, flags, timeout, locker); + } } // unwire the pages