From 18cf0b82d6f3ac4f3f83ee63cf76a0aeaa10c30a Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Mon, 19 Jul 2021 05:11:30 +0530 Subject: [PATCH] Fix KTransferMemory Destruction Behavior + Add Missing SVC Handle Logs KTransferMemory needs to be reset to RW on destruction unlike KSharedMemory which is simply freed on destruction, not emulating this behavior accurately leads to `deko_examples` from `switch-examples` to lead to a SEGFAULT on selecting an example as it expects the memory to be R/W while it ends up being freed instead --- app/src/main/cpp/skyline/kernel/svc.cpp | 14 +++--- .../skyline/kernel/types/KSharedMemory.cpp | 47 +++++++++++++++---- .../skyline/kernel/types/KTransferMemory.h | 1 + 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/app/src/main/cpp/skyline/kernel/svc.cpp b/app/src/main/cpp/skyline/kernel/svc.cpp index 58ff2b20..6879228a 100644 --- a/app/src/main/cpp/skyline/kernel/svc.cpp +++ b/app/src/main/cpp/skyline/kernel/svc.cpp @@ -474,7 +474,8 @@ namespace skyline::kernel::svc { void MapSharedMemory(const DeviceState &state) { try { - auto object{state.process->GetHandle(state.ctx->gpr.w0)}; + KHandle handle{state.ctx->gpr.w0}; + auto object{state.process->GetHandle(handle)}; auto pointer{reinterpret_cast(state.ctx->gpr.x1)}; if (!util::PageAligned(pointer)) { @@ -497,7 +498,7 @@ namespace skyline::kernel::svc { return; } - state.logger->Debug("Mapping shared memory at 0x{:X} - 0x{:X} (0x{:X} bytes) ({}{}{})", pointer, pointer + size, size, permission.r ? 'R' : '-', permission.w ? 'W' : '-', permission.x ? 'X' : '-'); + state.logger->Debug("Mapping shared memory (0x{:X}) at 0x{:X} - 0x{:X} (0x{:X} bytes) ({}{}{})", handle, pointer, pointer + size, size, permission.r ? 'R' : '-', permission.w ? 'W' : '-', permission.x ? 'X' : '-'); object->Map(pointer, size, permission); @@ -510,7 +511,8 @@ namespace skyline::kernel::svc { void UnmapSharedMemory(const DeviceState &state) { try { - auto object{state.process->GetHandle(state.ctx->gpr.w0)}; + KHandle handle{state.ctx->gpr.w0}; + auto object{state.process->GetHandle(handle)}; auto pointer{reinterpret_cast(state.ctx->gpr.x1)}; if (!util::PageAligned(pointer)) { @@ -526,7 +528,7 @@ namespace skyline::kernel::svc { return; } - state.logger->Debug("Unmapping shared memory at 0x{:X} - 0x{:X} (0x{:X} bytes)", pointer, pointer + size, size); + state.logger->Debug("Unmapping shared memory (0x{:X}) at 0x{:X} - 0x{:X} (0x{:X} bytes)", handle, pointer, pointer + size, size); object->Unmap(pointer, size); @@ -559,8 +561,8 @@ namespace skyline::kernel::svc { return; } - auto tmem{state.process->NewHandle(pointer, size, permission)}; - state.logger->Debug("Creating transfer memory at 0x{:X} - 0x{:X} (0x{:X} bytes) ({}{}{})", pointer, pointer + size, size, permission.r ? 'R' : '-', permission.w ? 'W' : '-', permission.x ? 'X' : '-'); + auto tmem{state.process->NewHandle(pointer, size, permission, permission.raw ? memory::states::TransferMemory : memory::states::TransferMemoryIsolated)}; + state.logger->Debug("Creating transfer memory (0x{:X}) at 0x{:X} - 0x{:X} (0x{:X} bytes) ({}{}{})", tmem.handle, pointer, pointer + size, size, permission.r ? 'R' : '-', permission.w ? 'W' : '-', permission.x ? 'X' : '-'); state.ctx->gpr.w0 = Result{}; state.ctx->gpr.w1 = tmem.handle; diff --git a/app/src/main/cpp/skyline/kernel/types/KSharedMemory.cpp b/app/src/main/cpp/skyline/kernel/types/KSharedMemory.cpp index c3bf5629..0e565e62 100644 --- a/app/src/main/cpp/skyline/kernel/types/KSharedMemory.cpp +++ b/app/src/main/cpp/skyline/kernel/types/KSharedMemory.cpp @@ -38,6 +38,9 @@ namespace skyline::kernel::type { .size = size, .permission = permission, .state = memoryState, + .attributes = memory::MemoryAttribute{ + .isBorrowed = objectType == KType::KTransferMemory, + }, }); return guest.ptr; @@ -76,23 +79,49 @@ namespace skyline::kernel::type { .size = size, .permission = permission, .state = memoryState, + .attributes = memory::MemoryAttribute{ + .isBorrowed = objectType == KType::KTransferMemory, + }, }); } } KSharedMemory::~KSharedMemory() { + if (state.process && guest.Valid()) { + if (objectType != KType::KTransferMemory) { + mmap(guest.ptr, guest.size, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); // It doesn't particularly matter if this fails as it shouldn't really affect anything + state.process->memory.InsertChunk(ChunkDescriptor{ + .ptr = guest.ptr, + .size = guest.size, + .state = memory::states::Unmapped, + }); + } else { + // KTransferMemory remaps the region with R/W permissions during destruction + constexpr memory::Permission UnborrowPermission{true, true, false}; + + if (mmap(guest.ptr, guest.size, UnborrowPermission.Get(), MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0) == MAP_FAILED) + // It is likely that these exceptions will end up as asserts as the destructor is implicitly noexcept but we want to be notified about these not working so that's fine as it can be discovered in the debugger + throw exception("An error occurred while remapping transfer memory as anonymous memory in guest: {}", strerror(errno)); + else if (!host.Valid()) + throw exception("Expected host mapping of transfer memory to be valid during KTransferMemory destruction"); + + std::memcpy(guest.ptr, host.ptr, host.size); + + state.process->memory.InsertChunk(ChunkDescriptor{ + .ptr = guest.ptr, + .size = guest.size, + .permission = UnborrowPermission, + .state = memoryState, + .attributes = memory::MemoryAttribute{ + .isBorrowed = false, + } + }); + } + } + if (host.Valid()) munmap(host.ptr, host.size); - if (state.process && guest.Valid()) { - mmap(guest.ptr, guest.size, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); // As this is the destructor, we cannot throw on this failing - state.process->memory.InsertChunk(ChunkDescriptor{ - .ptr = guest.ptr, - .size = guest.size, - .state = memory::states::Unmapped, - }); - } - close(fd); } } diff --git a/app/src/main/cpp/skyline/kernel/types/KTransferMemory.h b/app/src/main/cpp/skyline/kernel/types/KTransferMemory.h index 1e3047f6..95b041fe 100644 --- a/app/src/main/cpp/skyline/kernel/types/KTransferMemory.h +++ b/app/src/main/cpp/skyline/kernel/types/KTransferMemory.h @@ -8,6 +8,7 @@ namespace skyline::kernel::type { /** * @brief KTransferMemory is used to transfer memory from one application to another on HOS, we emulate this abstraction using KSharedMemory as it's essentially the same with the main difference being that KSharedMemory is allocated by the kernel while KTransferMemory is created from memory that's been allocated by the guest beforehand + * @note KSharedMemory::{Map, Unmap, ~KSharedMemory} contains code to handle differences in memory attributes and destruction */ class KTransferMemory : public KSharedMemory { public: