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

This behavior was reverse engineered from the 3DS kernel.
This commit is contained in:
Subv 2017-10-23 15:52:23 -05:00
parent ceb9880034
commit 68dba11805
4 changed files with 39 additions and 18 deletions

View file

@ -13,6 +13,7 @@ enum {
OutOfHandles = 19, OutOfHandles = 19,
SessionClosedByRemote = 26, SessionClosedByRemote = 26,
PortNameTooLong = 30, PortNameTooLong = 30,
WrongLockingThread = 31,
NoPendingSessions = 35, NoPendingSessions = 35,
WrongPermission = 46, WrongPermission = 46,
InvalidBufferDescriptor = 48, InvalidBufferDescriptor = 48,

View file

@ -7,6 +7,7 @@
#include <boost/range/algorithm_ext/erase.hpp> #include <boost/range/algorithm_ext/erase.hpp>
#include "common/assert.h" #include "common/assert.h"
#include "core/core.h" #include "core/core.h"
#include "core/hle/kernel/errors.h"
#include "core/hle/kernel/kernel.h" #include "core/hle/kernel/kernel.h"
#include "core/hle/kernel/mutex.h" #include "core/hle/kernel/mutex.h"
#include "core/hle/kernel/thread.h" #include "core/hle/kernel/thread.h"
@ -58,19 +59,34 @@ void Mutex::Acquire(Thread* thread) {
lock_count++; 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

View file

@ -8,6 +8,7 @@
#include "common/common_types.h" #include "common/common_types.h"
#include "core/hle/kernel/kernel.h" #include "core/hle/kernel/kernel.h"
#include "core/hle/kernel/wait_object.h" #include "core/hle/kernel/wait_object.h"
#include "core/hle/result.h"
namespace Kernel { namespace Kernel {
@ -52,7 +53,12 @@ public:
void AddWaitingThread(SharedPtr<Thread> thread) override; void AddWaitingThread(SharedPtr<Thread> thread) override;
void RemoveWaitingThread(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: private:
Mutex(); Mutex();
@ -65,4 +71,4 @@ private:
*/ */
void ReleaseThreadMutexes(Thread* thread); void ReleaseThreadMutexes(Thread* thread);
} // namespace } // namespace Kernel

View file

@ -818,9 +818,7 @@ static ResultCode ReleaseMutex(Kernel::Handle handle) {
if (mutex == nullptr) if (mutex == nullptr)
return ERR_INVALID_HANDLE; return ERR_INVALID_HANDLE;
mutex->Release(); return mutex->Release(Kernel::GetCurrentThread());
return RESULT_SUCCESS;
} }
/// Get the ID of the specified process /// Get the ID of the specified process