From 7bd2faad9a41a04d81e5b33d454ca01d9eb650e0 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 13 Sep 2018 19:09:04 -0400 Subject: kernel/svc: Sanitize heap sizes within svcSetHeapSize() The kernel checks if the given size is a multiple of 2MB and <= to 4GB before going ahead and attempting to allocate that much memory. --- src/core/hle/kernel/errors.h | 2 ++ src/core/hle/kernel/svc.cpp | 6 ++++++ 2 files changed, 8 insertions(+) (limited to 'src/core') diff --git a/src/core/hle/kernel/errors.h b/src/core/hle/kernel/errors.h index ad39c8271..2be2fad82 100644 --- a/src/core/hle/kernel/errors.h +++ b/src/core/hle/kernel/errors.h @@ -17,6 +17,7 @@ enum { // Confirmed Switch OS error codes MaxConnectionsReached = 7, + InvalidSize = 101, InvalidAddress = 102, HandleTableFull = 105, InvalidMemoryState = 106, @@ -55,6 +56,7 @@ constexpr ResultCode ERR_INVALID_MEMORY_PERMISSIONS(ErrorModule::Kernel, ErrCodes::InvalidMemoryPermissions); constexpr ResultCode ERR_INVALID_HANDLE(ErrorModule::Kernel, ErrCodes::InvalidHandle); constexpr ResultCode ERR_INVALID_PROCESSOR_ID(ErrorModule::Kernel, ErrCodes::InvalidProcessorId); +constexpr ResultCode ERR_INVALID_SIZE(ErrorModule::Kernel, ErrCodes::InvalidSize); constexpr ResultCode ERR_INVALID_STATE(ErrorModule::Kernel, ErrCodes::InvalidState); constexpr ResultCode ERR_INVALID_THREAD_PRIORITY(ErrorModule::Kernel, ErrCodes::InvalidThreadPriority); diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index f500fd2e7..a3d169e46 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -39,6 +39,12 @@ namespace Kernel { /// Set the process heap to a given Size. It can both extend and shrink the heap. static ResultCode SetHeapSize(VAddr* heap_addr, u64 heap_size) { LOG_TRACE(Kernel_SVC, "called, heap_size=0x{:X}", heap_size); + + // Size must be a multiple of 0x200000 (2MB) and be equal to or less than 4GB. + if ((heap_size & 0xFFFFFFFE001FFFFF) != 0) { + return ERR_INVALID_SIZE; + } + auto& process = *Core::CurrentProcess(); CASCADE_RESULT(*heap_addr, process.HeapAllocate(Memory::HEAP_VADDR, heap_size, VMAPermission::ReadWrite)); -- cgit v1.2.3 From 496c67fd730cd27ed1a6ce087d224bd2b736ad4b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 13 Sep 2018 19:14:50 -0400 Subject: kernel/svc: Sanitize addresses and sizes within svcMapMemory() and svcUnmapMemory() The kernel checks if the addresses and given size is 4KB aligned before continuing onwards to map the memory. --- src/core/hle/kernel/svc.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'src/core') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index a3d169e46..3eb77812e 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -35,6 +35,11 @@ #include "core/hle/service/service.h" namespace Kernel { +namespace { +constexpr bool Is4KBAligned(VAddr address) { + return (address & 0xFFF) == 0; +} +} // Anonymous namespace /// Set the process heap to a given Size. It can both extend and shrink the heap. static ResultCode SetHeapSize(VAddr* heap_addr, u64 heap_size) { @@ -62,6 +67,15 @@ static ResultCode SetMemoryAttribute(VAddr addr, u64 size, u32 state0, u32 state static ResultCode MapMemory(VAddr dst_addr, VAddr src_addr, u64 size) { LOG_TRACE(Kernel_SVC, "called, dst_addr=0x{:X}, src_addr=0x{:X}, size=0x{:X}", dst_addr, src_addr, size); + + if (!Is4KBAligned(dst_addr) || !Is4KBAligned(src_addr)) { + return ERR_INVALID_ADDRESS; + } + + if (size == 0 || !Is4KBAligned(size)) { + return ERR_INVALID_SIZE; + } + return Core::CurrentProcess()->MirrorMemory(dst_addr, src_addr, size); } @@ -69,6 +83,15 @@ static ResultCode MapMemory(VAddr dst_addr, VAddr src_addr, u64 size) { static ResultCode UnmapMemory(VAddr dst_addr, VAddr src_addr, u64 size) { LOG_TRACE(Kernel_SVC, "called, dst_addr=0x{:X}, src_addr=0x{:X}, size=0x{:X}", dst_addr, src_addr, size); + + if (!Is4KBAligned(dst_addr) || !Is4KBAligned(src_addr)) { + return ERR_INVALID_ADDRESS; + } + + if (size == 0 || !Is4KBAligned(size)) { + return ERR_INVALID_SIZE; + } + return Core::CurrentProcess()->UnmapMemory(dst_addr, src_addr, size); } -- cgit v1.2.3 From accd1f17e451dfe23350055b5c1377846f3dce77 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 13 Sep 2018 20:16:43 -0400 Subject: kernel/svc: Sanitize addresses, permissions, and sizes within svcMapSharedMemory() and svcUnmapSharedMemory() Part of the checking done by the kernel is to check if the given address and size are 4KB aligned, as well as checking if the size isn't zero. It also only allows mapping shared memory as readable or read/write, but nothing else, and so we shouldn't allow mapping as anything else either. --- src/core/hle/kernel/svc.cpp | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) (limited to 'src/core') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 3eb77812e..d19182639 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -444,35 +444,43 @@ static ResultCode MapSharedMemory(Handle shared_memory_handle, VAddr addr, u64 s "called, shared_memory_handle=0x{:X}, addr=0x{:X}, size=0x{:X}, permissions=0x{:08X}", shared_memory_handle, addr, size, permissions); + if (!Is4KBAligned(addr)) { + return ERR_INVALID_ADDRESS; + } + + if (size == 0 || !Is4KBAligned(size)) { + return ERR_INVALID_SIZE; + } + + const auto permissions_type = static_cast(permissions); + if (permissions_type != MemoryPermission::Read && + permissions_type != MemoryPermission::ReadWrite) { + LOG_ERROR(Kernel_SVC, "Invalid permissions=0x{:08X}", permissions); + return ERR_INVALID_MEMORY_PERMISSIONS; + } + auto& kernel = Core::System::GetInstance().Kernel(); auto shared_memory = kernel.HandleTable().Get(shared_memory_handle); if (!shared_memory) { return ERR_INVALID_HANDLE; } - MemoryPermission permissions_type = static_cast(permissions); - switch (permissions_type) { - case MemoryPermission::Read: - case MemoryPermission::Write: - case MemoryPermission::ReadWrite: - case MemoryPermission::Execute: - case MemoryPermission::ReadExecute: - case MemoryPermission::WriteExecute: - case MemoryPermission::ReadWriteExecute: - case MemoryPermission::DontCare: - return shared_memory->Map(Core::CurrentProcess().get(), addr, permissions_type, - MemoryPermission::DontCare); - default: - LOG_ERROR(Kernel_SVC, "unknown permissions=0x{:08X}", permissions); - } - - return RESULT_SUCCESS; + return shared_memory->Map(Core::CurrentProcess().get(), addr, permissions_type, + MemoryPermission::DontCare); } static ResultCode UnmapSharedMemory(Handle shared_memory_handle, VAddr addr, u64 size) { LOG_WARNING(Kernel_SVC, "called, shared_memory_handle=0x{:08X}, addr=0x{:X}, size=0x{:X}", shared_memory_handle, addr, size); + if (!Is4KBAligned(addr)) { + return ERR_INVALID_ADDRESS; + } + + if (size == 0 || !Is4KBAligned(size)) { + return ERR_INVALID_SIZE; + } + auto& kernel = Core::System::GetInstance().Kernel(); auto shared_memory = kernel.HandleTable().Get(shared_memory_handle); -- cgit v1.2.3 From 4f8756edd06d76af33208047a5ed9b776132d97a Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 13 Sep 2018 21:04:43 -0400 Subject: kernel/svc: Sanitize creation of shared memory via svcCreateSharedMemory() The kernel caps the size limit of shared memory to 8589930496 bytes (or (1GB - 512 bytes) * 8), so approximately 8GB, where every GB has a 512 byte sector taken off of it. It also ensures the shared memory is created with either read or read/write permissions for both permission types passed in, allowing the remote permissions to also be set as "don't care". --- src/core/hle/kernel/svc.cpp | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'src/core') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index d19182639..4529002ba 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -935,12 +935,28 @@ static ResultCode CreateSharedMemory(Handle* handle, u64 size, u32 local_permiss LOG_TRACE(Kernel_SVC, "called, size=0x{:X}, localPerms=0x{:08X}, remotePerms=0x{:08X}", size, local_permissions, remote_permissions); + // Size must be a multiple of 4KB and be less than or equal to + // approx. 8 GB (actually (1GB - 512B) * 8) + if (size == 0 || (size & 0xFFFFFFFE00000FFF) != 0) { + return ERR_INVALID_SIZE; + } + + const auto local_perms = static_cast(local_permissions); + if (local_perms != MemoryPermission::Read && local_perms != MemoryPermission::ReadWrite) { + return ERR_INVALID_MEMORY_PERMISSIONS; + } + + const auto remote_perms = static_cast(remote_permissions); + if (remote_perms != MemoryPermission::Read && remote_perms != MemoryPermission::ReadWrite && + remote_perms != MemoryPermission::DontCare) { + return ERR_INVALID_MEMORY_PERMISSIONS; + } + auto& kernel = Core::System::GetInstance().Kernel(); auto& handle_table = kernel.HandleTable(); auto shared_mem_handle = SharedMemory::Create(kernel, handle_table.Get(KernelHandle::CurrentProcess), size, - static_cast(local_permissions), - static_cast(remote_permissions)); + local_perms, remote_perms); CASCADE_RESULT(*handle, handle_table.Create(shared_mem_handle)); return RESULT_SUCCESS; -- cgit v1.2.3