From e5d30217d34ff07bb8dcccc17ec89224a6d79a1c Mon Sep 17 00:00:00 2001 From: Michael Scire Date: Sat, 15 Aug 2020 03:01:43 -0700 Subject: [PATCH] kern: fix reference leak in KThread::GetThreadFromId callers --- .../source/kern_k_debug_base.cpp | 22 ++++++++++--------- .../source/svc/kern_svc_debug.cpp | 5 +++-- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/libraries/libmesosphere/source/kern_k_debug_base.cpp b/libraries/libmesosphere/source/kern_k_debug_base.cpp index 93c3d044c..f7ab4328c 100644 --- a/libraries/libmesosphere/source/kern_k_debug_base.cpp +++ b/libraries/libmesosphere/source/kern_k_debug_base.cpp @@ -471,8 +471,9 @@ namespace ams::kern { KScopedLightLock lk(this->lock); /* Get the thread from its id. */ - KScopedAutoObject thread = KThread::GetThreadFromId(thread_id); - R_UNLESS(thread.IsNotNull(), svc::ResultInvalidId()); + KThread *thread = KThread::GetThreadFromId(thread_id); + R_UNLESS(thread != nullptr, svc::ResultInvalidId()); + ON_SCOPE_EXIT { thread->Close(); }; /* Verify that the thread is owned by our process. */ R_UNLESS(this->process == thread->GetOwnerProcess(), svc::ResultInvalidId()); @@ -482,7 +483,7 @@ namespace ams::kern { /* Check that the thread is not the current one. */ /* NOTE: Nintendo does not check this, and thus the following loop will deadlock. */ - R_UNLESS(thread.GetPointerUnsafe() != GetCurrentThreadPointer(), svc::ResultInvalidId()); + R_UNLESS(thread != GetCurrentThreadPointer(), svc::ResultInvalidId()); /* Try to get the thread context until the thread isn't current on any core. */ while (true) { @@ -495,7 +496,7 @@ namespace ams::kern { if (thread->GetRawState() != KThread::ThreadState_Runnable) { bool current = false; for (auto i = 0; i < static_cast(cpu::NumCores); ++i) { - if (thread.GetPointerUnsafe() == Kernel::GetCurrentContext(i).current_thread) { + if (thread == Kernel::GetCurrentContext(i).current_thread) { current = true; } break; @@ -508,7 +509,7 @@ namespace ams::kern { } /* Get the thread context. */ - return this->GetThreadContextImpl(out, thread.GetPointerUnsafe(), context_flags); + return this->GetThreadContextImpl(out, thread, context_flags); } } @@ -517,8 +518,9 @@ namespace ams::kern { KScopedLightLock lk(this->lock); /* Get the thread from its id. */ - KScopedAutoObject thread = KThread::GetThreadFromId(thread_id); - R_UNLESS(thread.IsNotNull(), svc::ResultInvalidId()); + KThread *thread = KThread::GetThreadFromId(thread_id); + R_UNLESS(thread != nullptr, svc::ResultInvalidId()); + ON_SCOPE_EXIT { thread->Close(); }; /* Verify that the thread is owned by our process. */ R_UNLESS(this->process == thread->GetOwnerProcess(), svc::ResultInvalidId()); @@ -528,7 +530,7 @@ namespace ams::kern { /* Check that the thread is not the current one. */ /* NOTE: Nintendo does not check this, and thus the following loop will deadlock. */ - R_UNLESS(thread.GetPointerUnsafe() != GetCurrentThreadPointer(), svc::ResultInvalidId()); + R_UNLESS(thread != GetCurrentThreadPointer(), svc::ResultInvalidId()); /* Try to get the thread context until the thread isn't current on any core. */ while (true) { @@ -541,7 +543,7 @@ namespace ams::kern { if (thread->GetRawState() != KThread::ThreadState_Runnable) { bool current = false; for (auto i = 0; i < static_cast(cpu::NumCores); ++i) { - if (thread.GetPointerUnsafe() == Kernel::GetCurrentContext(i).current_thread) { + if (thread == Kernel::GetCurrentContext(i).current_thread) { current = true; } break; @@ -560,7 +562,7 @@ namespace ams::kern { } /* Set the thread context. */ - return this->SetThreadContextImpl(ctx, thread.GetPointerUnsafe(), context_flags); + return this->SetThreadContextImpl(ctx, thread, context_flags); } } diff --git a/libraries/libmesosphere/source/svc/kern_svc_debug.cpp b/libraries/libmesosphere/source/svc/kern_svc_debug.cpp index 846b749f6..2377117e0 100644 --- a/libraries/libmesosphere/source/svc/kern_svc_debug.cpp +++ b/libraries/libmesosphere/source/svc/kern_svc_debug.cpp @@ -291,8 +291,9 @@ namespace ams::kern::svc { R_UNLESS(debug.IsNotNull(), svc::ResultInvalidHandle()); /* Get the thread from its id. */ - KScopedAutoObject thread = KThread::GetThreadFromId(thread_id); - R_UNLESS(thread.IsNotNull(), svc::ResultInvalidThreadId()); + KThread *thread = KThread::GetThreadFromId(thread_id); + R_UNLESS(thread != nullptr, svc::ResultInvalidThreadId()); + ON_SCOPE_EXIT { thread->Close(); }; /* Get the process from the debug object. */ KScopedAutoObject process = debug->GetProcess();