From 197dcf0b5e426993f760374353cafb07126d45b2 Mon Sep 17 00:00:00 2001 From: bunnei Date: Sat, 9 Mar 2019 14:06:51 -0500 Subject: [PATCH] memory_manager: Add protections for invalid GPU addresses. - Avoid a crash in Xenoblade Chronicles 2. --- src/video_core/memory_manager.cpp | 54 +++++++++++++++++++++---------- src/video_core/memory_manager.h | 15 +++++---- 2 files changed, 45 insertions(+), 24 deletions(-) diff --git a/src/video_core/memory_manager.cpp b/src/video_core/memory_manager.cpp index 4c7faa067..e8edf9b14 100644 --- a/src/video_core/memory_manager.cpp +++ b/src/video_core/memory_manager.cpp @@ -90,32 +90,44 @@ GPUVAddr MemoryManager::FindFreeRegion(GPUVAddr region_start, u64 size, u64 alig return std::max(base, vma_handle->second.base); } -std::optional MemoryManager::GpuToCpuAddress(GPUVAddr gpu_addr) { - VAddr cpu_addr = page_table.backing_addr[gpu_addr >> page_bits]; +bool MemoryManager::IsAddressValid(GPUVAddr addr) const { + return (addr >> page_bits) < page_table.pointers.size(); +} + +std::optional MemoryManager::GpuToCpuAddress(GPUVAddr addr) { + if (!IsAddressValid(addr)) { + return {}; + } + + VAddr cpu_addr = page_table.backing_addr[addr >> page_bits]; if (cpu_addr) { - return cpu_addr + (gpu_addr & page_mask); + return cpu_addr + (addr & page_mask); } return {}; } template -T MemoryManager::Read(GPUVAddr vaddr) { - const u8* page_pointer = page_table.pointers[vaddr >> page_bits]; +T MemoryManager::Read(GPUVAddr addr) { + if (!IsAddressValid(addr)) { + return {}; + } + + const u8* page_pointer = page_table.pointers[addr >> page_bits]; if (page_pointer) { // NOTE: Avoid adding any extra logic to this fast-path block T value; - std::memcpy(&value, &page_pointer[vaddr & page_mask], sizeof(T)); + std::memcpy(&value, &page_pointer[addr & page_mask], sizeof(T)); return value; } - Common::PageType type = page_table.attributes[vaddr >> page_bits]; + Common::PageType type = page_table.attributes[addr >> page_bits]; switch (type) { case Common::PageType::Unmapped: - LOG_ERROR(HW_GPU, "Unmapped Read{} @ 0x{:08X}", sizeof(T) * 8, vaddr); + LOG_ERROR(HW_GPU, "Unmapped Read{} @ 0x{:08X}", sizeof(T) * 8, addr); return 0; case Common::PageType::Memory: - ASSERT_MSG(false, "Mapped memory page without a pointer @ {:016X}", vaddr); + ASSERT_MSG(false, "Mapped memory page without a pointer @ {:016X}", addr); break; default: UNREACHABLE(); @@ -124,22 +136,26 @@ T MemoryManager::Read(GPUVAddr vaddr) { } template -void MemoryManager::Write(GPUVAddr vaddr, T data) { - u8* page_pointer = page_table.pointers[vaddr >> page_bits]; - if (page_pointer) { - // NOTE: Avoid adding any extra logic to this fast-path block - std::memcpy(&page_pointer[vaddr & page_mask], &data, sizeof(T)); +void MemoryManager::Write(GPUVAddr addr, T data) { + if (!IsAddressValid(addr)) { return; } - Common::PageType type = page_table.attributes[vaddr >> page_bits]; + u8* page_pointer = page_table.pointers[addr >> page_bits]; + if (page_pointer) { + // NOTE: Avoid adding any extra logic to this fast-path block + std::memcpy(&page_pointer[addr & page_mask], &data, sizeof(T)); + return; + } + + Common::PageType type = page_table.attributes[addr >> page_bits]; switch (type) { case Common::PageType::Unmapped: LOG_ERROR(HW_GPU, "Unmapped Write{} 0x{:08X} @ 0x{:016X}", sizeof(data) * 8, - static_cast(data), vaddr); + static_cast(data), addr); return; case Common::PageType::Memory: - ASSERT_MSG(false, "Mapped memory page without a pointer @ {:016X}", vaddr); + ASSERT_MSG(false, "Mapped memory page without a pointer @ {:016X}", addr); break; default: UNREACHABLE(); @@ -156,6 +172,10 @@ template void MemoryManager::Write(GPUVAddr addr, u32 data); template void MemoryManager::Write(GPUVAddr addr, u64 data); u8* MemoryManager::GetPointer(GPUVAddr addr) { + if (!IsAddressValid(addr)) { + return {}; + } + u8* page_pointer = page_table.pointers[addr >> page_bits]; if (page_pointer) { return page_pointer + (addr & page_mask); diff --git a/src/video_core/memory_manager.h b/src/video_core/memory_manager.h index ac1b42936..76fa3d916 100644 --- a/src/video_core/memory_manager.h +++ b/src/video_core/memory_manager.h @@ -46,19 +46,19 @@ public: MemoryManager(); GPUVAddr AllocateSpace(u64 size, u64 align); - GPUVAddr AllocateSpace(GPUVAddr gpu_addr, u64 size, u64 align); + GPUVAddr AllocateSpace(GPUVAddr addr, u64 size, u64 align); GPUVAddr MapBufferEx(GPUVAddr cpu_addr, u64 size); - GPUVAddr MapBufferEx(GPUVAddr cpu_addr, GPUVAddr gpu_addr, u64 size); - GPUVAddr UnmapBuffer(GPUVAddr gpu_addr, u64 size); - std::optional GpuToCpuAddress(GPUVAddr gpu_addr); + GPUVAddr MapBufferEx(GPUVAddr cpu_addr, GPUVAddr addr, u64 size); + GPUVAddr UnmapBuffer(GPUVAddr addr, u64 size); + std::optional GpuToCpuAddress(GPUVAddr addr); template - T Read(GPUVAddr vaddr); + T Read(GPUVAddr addr); template - void Write(GPUVAddr vaddr, T data); + void Write(GPUVAddr addr, T data); - u8* GetPointer(GPUVAddr vaddr); + u8* GetPointer(GPUVAddr addr); void ReadBlock(GPUVAddr src_addr, void* dest_buffer, std::size_t size); void WriteBlock(GPUVAddr dest_addr, const void* src_buffer, std::size_t size); @@ -69,6 +69,7 @@ private: using VMAHandle = VMAMap::const_iterator; using VMAIter = VMAMap::iterator; + bool IsAddressValid(GPUVAddr addr) const; void MapPages(GPUVAddr base, u64 size, u8* memory, Common::PageType type, VAddr backing_addr = 0); void MapMemoryRegion(GPUVAddr base, u64 size, u8* target, VAddr backing_addr);