From 6bbe9de8819908c9da98dab640dbfeebab449e32 Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Sat, 26 Nov 2022 00:36:46 +0530 Subject: [PATCH] Fix result returned by `MutexLock` `MutexLock` incorrectly returned `InvalidCurrentMemory` for cases where the userspace value didn't match the expected value. It's been corrected to return no error in those cases while preserving the error code for usage in `ConditionalVariableWait`. --- .../main/cpp/skyline/kernel/types/KProcess.cpp | 15 ++++++--------- app/src/main/cpp/skyline/kernel/types/KProcess.h | 3 ++- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/app/src/main/cpp/skyline/kernel/types/KProcess.cpp b/app/src/main/cpp/skyline/kernel/types/KProcess.cpp index 8dd9e43c..3a428470 100644 --- a/app/src/main/cpp/skyline/kernel/types/KProcess.cpp +++ b/app/src/main/cpp/skyline/kernel/types/KProcess.cpp @@ -111,15 +111,15 @@ namespace skyline::kernel::type { constexpr u32 HandleWaitersBit{1UL << 30}; //!< A bit which denotes if a mutex psuedo-handle has waiters or not - Result KProcess::MutexLock(u32 *mutex, KHandle ownerHandle, KHandle tag) { + Result KProcess::MutexLock(u32 *mutex, KHandle ownerHandle, KHandle tag, bool failOnOutdated) { TRACE_EVENT_FMT("kernel", "MutexLock 0x{:X}", mutex); std::shared_ptr owner; try { owner = GetHandle(ownerHandle); } catch (const std::out_of_range &) { - if (*mutex != (ownerHandle | HandleWaitersBit)) - return result::InvalidCurrentMemory; + if (__atomic_load_n(mutex, __ATOMIC_SEQ_CST) != (ownerHandle | HandleWaitersBit)) + return failOnOutdated ? result::InvalidCurrentMemory : Result{}; return result::InvalidHandle; } @@ -128,13 +128,10 @@ namespace skyline::kernel::type { { std::scoped_lock lock{owner->waiterMutex, state.thread->waiterMutex}; // We need to lock both mutexes at the same time as we mutate the owner and the current thread, the ordering of locks **must** match MutexUnlock to avoid deadlocks - u32 value{}; - if (__atomic_compare_exchange_n(mutex, &value, tag, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)) - // We try to do a CAS to get ownership of the mutex in the case that it's unoccupied - return {}; + u32 value{__atomic_load_n(mutex, __ATOMIC_SEQ_CST)}; if (value != (ownerHandle | HandleWaitersBit)) // We ensure that the mutex's value is the handle with the waiter bit set - return result::InvalidCurrentMemory; + return failOnOutdated ? result::InvalidCurrentMemory : Result{}; auto &waiters{owner->waiters}; isHighestPriority = waiters.insert(std::upper_bound(waiters.begin(), waiters.end(), state.thread->priority.load(), KThread::IsHigherPriority), state.thread) == waiters.begin(); @@ -250,7 +247,7 @@ namespace skyline::kernel::type { KHandle value{}; if (!__atomic_compare_exchange_n(mutex, &value, tag, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)) - while (MutexLock(mutex, value & ~HandleWaitersBit, tag) != Result{}) + while (MutexLock(mutex, value & ~HandleWaitersBit, tag, true) != Result{}) if ((value = __atomic_or_fetch(mutex, HandleWaitersBit, __ATOMIC_SEQ_CST)) == HandleWaitersBit) if (__atomic_compare_exchange_n(mutex, &value, tag, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)) break; diff --git a/app/src/main/cpp/skyline/kernel/types/KProcess.h b/app/src/main/cpp/skyline/kernel/types/KProcess.h index 628229e3..08158785 100644 --- a/app/src/main/cpp/skyline/kernel/types/KProcess.h +++ b/app/src/main/cpp/skyline/kernel/types/KProcess.h @@ -212,8 +212,9 @@ namespace skyline { * @brief Locks the mutex at the specified address * @param ownerHandle The psuedo-handle of the current mutex owner * @param tag The handle of the thread which is requesting this lock + * @param failOnOutdated If true, the function will return InvalidCurrentMemory if the supplied ownerHandle is outdated */ - Result MutexLock(u32 *mutex, KHandle ownerHandle, KHandle tag); + Result MutexLock(u32 *mutex, KHandle ownerHandle, KHandle tag, bool failOnOutdated = false); /** * @brief Unlocks the mutex at the specified address