From 5dd2ddb1df03cb174c96db1c61a0ee01f7c46fa7 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Mon, 20 Dec 2021 21:41:52 +0100 Subject: [PATCH] Memmap: Replace some GetPointer calls These GetPointer calls could cause crashes, in part because the callers didn't do null checks and in part because GetPointer inherently is unsafe to use for accesses larger than 1 byte. --- Source/Core/Core/HW/Memmap.cpp | 36 +++++++++++++++++++++++----------- Source/Core/Core/HW/Memmap.h | 5 +++-- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/Source/Core/Core/HW/Memmap.cpp b/Source/Core/Core/HW/Memmap.cpp index 0dbd22bc33..2a20f9a569 100644 --- a/Source/Core/Core/HW/Memmap.cpp +++ b/Source/Core/Core/HW/Memmap.cpp @@ -466,16 +466,22 @@ void Clear() memset(m_pEXRAM, 0, GetExRamSize()); } -static inline u8* GetPointerForRange(u32 address, size_t size) +u8* GetPointerForRange(u32 address, size_t size) { // Make sure we don't have a range spanning 2 separate banks if (size >= GetExRamSizeReal()) + { + PanicAlertFmt("Oversized range in GetPointerForRange. {:x} bytes at {:#010x}", size, address); return nullptr; + } // Check that the beginning and end of the range are valid u8* pointer = GetPointer(address); if (!pointer || !GetPointer(address + u32(size) - 1)) + { + // A panic alert has already been raised by GetPointer return nullptr; + } return pointer; } @@ -559,55 +565,63 @@ u8* GetPointer(u32 address) u8 Read_U8(u32 address) { - return *GetPointer(address); + u8 value = 0; + CopyFromEmu(&value, address, sizeof(value)); + return value; } u16 Read_U16(u32 address) { - return Common::swap16(GetPointer(address)); + u16 value = 0; + CopyFromEmu(&value, address, sizeof(value)); + return Common::swap16(value); } u32 Read_U32(u32 address) { - return Common::swap32(GetPointer(address)); + u32 value = 0; + CopyFromEmu(&value, address, sizeof(value)); + return Common::swap32(value); } u64 Read_U64(u32 address) { - return Common::swap64(GetPointer(address)); + u64 value = 0; + CopyFromEmu(&value, address, sizeof(value)); + return Common::swap64(value); } void Write_U8(u8 value, u32 address) { - *GetPointer(address) = value; + CopyToEmu(address, &value, sizeof(value)); } void Write_U16(u16 value, u32 address) { u16 swapped_value = Common::swap16(value); - std::memcpy(GetPointer(address), &swapped_value, sizeof(u16)); + CopyToEmu(address, &swapped_value, sizeof(swapped_value)); } void Write_U32(u32 value, u32 address) { u32 swapped_value = Common::swap32(value); - std::memcpy(GetPointer(address), &swapped_value, sizeof(u32)); + CopyToEmu(address, &swapped_value, sizeof(swapped_value)); } void Write_U64(u64 value, u32 address) { u64 swapped_value = Common::swap64(value); - std::memcpy(GetPointer(address), &swapped_value, sizeof(u64)); + CopyToEmu(address, &swapped_value, sizeof(swapped_value)); } void Write_U32_Swap(u32 value, u32 address) { - std::memcpy(GetPointer(address), &value, sizeof(u32)); + CopyToEmu(address, &value, sizeof(value)); } void Write_U64_Swap(u64 value, u32 address) { - std::memcpy(GetPointer(address), &value, sizeof(u64)); + CopyToEmu(address, &value, sizeof(value)); } } // namespace Memory diff --git a/Source/Core/Core/HW/Memmap.h b/Source/Core/Core/HW/Memmap.h index a8439bfea0..14ed19291f 100644 --- a/Source/Core/Core/HW/Memmap.h +++ b/Source/Core/Core/HW/Memmap.h @@ -74,6 +74,7 @@ void Clear(); // emulated hardware outside the CPU. Use "Device_" prefix. std::string GetString(u32 em_address, size_t size = 0); u8* GetPointer(u32 address); +u8* GetPointerForRange(u32 address, size_t size); void CopyFromEmu(void* data, u32 address, size_t size); void CopyToEmu(u32 address, const void* data, size_t size); void Memset(u32 address, u8 value, size_t size); @@ -92,7 +93,7 @@ void Write_U64_Swap(u64 var, u32 address); template void CopyFromEmuSwapped(T* data, u32 address, size_t size) { - const T* src = reinterpret_cast(GetPointer(address)); + const T* src = reinterpret_cast(GetPointerForRange(address, size)); if (src == nullptr) return; @@ -104,7 +105,7 @@ void CopyFromEmuSwapped(T* data, u32 address, size_t size) template void CopyToEmuSwapped(u32 address, const T* data, size_t size) { - T* dest = reinterpret_cast(GetPointer(address)); + T* dest = reinterpret_cast(GetPointerForRange(address, size)); if (dest == nullptr) return;