From d8deb39b83409f1d9b5eeec0a719d560e4409aae Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 11:48:06 -0500 Subject: svc: Handle memory writing explicitly within QueryProcessMemory Moves the memory writes directly into QueryProcessMemory instead of letting the wrapper function do it. It would be inaccurate to allow the handler to do it because there's cases where memory shouldn't even be written to. For example, if the given process handle is invalid. HOWEVER, if the memory writing is within the wrapper, then we have no control over if these memory writes occur, meaning in an error case, 68 bytes of memory randomly get trashed with zeroes, 64 of those being written to wherever the memory info address points to, and the remaining 4 being written wherever the page info address points to. One solution in this case would be to just conditionally check within the handler itself, but this is kind of smelly, given the handler shouldn't be performing conditional behavior itself, it's a behavior of the managed function. In other words, if you remove the handler from the equation entirely, does the function still retain its proper behavior? In this case, no. Now, we don't potentially trash memory from this function if an invalid query is performed. --- src/core/hle/kernel/svc.cpp | 31 ++++++++++++++++++++++--------- src/core/hle/kernel/svc_wrap.h | 17 ----------------- 2 files changed, 22 insertions(+), 26 deletions(-) (limited to 'src/core/hle/kernel') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 8b079bc40..a1ecc4540 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -35,6 +35,7 @@ #include "core/hle/lock.h" #include "core/hle/result.h" #include "core/hle/service/service.h" +#include "core/memory.h" namespace Kernel { namespace { @@ -1066,9 +1067,8 @@ static ResultCode UnmapSharedMemory(Handle shared_memory_handle, VAddr addr, u64 return shared_memory->Unmap(*current_process, addr); } -/// Query process memory -static ResultCode QueryProcessMemory(MemoryInfo* memory_info, PageInfo* /*page_info*/, - Handle process_handle, u64 address) { +static ResultCode QueryProcessMemory(VAddr memory_info_address, VAddr page_info_address, + Handle process_handle, VAddr address) { LOG_TRACE(Kernel_SVC, "called process=0x{:08X} address={:X}", process_handle, address); const auto& handle_table = Core::CurrentProcess()->GetHandleTable(); SharedPtr process = handle_table.Get(process_handle); @@ -1079,16 +1079,29 @@ static ResultCode QueryProcessMemory(MemoryInfo* memory_info, PageInfo* /*page_i } const auto& vm_manager = process->VMManager(); - const auto result = vm_manager.QueryMemory(address); + const MemoryInfo memory_info = vm_manager.QueryMemory(address); + + Memory::Write64(memory_info_address, memory_info.base_address); + Memory::Write64(memory_info_address + 8, memory_info.size); + Memory::Write32(memory_info_address + 16, memory_info.state); + Memory::Write32(memory_info_address + 20, memory_info.attributes); + Memory::Write32(memory_info_address + 24, memory_info.permission); + + // Page info appears to be currently unused by the kernel and is always set to zero. + Memory::Write32(page_info_address, 0); - *memory_info = result; return RESULT_SUCCESS; } -/// Query memory -static ResultCode QueryMemory(MemoryInfo* memory_info, PageInfo* page_info, VAddr addr) { - LOG_TRACE(Kernel_SVC, "called, addr={:X}", addr); - return QueryProcessMemory(memory_info, page_info, CurrentProcess, addr); +static ResultCode QueryMemory(VAddr memory_info_address, VAddr page_info_address, + VAddr query_address) { + LOG_TRACE(Kernel_SVC, + "called, memory_info_address=0x{:016X}, page_info_address=0x{:016X}, " + "query_address=0x{:016X}", + memory_info_address, page_info_address, query_address); + + return QueryProcessMemory(memory_info_address, page_info_address, CurrentProcess, + query_address); } /// Exits the current process diff --git a/src/core/hle/kernel/svc_wrap.h b/src/core/hle/kernel/svc_wrap.h index 27a11d82e..f03b5438b 100644 --- a/src/core/hle/kernel/svc_wrap.h +++ b/src/core/hle/kernel/svc_wrap.h @@ -7,9 +7,7 @@ #include "common/common_types.h" #include "core/arm/arm_interface.h" #include "core/core.h" -#include "core/hle/kernel/vm_manager.h" #include "core/hle/result.h" -#include "core/memory.h" namespace Kernel { @@ -191,21 +189,6 @@ void SvcWrap() { FuncReturn(retval); } -template -void SvcWrap() { - MemoryInfo memory_info = {}; - PageInfo page_info = {}; - u32 retval = func(&memory_info, &page_info, Param(2)).raw; - - Memory::Write64(Param(0), memory_info.base_address); - Memory::Write64(Param(0) + 8, memory_info.size); - Memory::Write32(Param(0) + 16, memory_info.state); - Memory::Write32(Param(0) + 20, memory_info.attributes); - Memory::Write32(Param(0) + 24, memory_info.permission); - - FuncReturn(retval); -} - template void SvcWrap() { u32 param_1 = 0; -- cgit v1.2.3