From 68dba11805881b0c5e0a35c7698e3a9568fc4241 Mon Sep 17 00:00:00 2001 From: Subv Date: Mon, 23 Oct 2017 15:52:23 -0500 Subject: [PATCH] Kernel/SVC: Don't let svcReleaseMutex release a mutex owned by another thread. This behavior was reverse engineered from the 3DS kernel. --- src/core/hle/kernel/errors.h | 1 + src/core/hle/kernel/mutex.cpp | 42 ++++++++++++++++++++++++----------- src/core/hle/kernel/mutex.h | 10 +++++++-- src/core/hle/svc.cpp | 4 +--- 4 files changed, 39 insertions(+), 18 deletions(-) diff --git a/src/core/hle/kernel/errors.h b/src/core/hle/kernel/errors.h index 64aa61460..004764c63 100644 --- a/src/core/hle/kernel/errors.h +++ b/src/core/hle/kernel/errors.h @@ -13,6 +13,7 @@ enum { OutOfHandles = 19, SessionClosedByRemote = 26, PortNameTooLong = 30, + WrongLockingThread = 31, NoPendingSessions = 35, WrongPermission = 46, InvalidBufferDescriptor = 48, diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp index 2cbca5e5b..d559c06be 100644 --- a/src/core/hle/kernel/mutex.cpp +++ b/src/core/hle/kernel/mutex.cpp @@ -7,6 +7,7 @@ #include #include "common/assert.h" #include "core/core.h" +#include "core/hle/kernel/errors.h" #include "core/hle/kernel/kernel.h" #include "core/hle/kernel/mutex.h" #include "core/hle/kernel/thread.h" @@ -58,19 +59,34 @@ void Mutex::Acquire(Thread* thread) { lock_count++; } -void Mutex::Release() { - // Only release if the mutex is held - if (lock_count > 0) { - lock_count--; - - // Yield to the next thread only if we've fully released the mutex - if (lock_count == 0) { - holding_thread->held_mutexes.erase(this); - holding_thread->UpdatePriority(); - holding_thread = nullptr; - WakeupAllWaitingThreads(); - Core::System::GetInstance().PrepareReschedule(); +ResultCode Mutex::Release(Thread* thread) { + // We can only release the mutex if it's held by the calling thread. + if (thread != holding_thread) { + if (holding_thread) { + LOG_ERROR( + Kernel, + "Tried to release a mutex (owned by thread id %u) from a different thread id %u", + holding_thread->thread_id, thread->thread_id); } + return ResultCode(ErrCodes::WrongLockingThread, ErrorModule::Kernel, + ErrorSummary::InvalidArgument, ErrorLevel::Permanent); + } + + // Note: It should not be possible for the situation where the mutex has a holding thread with a + // zero lock count to occur. The real kernel still checks for this, so we do too. + if (lock_count <= 0) + return ResultCode(ErrorDescription::InvalidResultValue, ErrorModule::Kernel, + ErrorSummary::InvalidState, ErrorLevel::Permanent); + + lock_count--; + + // Yield to the next thread only if we've fully released the mutex + if (lock_count == 0) { + holding_thread->held_mutexes.erase(this); + holding_thread->UpdatePriority(); + holding_thread = nullptr; + WakeupAllWaitingThreads(); + Core::System::GetInstance().PrepareReschedule(); } } @@ -102,4 +118,4 @@ void Mutex::UpdatePriority() { } } -} // namespace +} // namespace Kernel diff --git a/src/core/hle/kernel/mutex.h b/src/core/hle/kernel/mutex.h index bacacd690..3afb99c59 100644 --- a/src/core/hle/kernel/mutex.h +++ b/src/core/hle/kernel/mutex.h @@ -8,6 +8,7 @@ #include "common/common_types.h" #include "core/hle/kernel/kernel.h" #include "core/hle/kernel/wait_object.h" +#include "core/hle/result.h" namespace Kernel { @@ -52,7 +53,12 @@ public: void AddWaitingThread(SharedPtr thread) override; void RemoveWaitingThread(Thread* thread) override; - void Release(); + /** + * Attempts to release the mutex from the specified thread. + * @param thread Thread that wants to release the mutex. + * @returns The result code of the operation. + */ + ResultCode Release(Thread* thread); private: Mutex(); @@ -65,4 +71,4 @@ private: */ void ReleaseThreadMutexes(Thread* thread); -} // namespace +} // namespace Kernel diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp index e8ca419d5..fe06af8f8 100644 --- a/src/core/hle/svc.cpp +++ b/src/core/hle/svc.cpp @@ -818,9 +818,7 @@ static ResultCode ReleaseMutex(Kernel::Handle handle) { if (mutex == nullptr) return ERR_INVALID_HANDLE; - mutex->Release(); - - return RESULT_SUCCESS; + return mutex->Release(Kernel::GetCurrentThread()); } /// Get the ID of the specified process