From 366985ca925c28867b4cdf847db978acd6c2db1e Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 10:08:46 -0500 Subject: vm_manager: Amend MemoryState enum members Amends the MemoryState enum to use the same values like the actual kernel does. Also provides the necessary operators to operate on them. This will be necessary in the future for implementing svcSetMemoryAttribute, as memory block state is checked before applying the attribute. --- src/core/hle/kernel/svc.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/core/hle/kernel/svc.cpp') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index f43c7201c..4ae92ff9e 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -273,7 +273,7 @@ static ResultCode MapMemory(VAddr dst_addr, VAddr src_addr, u64 size) { return result; } - return current_process->MirrorMemory(dst_addr, src_addr, size); + return current_process->MirrorMemory(dst_addr, src_addr, size, MemoryState::Stack); } /// Unmaps a region that was previously mapped with svcMapMemory @@ -1086,7 +1086,7 @@ static ResultCode QueryProcessMemory(MemoryInfo* memory_info, PageInfo* /*page_i memory_info->base_address = vma->second.base; memory_info->permission = static_cast(vma->second.permissions); memory_info->size = vma->second.size; - memory_info->type = static_cast(vma->second.meminfo_state); + memory_info->type = ToSvcMemoryState(vma->second.meminfo_state); } else { memory_info->base_address = 0; memory_info->permission = static_cast(VMAPermission::None); -- cgit v1.2.3 From a8cc03502b41b44150af71535d2b662a7ee3390c Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 11:34:01 -0500 Subject: vm_manager: Migrate memory querying to the VMManager interface Gets rid of the need to directly access the managed VMAs outside of the memory manager itself just for querying memory. --- src/core/hle/kernel/svc.cpp | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) (limited to 'src/core/hle/kernel/svc.cpp') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 4ae92ff9e..8b079bc40 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1068,8 +1068,8 @@ static ResultCode UnmapSharedMemory(Handle shared_memory_handle, VAddr addr, u64 /// Query process memory static ResultCode QueryProcessMemory(MemoryInfo* memory_info, PageInfo* /*page_info*/, - Handle process_handle, u64 addr) { - LOG_TRACE(Kernel_SVC, "called process=0x{:08X} addr={:X}", process_handle, addr); + Handle process_handle, u64 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); if (!process) { @@ -1079,21 +1079,9 @@ static ResultCode QueryProcessMemory(MemoryInfo* memory_info, PageInfo* /*page_i } const auto& vm_manager = process->VMManager(); - const auto vma = vm_manager.FindVMA(addr); - - memory_info->attributes = 0; - if (vm_manager.IsValidHandle(vma)) { - memory_info->base_address = vma->second.base; - memory_info->permission = static_cast(vma->second.permissions); - memory_info->size = vma->second.size; - memory_info->type = ToSvcMemoryState(vma->second.meminfo_state); - } else { - memory_info->base_address = 0; - memory_info->permission = static_cast(VMAPermission::None); - memory_info->size = 0; - memory_info->type = static_cast(MemoryState::Unmapped); - } + const auto result = vm_manager.QueryMemory(address); + *memory_info = result; return RESULT_SUCCESS; } -- cgit v1.2.3 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 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) (limited to 'src/core/hle/kernel/svc.cpp') 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 -- cgit v1.2.3 From 09a219d5b4a45537502c219542a51a57d6d0c317 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 12:52:31 -0500 Subject: svc: Write out the complete MemoryInfo structure in QueryProcessMemory In the previous change, the memory writing was moved into the service function itself, however it still had a problem, in that the entire MemoryInfo structure wasn't being written out, only the first 32 bytes of it were being written out. We still need to write out the trailing two reference count members and zero out the padding bits. Not doing this can result in wrong behavior in userland code in the following scenario: MemoryInfo info; // Put on the stack, not quaranteed to be zeroed out. svcQueryMemory(&info, ...); if (info.device_refcount == ...) // Whoops, uninitialized read. This can also cause the wrong thing to happen if the user code uses std::memcmp to compare the struct, with another one (questionable, but allowed), as the padding bits are not guaranteed to be a deterministic value. Note that the kernel itself also fully zeroes out the structure before writing it out including the padding bits. --- src/core/hle/kernel/svc.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/core/hle/kernel/svc.cpp') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index a1ecc4540..8b51aa2c7 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1086,6 +1086,9 @@ static ResultCode QueryProcessMemory(VAddr memory_info_address, VAddr page_info_ 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); + Memory::Write32(memory_info_address + 32, memory_info.ipc_ref_count); + Memory::Write32(memory_info_address + 28, memory_info.device_ref_count); + Memory::Write32(memory_info_address + 36, 0); // Page info appears to be currently unused by the kernel and is always set to zero. Memory::Write32(page_info_address, 0); -- cgit v1.2.3 From b79f08661337f22d46d519218c1cc6c013a14d2c Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 13:42:21 -0500 Subject: svc: Enable svcQueryProcessMemory svcQueryProcessMemory is trivial to implement, given all the behavior necessary for it is present, it just needs a handler for it. --- src/core/hle/kernel/svc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/core/hle/kernel/svc.cpp') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 8b51aa2c7..c1100f197 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1912,7 +1912,7 @@ static const FunctionDef SVC_Table[] = { {0x73, nullptr, "SetProcessMemoryPermission"}, {0x74, nullptr, "MapProcessMemory"}, {0x75, nullptr, "UnmapProcessMemory"}, - {0x76, nullptr, "QueryProcessMemory"}, + {0x76, SvcWrap, "QueryProcessMemory"}, {0x77, nullptr, "MapProcessCodeMemory"}, {0x78, nullptr, "UnmapProcessCodeMemory"}, {0x79, nullptr, "CreateProcess"}, -- cgit v1.2.3