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.
This commit is contained in:
Lioncash 2018-12-12 11:48:06 -05:00
parent b1b855c5d9
commit d8deb39b83
2 changed files with 22 additions and 26 deletions

View file

@ -35,6 +35,7 @@
#include "core/hle/lock.h" #include "core/hle/lock.h"
#include "core/hle/result.h" #include "core/hle/result.h"
#include "core/hle/service/service.h" #include "core/hle/service/service.h"
#include "core/memory.h"
namespace Kernel { namespace Kernel {
namespace { namespace {
@ -1066,9 +1067,8 @@ static ResultCode UnmapSharedMemory(Handle shared_memory_handle, VAddr addr, u64
return shared_memory->Unmap(*current_process, addr); return shared_memory->Unmap(*current_process, addr);
} }
/// Query process memory static ResultCode QueryProcessMemory(VAddr memory_info_address, VAddr page_info_address,
static ResultCode QueryProcessMemory(MemoryInfo* memory_info, PageInfo* /*page_info*/, Handle process_handle, VAddr address) {
Handle process_handle, u64 address) {
LOG_TRACE(Kernel_SVC, "called process=0x{:08X} address={:X}", process_handle, address); LOG_TRACE(Kernel_SVC, "called process=0x{:08X} address={:X}", process_handle, address);
const auto& handle_table = Core::CurrentProcess()->GetHandleTable(); const auto& handle_table = Core::CurrentProcess()->GetHandleTable();
SharedPtr<Process> process = handle_table.Get<Process>(process_handle); SharedPtr<Process> process = handle_table.Get<Process>(process_handle);
@ -1079,16 +1079,29 @@ static ResultCode QueryProcessMemory(MemoryInfo* memory_info, PageInfo* /*page_i
} }
const auto& vm_manager = process->VMManager(); 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; return RESULT_SUCCESS;
} }
/// Query memory static ResultCode QueryMemory(VAddr memory_info_address, VAddr page_info_address,
static ResultCode QueryMemory(MemoryInfo* memory_info, PageInfo* page_info, VAddr addr) { VAddr query_address) {
LOG_TRACE(Kernel_SVC, "called, addr={:X}", addr); LOG_TRACE(Kernel_SVC,
return QueryProcessMemory(memory_info, page_info, CurrentProcess, addr); "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 /// Exits the current process

View file

@ -7,9 +7,7 @@
#include "common/common_types.h" #include "common/common_types.h"
#include "core/arm/arm_interface.h" #include "core/arm/arm_interface.h"
#include "core/core.h" #include "core/core.h"
#include "core/hle/kernel/vm_manager.h"
#include "core/hle/result.h" #include "core/hle/result.h"
#include "core/memory.h"
namespace Kernel { namespace Kernel {
@ -191,21 +189,6 @@ void SvcWrap() {
FuncReturn(retval); FuncReturn(retval);
} }
template <ResultCode func(MemoryInfo*, PageInfo*, u64)>
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 <ResultCode func(u32*, u64, u64, u32)> template <ResultCode func(u32*, u64, u64, u32)>
void SvcWrap() { void SvcWrap() {
u32 param_1 = 0; u32 param_1 = 0;