From 366985ca925c28867b4cdf847db978acd6c2db1e Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 10:08:46 -0500 Subject: [PATCH 1/8] 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/process.cpp | 2 +- src/core/hle/kernel/process.h | 3 +- src/core/hle/kernel/svc.cpp | 4 +- src/core/hle/kernel/vm_manager.cpp | 4 +- src/core/hle/kernel/vm_manager.h | 126 ++++++++++++++++++++++++----- 5 files changed, 111 insertions(+), 28 deletions(-) diff --git a/src/core/hle/kernel/process.cpp b/src/core/hle/kernel/process.cpp index c817fb4495..5356a4a3f6 100644 --- a/src/core/hle/kernel/process.cpp +++ b/src/core/hle/kernel/process.cpp @@ -150,7 +150,7 @@ void Process::Run(VAddr entry_point, s32 main_thread_priority, u32 stack_size) { vm_manager .MapMemoryBlock(vm_manager.GetTLSIORegionEndAddress() - stack_size, std::make_shared>(stack_size, 0), 0, stack_size, - MemoryState::Mapped) + MemoryState::Stack) .Unwrap(); vm_manager.LogLayout(); diff --git a/src/core/hle/kernel/process.h b/src/core/hle/kernel/process.h index bcb9ac4b81..459eedfa6d 100644 --- a/src/core/hle/kernel/process.h +++ b/src/core/hle/kernel/process.h @@ -262,8 +262,7 @@ public: ResultVal HeapAllocate(VAddr target, u64 size, VMAPermission perms); ResultCode HeapFree(VAddr target, u32 size); - ResultCode MirrorMemory(VAddr dst_addr, VAddr src_addr, u64 size, - MemoryState state = MemoryState::Mapped); + ResultCode MirrorMemory(VAddr dst_addr, VAddr src_addr, u64 size, MemoryState state); ResultCode UnmapMemory(VAddr dst_addr, VAddr src_addr, u64 size); diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index f43c7201cc..4ae92ff9e0 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); diff --git a/src/core/hle/kernel/vm_manager.cpp b/src/core/hle/kernel/vm_manager.cpp index 6187993ce3..e0f204b0b6 100644 --- a/src/core/hle/kernel/vm_manager.cpp +++ b/src/core/hle/kernel/vm_manager.cpp @@ -25,14 +25,14 @@ static const char* GetMemoryStateName(MemoryState state) { "CodeMutable", "Heap", "Shared", "Unknown1", "ModuleCodeStatic", "ModuleCodeMutable", - "IpcBuffer0", "Mapped", + "IpcBuffer0", "Stack", "ThreadLocal", "TransferMemoryIsolated", "TransferMemory", "ProcessMemory", "Unknown2", "IpcBuffer1", "IpcBuffer3", "KernelStack", }; - return names[static_cast(state)]; + return names[ToSvcMemoryState(state)]; } bool VirtualMemoryArea::CanBeMergedWith(const VirtualMemoryArea& next) const { diff --git a/src/core/hle/kernel/vm_manager.h b/src/core/hle/kernel/vm_manager.h index a12419d1e2..0a600c23ce 100644 --- a/src/core/hle/kernel/vm_manager.h +++ b/src/core/hle/kernel/vm_manager.h @@ -43,27 +43,112 @@ enum class VMAPermission : u8 { ReadWriteExecute = Read | Write | Execute, }; -/// Set of values returned in MemoryInfo.state by svcQueryMemory. +// clang-format off +/// Represents memory states and any relevant flags, as used by the kernel. +/// svcQueryMemory interprets these by masking away all but the first eight +/// bits when storing memory state into a MemoryInfo instance. enum class MemoryState : u32 { - Unmapped = 0x0, - Io = 0x1, - Normal = 0x2, - CodeStatic = 0x3, - CodeMutable = 0x4, - Heap = 0x5, - Shared = 0x6, - ModuleCodeStatic = 0x8, - ModuleCodeMutable = 0x9, - IpcBuffer0 = 0xA, - Mapped = 0xB, - ThreadLocal = 0xC, - TransferMemoryIsolated = 0xD, - TransferMemory = 0xE, - ProcessMemory = 0xF, - IpcBuffer1 = 0x11, - IpcBuffer3 = 0x12, - KernelStack = 0x13, + Mask = 0xFF, + FlagProtect = 1U << 8, + FlagDebug = 1U << 9, + FlagIPC0 = 1U << 10, + FlagIPC3 = 1U << 11, + FlagIPC1 = 1U << 12, + FlagMapped = 1U << 13, + FlagCode = 1U << 14, + FlagAlias = 1U << 15, + FlagModule = 1U << 16, + FlagTransfer = 1U << 17, + FlagQueryPhysicalAddressAllowed = 1U << 18, + FlagSharedDevice = 1U << 19, + FlagSharedDeviceAligned = 1U << 20, + FlagIPCBuffer = 1U << 21, + FlagMemoryPoolAllocated = 1U << 22, + FlagMapProcess = 1U << 23, + FlagUncached = 1U << 24, + FlagCodeMemory = 1U << 25, + + // Convenience flag sets to reduce repetition + IPCFlags = FlagIPC0 | FlagIPC3 | FlagIPC1, + + CodeFlags = FlagDebug | IPCFlags | FlagMapped | FlagCode | FlagQueryPhysicalAddressAllowed | + FlagSharedDevice | FlagSharedDeviceAligned | FlagMemoryPoolAllocated, + + DataFlags = FlagProtect | IPCFlags | FlagMapped | FlagAlias | FlagTransfer | + FlagQueryPhysicalAddressAllowed | FlagSharedDevice | FlagSharedDeviceAligned | + FlagMemoryPoolAllocated | FlagIPCBuffer | FlagUncached, + + Unmapped = 0x00, + Io = 0x01 | FlagMapped, + Normal = 0x02 | FlagMapped | FlagQueryPhysicalAddressAllowed, + CodeStatic = 0x03 | CodeFlags | FlagMapProcess, + CodeMutable = 0x04 | CodeFlags | FlagMapProcess | FlagCodeMemory, + Heap = 0x05 | DataFlags | FlagCodeMemory, + Shared = 0x06 | FlagMapped | FlagMemoryPoolAllocated, + ModuleCodeStatic = 0x08 | CodeFlags | FlagModule | FlagMapProcess, + ModuleCodeMutable = 0x09 | DataFlags | FlagModule | FlagMapProcess | FlagCodeMemory, + + IpcBuffer0 = 0x0A | FlagMapped | FlagQueryPhysicalAddressAllowed | FlagMemoryPoolAllocated | + IPCFlags | FlagSharedDevice | FlagSharedDeviceAligned, + + Stack = 0x0B | FlagMapped | IPCFlags | FlagQueryPhysicalAddressAllowed | + FlagSharedDevice | FlagSharedDeviceAligned | FlagMemoryPoolAllocated, + + ThreadLocal = 0x0C | FlagMapped | FlagMemoryPoolAllocated, + + TransferMemoryIsolated = 0x0D | IPCFlags | FlagMapped | FlagQueryPhysicalAddressAllowed | + FlagSharedDevice | FlagSharedDeviceAligned | FlagMemoryPoolAllocated | + FlagUncached, + + TransferMemory = 0x0E | FlagIPC3 | FlagIPC1 | FlagMapped | FlagQueryPhysicalAddressAllowed | + FlagSharedDevice | FlagSharedDeviceAligned | FlagMemoryPoolAllocated, + + ProcessMemory = 0x0F | FlagIPC3 | FlagIPC1 | FlagMapped | FlagMemoryPoolAllocated, + + IpcBuffer1 = 0x11 | FlagIPC3 | FlagIPC1 | FlagMapped | FlagQueryPhysicalAddressAllowed | + FlagSharedDevice | FlagSharedDeviceAligned | FlagMemoryPoolAllocated, + + IpcBuffer3 = 0x12 | FlagIPC3 | FlagMapped | FlagQueryPhysicalAddressAllowed | + FlagSharedDeviceAligned | FlagMemoryPoolAllocated, + + KernelStack = 0x13 | FlagMapped, }; +// clang-format on + +constexpr MemoryState operator|(MemoryState lhs, MemoryState rhs) { + return static_cast(u32(lhs) | u32(rhs)); +} + +constexpr MemoryState operator&(MemoryState lhs, MemoryState rhs) { + return static_cast(u32(lhs) & u32(rhs)); +} + +constexpr MemoryState operator^(MemoryState lhs, MemoryState rhs) { + return static_cast(u32(lhs) ^ u32(rhs)); +} + +constexpr MemoryState operator~(MemoryState lhs) { + return static_cast(~u32(lhs)); +} + +constexpr MemoryState& operator|=(MemoryState& lhs, MemoryState rhs) { + lhs = lhs | rhs; + return lhs; +} + +constexpr MemoryState& operator&=(MemoryState& lhs, MemoryState rhs) { + lhs = lhs & rhs; + return lhs; +} + +constexpr MemoryState& operator^=(MemoryState& lhs, MemoryState rhs) { + lhs = lhs ^ rhs; + return lhs; +} + +constexpr u32 ToSvcMemoryState(MemoryState state) { + return static_cast(state & MemoryState::Mask); +} /** * Represents a VMA in an address space. A VMA is a contiguous region of virtual addressing space @@ -186,8 +271,7 @@ public: ResultVal HeapAllocate(VAddr target, u64 size, VMAPermission perms); ResultCode HeapFree(VAddr target, u64 size); - ResultCode MirrorMemory(VAddr dst_addr, VAddr src_addr, u64 size, - MemoryState state = MemoryState::Mapped); + ResultCode MirrorMemory(VAddr dst_addr, VAddr src_addr, u64 size, MemoryState state); /** * Scans all VMAs and updates the page table range of any that use the given vector as backing From c02b8c895b49b511a4316ee5e7bf1ab9a081869b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 11:04:10 -0500 Subject: [PATCH 2/8] vm_manager: Migrate MemoryInfo and PageInfo to vm_manager.h Gets the two structures out of an unrelated header and places them with the rest of the memory management code. This also corrects the structures. PageInfo appears to only contain a 32-bit flags member, and the extra padding word in MemoryInfo isn't necessary. --- src/core/hle/kernel/svc.h | 16 ---------------- src/core/hle/kernel/svc_wrap.h | 2 +- src/core/hle/kernel/vm_manager.h | 15 +++++++++++++++ 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/core/hle/kernel/svc.h b/src/core/hle/kernel/svc.h index b06aac4ec4..c37ae0f98f 100644 --- a/src/core/hle/kernel/svc.h +++ b/src/core/hle/kernel/svc.h @@ -8,22 +8,6 @@ namespace Kernel { -struct MemoryInfo { - u64 base_address; - u64 size; - u32 type; - u32 attributes; - u32 permission; - u32 device_refcount; - u32 ipc_refcount; - INSERT_PADDING_WORDS(1); -}; -static_assert(sizeof(MemoryInfo) == 0x28, "MemoryInfo has incorrect size."); - -struct PageInfo { - u64 flags; -}; - void CallSVC(u32 immediate); } // namespace Kernel diff --git a/src/core/hle/kernel/svc_wrap.h b/src/core/hle/kernel/svc_wrap.h index 24aef46c93..3893b0f4a9 100644 --- a/src/core/hle/kernel/svc_wrap.h +++ b/src/core/hle/kernel/svc_wrap.h @@ -7,7 +7,7 @@ #include "common/common_types.h" #include "core/arm/arm_interface.h" #include "core/core.h" -#include "core/hle/kernel/svc.h" +#include "core/hle/kernel/vm_manager.h" #include "core/hle/result.h" #include "core/memory.h" diff --git a/src/core/hle/kernel/vm_manager.h b/src/core/hle/kernel/vm_manager.h index 0a600c23ce..35f0f276ea 100644 --- a/src/core/hle/kernel/vm_manager.h +++ b/src/core/hle/kernel/vm_manager.h @@ -150,6 +150,21 @@ constexpr u32 ToSvcMemoryState(MemoryState state) { return static_cast(state & MemoryState::Mask); } +struct MemoryInfo { + u64 base_address; + u64 size; + u32 type; + u32 attributes; + u32 permission; + u32 device_refcount; + u32 ipc_refcount; +}; +static_assert(sizeof(MemoryInfo) == 0x28, "MemoryInfo has incorrect size."); + +struct PageInfo { + u32 flags; +}; + /** * Represents a VMA in an address space. A VMA is a contiguous region of virtual addressing space * with homogeneous attributes across its extents. In this particular implementation each VMA is From a8cc03502b41b44150af71535d2b662a7ee3390c Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 11:34:01 -0500 Subject: [PATCH 3/8] 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 ++++---------------- src/core/hle/kernel/svc_wrap.h | 2 +- src/core/hle/kernel/vm_manager.cpp | 19 +++++++++++++++++++ src/core/hle/kernel/vm_manager.h | 10 +++++++++- 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 4ae92ff9e0..8b079bc402 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; } diff --git a/src/core/hle/kernel/svc_wrap.h b/src/core/hle/kernel/svc_wrap.h index 3893b0f4a9..27a11d82e2 100644 --- a/src/core/hle/kernel/svc_wrap.h +++ b/src/core/hle/kernel/svc_wrap.h @@ -199,7 +199,7 @@ void SvcWrap() { Memory::Write64(Param(0), memory_info.base_address); Memory::Write64(Param(0) + 8, memory_info.size); - Memory::Write32(Param(0) + 16, memory_info.type); + Memory::Write32(Param(0) + 16, memory_info.state); Memory::Write32(Param(0) + 20, memory_info.attributes); Memory::Write32(Param(0) + 24, memory_info.permission); diff --git a/src/core/hle/kernel/vm_manager.cpp b/src/core/hle/kernel/vm_manager.cpp index e0f204b0b6..21bcee192e 100644 --- a/src/core/hle/kernel/vm_manager.cpp +++ b/src/core/hle/kernel/vm_manager.cpp @@ -302,6 +302,25 @@ ResultCode VMManager::HeapFree(VAddr target, u64 size) { return RESULT_SUCCESS; } +MemoryInfo VMManager::QueryMemory(VAddr address) const { + const auto vma = FindVMA(address); + MemoryInfo memory_info{}; + + if (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.state = ToSvcMemoryState(vma->second.meminfo_state); + } else { + memory_info.base_address = 0; + memory_info.permission = static_cast(VMAPermission::None); + memory_info.size = 0; + memory_info.state = static_cast(MemoryState::Unmapped); + } + + return memory_info; +} + ResultCode VMManager::MirrorMemory(VAddr dst_addr, VAddr src_addr, u64 size, MemoryState state) { const auto vma = FindVMA(src_addr); diff --git a/src/core/hle/kernel/vm_manager.h b/src/core/hle/kernel/vm_manager.h index 35f0f276ea..91e8e8c8c4 100644 --- a/src/core/hle/kernel/vm_manager.h +++ b/src/core/hle/kernel/vm_manager.h @@ -153,7 +153,7 @@ constexpr u32 ToSvcMemoryState(MemoryState state) { struct MemoryInfo { u64 base_address; u64 size; - u32 type; + u32 state; u32 attributes; u32 permission; u32 device_refcount; @@ -288,6 +288,14 @@ public: ResultCode MirrorMemory(VAddr dst_addr, VAddr src_addr, u64 size, MemoryState state); + /// Queries the memory manager for information about the given address. + /// + /// @param address The address to query the memory manager about for information. + /// + /// @return A MemoryInfo instance containing information about the given address. + /// + MemoryInfo QueryMemory(VAddr address) const; + /** * Scans all VMAs and updates the page table range of any that use the given vector as backing * memory. This should be called after any operation that causes reallocation of the vector. From eb5f3f67f62534126e9598e9ea3e165238590b93 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 13:26:33 -0500 Subject: [PATCH 4/8] vm_manager: Amend the returned values for invalid memory queries in QueryMemory() The kernel returns a memory info instance with the base address set to the end of the address space, and the size of said block as 0 - address_space_end, it doesn't set both of said members to zero. --- src/core/hle/kernel/vm_manager.cpp | 8 ++++---- src/core/hle/kernel/vm_manager.h | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/core/hle/kernel/vm_manager.cpp b/src/core/hle/kernel/vm_manager.cpp index 21bcee192e..d3b55a51ec 100644 --- a/src/core/hle/kernel/vm_manager.cpp +++ b/src/core/hle/kernel/vm_manager.cpp @@ -28,7 +28,7 @@ static const char* GetMemoryStateName(MemoryState state) { "IpcBuffer0", "Stack", "ThreadLocal", "TransferMemoryIsolated", "TransferMemory", "ProcessMemory", - "Unknown2", "IpcBuffer1", + "Inaccessible", "IpcBuffer1", "IpcBuffer3", "KernelStack", }; @@ -312,10 +312,10 @@ MemoryInfo VMManager::QueryMemory(VAddr address) const { memory_info.size = vma->second.size; memory_info.state = ToSvcMemoryState(vma->second.meminfo_state); } else { - memory_info.base_address = 0; + memory_info.base_address = address_space_end; memory_info.permission = static_cast(VMAPermission::None); - memory_info.size = 0; - memory_info.state = static_cast(MemoryState::Unmapped); + memory_info.size = 0 - address_space_end; + memory_info.state = static_cast(MemoryState::Inaccessible); } return memory_info; diff --git a/src/core/hle/kernel/vm_manager.h b/src/core/hle/kernel/vm_manager.h index 91e8e8c8c4..7befa35269 100644 --- a/src/core/hle/kernel/vm_manager.h +++ b/src/core/hle/kernel/vm_manager.h @@ -105,6 +105,9 @@ enum class MemoryState : u32 { ProcessMemory = 0x0F | FlagIPC3 | FlagIPC1 | FlagMapped | FlagMemoryPoolAllocated, + // Used to signify an inaccessible or invalid memory region with memory queries + Inaccessible = 0x10, + IpcBuffer1 = 0x11 | FlagIPC3 | FlagIPC1 | FlagMapped | FlagQueryPhysicalAddressAllowed | FlagSharedDevice | FlagSharedDeviceAligned | FlagMemoryPoolAllocated, From b1b855c5d9eccae861b61652e24eeb24f1909bae Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 15:42:43 -0500 Subject: [PATCH 5/8] vm_manager: Correct ordering of last two struct members of MemoryInfo These should be swapped. --- src/core/hle/kernel/vm_manager.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/hle/kernel/vm_manager.h b/src/core/hle/kernel/vm_manager.h index 7befa35269..10bacac3e4 100644 --- a/src/core/hle/kernel/vm_manager.h +++ b/src/core/hle/kernel/vm_manager.h @@ -159,8 +159,8 @@ struct MemoryInfo { u32 state; u32 attributes; u32 permission; - u32 device_refcount; - u32 ipc_refcount; + u32 ipc_ref_count; + u32 device_ref_count; }; static_assert(sizeof(MemoryInfo) == 0x28, "MemoryInfo has incorrect size."); From d8deb39b83409f1d9b5eeec0a719d560e4409aae Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 11:48:06 -0500 Subject: [PATCH 6/8] 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(-) diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 8b079bc402..a1ecc4540b 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 27a11d82e2..f03b5438bf 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; From 09a219d5b4a45537502c219542a51a57d6d0c317 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 12:52:31 -0500 Subject: [PATCH 7/8] 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(+) diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index a1ecc4540b..8b51aa2c7c 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); From b79f08661337f22d46d519218c1cc6c013a14d2c Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 12 Dec 2018 13:42:21 -0500 Subject: [PATCH 8/8] 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 +- src/core/hle/kernel/svc_wrap.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 8b51aa2c7c..c1100f1972 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"}, diff --git a/src/core/hle/kernel/svc_wrap.h b/src/core/hle/kernel/svc_wrap.h index f03b5438bf..b762fd93ef 100644 --- a/src/core/hle/kernel/svc_wrap.h +++ b/src/core/hle/kernel/svc_wrap.h @@ -130,6 +130,11 @@ void SvcWrap() { func(Param(0), Param(1), static_cast(Param(3)), static_cast(Param(3))).raw); } +template +void SvcWrap() { + FuncReturn(func(Param(0), Param(1), static_cast(Param(2)), Param(3)).raw); +} + template void SvcWrap() { FuncReturn(func(static_cast(Param(0)), Param(1), static_cast(Param(2))).raw);