From 205e6d3b975a7f699aeb0341c78a55562094c32b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 30 Dec 2018 20:59:52 -0500 Subject: kernel/svc: Simplify thread core ID sanitizing in CreateThread Rather than use a switch here, this can be collapsed into a simple range check, which is a little easier on the eyes. --- src/core/hle/kernel/svc.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'src') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 660e6f577..352ab2d14 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1233,13 +1233,7 @@ static ResultCode CreateThread(Handle* out_handle, VAddr entry_point, u64 arg, V ASSERT(processor_id != THREADPROCESSORID_IDEAL); } - switch (processor_id) { - case THREADPROCESSORID_0: - case THREADPROCESSORID_1: - case THREADPROCESSORID_2: - case THREADPROCESSORID_3: - break; - default: + if (processor_id < THREADPROCESSORID_0 || processor_id > THREADPROCESSORID_3) { LOG_ERROR(Kernel_SVC, "Invalid thread processor ID: {}", processor_id); return ERR_INVALID_PROCESSOR_ID; } -- cgit v1.2.3 From 87696041442fdcb735f405f05ca4e3722b4f7128 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 30 Dec 2018 21:09:00 -0500 Subject: kernel/process: Rename GetAllowedProcessorMask() and GetAllowedThreadPriorityMask() Makes them consistent with their kernel capability counterparts. --- src/core/hle/kernel/process.h | 6 +++--- src/core/hle/kernel/svc.cpp | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/core/hle/kernel/process.h b/src/core/hle/kernel/process.h index 450dc6eeb..b710104ab 100644 --- a/src/core/hle/kernel/process.h +++ b/src/core/hle/kernel/process.h @@ -173,13 +173,13 @@ public: return ideal_core; } - /// Gets the bitmask of allowed CPUs that this process' threads can run on. - u64 GetAllowedProcessorMask() const { + /// Gets the bitmask of allowed cores that this process' threads can run on. + u64 GetCoreMask() const { return capabilities.GetCoreMask(); } /// Gets the bitmask of allowed thread priorities. - u64 GetAllowedThreadPriorityMask() const { + u64 GetPriorityMask() const { return capabilities.GetPriorityMask(); } diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 352ab2d14..8d8d4e0ab 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -715,8 +715,8 @@ static ResultCode GetInfo(u64* result, u64 info_id, u64 handle, u64 info_sub_id) enum class GetInfoType : u64 { // 1.0.0+ - AllowedCpuIdBitmask = 0, - AllowedThreadPrioBitmask = 1, + AllowedCPUCoreMask = 0, + AllowedThreadPriorityMask = 1, MapRegionBaseAddr = 2, MapRegionSize = 3, HeapRegionBaseAddr = 4, @@ -747,8 +747,8 @@ static ResultCode GetInfo(u64* result, u64 info_id, u64 handle, u64 info_sub_id) const auto info_id_type = static_cast(info_id); switch (info_id_type) { - case GetInfoType::AllowedCpuIdBitmask: - case GetInfoType::AllowedThreadPrioBitmask: + case GetInfoType::AllowedCPUCoreMask: + case GetInfoType::AllowedThreadPriorityMask: case GetInfoType::MapRegionBaseAddr: case GetInfoType::MapRegionSize: case GetInfoType::HeapRegionBaseAddr: @@ -774,12 +774,12 @@ static ResultCode GetInfo(u64* result, u64 info_id, u64 handle, u64 info_sub_id) } switch (info_id_type) { - case GetInfoType::AllowedCpuIdBitmask: - *result = process->GetAllowedProcessorMask(); + case GetInfoType::AllowedCPUCoreMask: + *result = process->GetCoreMask(); return RESULT_SUCCESS; - case GetInfoType::AllowedThreadPrioBitmask: - *result = process->GetAllowedThreadPriorityMask(); + case GetInfoType::AllowedThreadPriorityMask: + *result = process->GetPriorityMask(); return RESULT_SUCCESS; case GetInfoType::MapRegionBaseAddr: -- cgit v1.2.3 From 3a8d38be7e584d1fba5f35f1e4e4449f40fa2073 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 30 Dec 2018 21:20:07 -0500 Subject: kernel/svc: Sanitize core number and thread priorities in CreateThread() Now that we handle the kernel capability descriptors we can correct CreateThread to properly check against the core and priority masks like the actual kernel does. --- src/core/hle/kernel/svc.cpp | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 8d8d4e0ab..ada05abd2 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1219,12 +1219,6 @@ static ResultCode CreateThread(Handle* out_handle, VAddr entry_point, u64 arg, V "threadpriority=0x{:08X}, processorid=0x{:08X} : created handle=0x{:08X}", entry_point, arg, stack_top, priority, processor_id, *out_handle); - if (priority > THREADPRIO_LOWEST) { - LOG_ERROR(Kernel_SVC, "An invalid priority was specified, expected {} but got {}", - THREADPRIO_LOWEST, priority); - return ERR_INVALID_THREAD_PRIORITY; - } - auto* const current_process = Core::CurrentProcess(); if (processor_id == THREADPROCESSORID_IDEAL) { @@ -1238,6 +1232,23 @@ static ResultCode CreateThread(Handle* out_handle, VAddr entry_point, u64 arg, V return ERR_INVALID_PROCESSOR_ID; } + const u64 core_mask = current_process->GetCoreMask(); + if ((core_mask | (1ULL << processor_id)) != core_mask) { + LOG_ERROR(Kernel_SVC, "Invalid thread core specified ({})", processor_id); + return ERR_INVALID_PROCESSOR_ID; + } + + if (priority > THREADPRIO_LOWEST) { + LOG_ERROR(Kernel_SVC, "An invalid priority was specified, expected {} but got {}", + THREADPRIO_LOWEST, priority); + return ERR_INVALID_THREAD_PRIORITY; + } + + if (((1ULL << priority) & current_process->GetPriorityMask()) == 0) { + LOG_ERROR(Kernel_SVC, "Invalid thread priority specified ({})", priority); + return ERR_INVALID_THREAD_PRIORITY; + } + const std::string name = fmt::format("thread-{:X}", entry_point); auto& kernel = Core::System::GetInstance().Kernel(); CASCADE_RESULT(SharedPtr thread, -- cgit v1.2.3 From b4242633ad542f1f442e825c7ad426f05d703e40 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 30 Dec 2018 21:27:30 -0500 Subject: kernel/svc: Correct misleading error message within CreateThread() This is a bounds check to ensure that the thread priority is within the valid range of 0-64. If it exceeds 64, that doesn't necessarily mean that an actual priority of 64 was expected (it actually means whoever called the function screwed up their math). Instead clarify the message to indicate the allowed range of thread priorities. --- src/core/hle/kernel/svc.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index ada05abd2..6588bd3b8 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1239,8 +1239,9 @@ static ResultCode CreateThread(Handle* out_handle, VAddr entry_point, u64 arg, V } if (priority > THREADPRIO_LOWEST) { - LOG_ERROR(Kernel_SVC, "An invalid priority was specified, expected {} but got {}", - THREADPRIO_LOWEST, priority); + LOG_ERROR(Kernel_SVC, + "Invalid thread priority specified ({}). Must be within the range 0-64", + priority); return ERR_INVALID_THREAD_PRIORITY; } -- cgit v1.2.3