From 5ddc2e09b1a1b4a2b876d93dba1ccc4c56a24207 Mon Sep 17 00:00:00 2001 From: Subv Date: Wed, 11 Jan 2017 11:47:20 -0500 Subject: Y2R: Use the proper error code when GetStandardCoefficient receives an invalid value. --- src/core/hle/service/y2r_u.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/core') diff --git a/src/core/hle/service/y2r_u.cpp b/src/core/hle/service/y2r_u.cpp index a20194107..31bb466fc 100644 --- a/src/core/hle/service/y2r_u.cpp +++ b/src/core/hle/service/y2r_u.cpp @@ -531,7 +531,9 @@ static void GetStandardCoefficient(Interface* self) { LOG_DEBUG(Service_Y2R, "called standard_coefficient=%u ", index); } else { cmd_buff[0] = IPC::MakeHeader(0x21, 1, 0); - cmd_buff[1] = -1; // TODO(bunnei): Identify the correct error code for this + cmd_buff[1] = ResultCode(ErrorDescription::InvalidEnumValue, ErrorModule::CAM, + ErrorSummary::InvalidArgument, ErrorLevel::Usage) + .raw; LOG_ERROR(Service_Y2R, "called standard_coefficient=%u The argument is invalid!", index); } -- cgit v1.2.3 From f2f2572fed075d2dca5ae7abcea451ac5eb382ec Mon Sep 17 00:00:00 2001 From: Subv Date: Wed, 11 Jan 2017 12:08:10 -0500 Subject: Thread: Added priority range checking to svcSetThreadPriority and removed priority clamping code from Thread::SetPriority. --- src/core/hle/kernel/resource_limit.cpp | 2 ++ src/core/hle/kernel/thread.cpp | 20 ++------------------ src/core/hle/svc.cpp | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 18 deletions(-) (limited to 'src/core') diff --git a/src/core/hle/kernel/resource_limit.cpp b/src/core/hle/kernel/resource_limit.cpp index 253ab7045..3f51bc5de 100644 --- a/src/core/hle/kernel/resource_limit.cpp +++ b/src/core/hle/kernel/resource_limit.cpp @@ -62,6 +62,8 @@ s32 ResourceLimit::GetCurrentResourceValue(u32 resource) const { s32 ResourceLimit::GetMaxResourceValue(u32 resource) const { switch (resource) { + case PRIORITY: + return max_priority; case COMMIT: return max_commit; case THREAD: diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 8c6fbcd04..5ba9abf29 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -444,25 +444,9 @@ ResultVal> Thread::Create(std::string name, VAddr entry_point, return MakeResult>(std::move(thread)); } -// TODO(peachum): Remove this. Range checking should be done, and an appropriate error should be -// returned. -static void ClampPriority(const Thread* thread, s32* priority) { - if (*priority < THREADPRIO_HIGHEST || *priority > THREADPRIO_LOWEST) { - DEBUG_ASSERT_MSG( - false, "Application passed an out of range priority. An error should be returned."); - - s32 new_priority = MathUtil::Clamp(*priority, THREADPRIO_HIGHEST, THREADPRIO_LOWEST); - LOG_WARNING(Kernel_SVC, "(name=%s): invalid priority=%d, clamping to %d", - thread->name.c_str(), *priority, new_priority); - // TODO(bunnei): Clamping to a valid priority is not necessarily correct behavior... Confirm - // validity of this - *priority = new_priority; - } -} - void Thread::SetPriority(s32 priority) { - ClampPriority(this, &priority); - + ASSERT_MSG(priority <= THREADPRIO_LOWEST && priority >= THREADPRIO_HIGHEST, + "Invalid priority value."); // If thread was ready, adjust queues if (status == THREADSTATUS_READY) ready_queue.move(this, current_priority, priority); diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp index 2b242ff98..8cb6a1c94 100644 --- a/src/core/hle/svc.cpp +++ b/src/core/hle/svc.cpp @@ -598,10 +598,24 @@ static ResultCode GetThreadPriority(s32* priority, Kernel::Handle handle) { /// Sets the priority for the specified thread static ResultCode SetThreadPriority(Kernel::Handle handle, s32 priority) { + if (priority > THREADPRIO_LOWEST) { + return ResultCode(ErrorDescription::OutOfRange, ErrorModule::OS, + ErrorSummary::InvalidArgument, ErrorLevel::Usage); + } + SharedPtr thread = Kernel::g_handle_table.Get(handle); if (thread == nullptr) return ERR_INVALID_HANDLE; + using Kernel::ResourceLimit; + // Note: The kernel uses the current process's resource limit instead of + // the one from the thread owner's resource limit. + Kernel::SharedPtr& resource_limit = Kernel::g_current_process->resource_limit; + if (resource_limit->GetMaxResourceValue(Kernel::ResourceTypes::PRIORITY) > priority) { + return ResultCode(ErrorDescription::NotAuthorized, ErrorModule::OS, + ErrorSummary::WrongArgument, ErrorLevel::Permanent); + } + thread->SetPriority(priority); thread->UpdatePriority(); -- cgit v1.2.3 From 1ddff1451140ef58058237a3198d363b96dc238e Mon Sep 17 00:00:00 2001 From: Subv Date: Wed, 11 Jan 2017 13:20:14 -0500 Subject: Threads: Check the process' resource limit for the max allowed priority when creating a thread and remove the priority clamping code. --- src/core/hle/kernel/thread.cpp | 10 ++-------- src/core/hle/svc.cpp | 12 +++++++----- 2 files changed, 9 insertions(+), 13 deletions(-) (limited to 'src/core') diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 5ba9abf29..3b7555d87 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -353,14 +353,8 @@ static void ResetThreadContext(ARM_Interface::ThreadContext& context, u32 stack_ ResultVal> Thread::Create(std::string name, VAddr entry_point, s32 priority, u32 arg, s32 processor_id, VAddr stack_top) { - if (priority < THREADPRIO_HIGHEST || priority > THREADPRIO_LOWEST) { - s32 new_priority = MathUtil::Clamp(priority, THREADPRIO_HIGHEST, THREADPRIO_LOWEST); - LOG_WARNING(Kernel_SVC, "(name=%s): invalid priority=%d, clamping to %d", name.c_str(), - priority, new_priority); - // TODO(bunnei): Clamping to a valid priority is not necessarily correct behavior... Confirm - // validity of this - priority = new_priority; - } + ASSERT_MSG(priority >= THREADPRIO_HIGHEST && priority <= THREADPRIO_LOWEST, + "Invalid thread priority"); if (!Memory::IsValidVirtualAddress(entry_point)) { LOG_ERROR(Kernel_SVC, "(name=%s): invalid entry %08x", name.c_str(), entry_point); diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp index 8cb6a1c94..96db39ad9 100644 --- a/src/core/hle/svc.cpp +++ b/src/core/hle/svc.cpp @@ -532,16 +532,18 @@ static ResultCode CreateThread(Kernel::Handle* out_handle, s32 priority, u32 ent name = Common::StringFromFormat("unknown-%08x", entry_point); } - // TODO(bunnei): Implement resource limits to return an error code instead of the below assert. - // The error code should be: Description::NotAuthorized, Module::OS, Summary::WrongArgument, - // Level::Permanent - ASSERT_MSG(priority >= THREADPRIO_USERLAND_MAX, "Unexpected thread priority!"); - if (priority > THREADPRIO_LOWEST) { return ResultCode(ErrorDescription::OutOfRange, ErrorModule::OS, ErrorSummary::InvalidArgument, ErrorLevel::Usage); } + using Kernel::ResourceLimit; + Kernel::SharedPtr& resource_limit = Kernel::g_current_process->resource_limit; + if (resource_limit->GetMaxResourceValue(Kernel::ResourceTypes::PRIORITY) > priority) { + return ResultCode(ErrorDescription::NotAuthorized, ErrorModule::OS, + ErrorSummary::WrongArgument, ErrorLevel::Permanent); + } + switch (processor_id) { case THREADPROCESSORID_ALL: case THREADPROCESSORID_DEFAULT: -- cgit v1.2.3