Merge pull request #3042 from Subv/releasemutex

Kernel/SVC: Don't let svcReleaseMutex release a mutex owned by another thread

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,

src/core/hle/kernel/mutex.cpp

@@ -7,6 +7,7 @@
 #include <boost/range/algorithm_ext/erase.hpp>
 #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() {
+ResultCode Mutex::Release(Thread* thread) {
-    // Only release if the mutex is held
+    // We can only release the mutex if it's held by the calling thread.
-    if (lock_count > 0) {
+    if (thread != holding_thread) {
-        lock_count--;
+        if (holding_thread) {
-
+            LOG_ERROR(
-        // Yield to the next thread only if we've fully released the mutex
+                Kernel,
-        if (lock_count == 0) {
+                "Tried to release a mutex (owned by thread id %u) from a different thread id %u",
-            holding_thread->held_mutexes.erase(this);
+                holding_thread->thread_id, thread->thread_id);
-            holding_thread->UpdatePriority();
-            holding_thread = nullptr;
-            WakeupAllWaitingThreads();
-            Core::System::GetInstance().PrepareReschedule();
         }
+        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

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> 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

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 mutex->Release(Kernel::GetCurrentThread());
-
-    return RESULT_SUCCESS;
 }
 
 /// Get the ID of the specified process