[Memory] Add alignment guards to PhysicalHeap allocations

Add alignment checks and parent release on failure in Alloc, AllocFixed,
and AllocRange to prevent misaligned addresses reaching BaseHeap.

Fixes #954
This commit is contained in:
Herman S.
2026-03-31 18:00:26 +09:00
parent d2c8ca675e
commit 11ad6b0a87
3 changed files with 243 additions and 1 deletions

View File

@@ -1,3 +1,3 @@
xe_test_suite(xenia-base-tests ${CMAKE_CURRENT_SOURCE_DIR} xe_test_suite(xenia-base-tests ${CMAKE_CURRENT_SOURCE_DIR}
LINKS fmt xenia-base LINKS fmt xenia-base xenia-core xenia-cpu
) )

View File

@@ -0,0 +1,211 @@
/**
******************************************************************************
* Xenia : Xbox 360 Emulator Research Project *
******************************************************************************
* Copyright 2026 Ben Vanik. All rights reserved. *
* Released under the BSD license - see LICENSE in the root for more details. *
******************************************************************************
*/
#include "xenia/memory.h"
#include "third_party/catch/include/catch.hpp"
#include "xenia/base/memory.h"
namespace xe {
namespace test {
// All tests use kMemoryAllocationReserve which only touches the page table,
// not host memory. This lets us pass nullptr for membase and Memory*.
TEST_CASE("PhysicalHeap::GetPhysicalAddress", "[memory]") {
VirtualHeap parent;
parent.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0x00000000,
0x20000000, 4096);
SECTION("heap with no offset returns heap-relative address") {
PhysicalHeap heap;
heap.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0xA0000000,
0x20000000, 64 * 1024, &parent);
REQUIRE(heap.host_address_offset() == 0);
REQUIRE(heap.GetPhysicalAddress(0xA0000000) == 0);
REQUIRE(heap.GetPhysicalAddress(0xA0010000) == 0x10000);
REQUIRE(heap.GetPhysicalAddress(0xA1000000) == 0x1000000);
}
SECTION("0xE0000000 heap always has 0x1000 physical offset") {
PhysicalHeap heap;
heap.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0xE0000000,
0x1FD00000, 4096, &parent);
// The 0x1000 physical offset is baked into the view mapping
// (map_info target_address), not derived from host_address_offset.
REQUIRE(heap.GetPhysicalAddress(0xE0000000) == 0x1000);
REQUIRE(heap.GetPhysicalAddress(0xE0001000) == 0x2000);
REQUIRE(heap.GetPhysicalAddress(0xE0010000) == 0x11000);
}
}
TEST_CASE("PhysicalHeap::Alloc alignment", "[memory]") {
VirtualHeap parent;
parent.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0x00000000,
0x20000000, 4096);
SECTION("returned address is page-aligned") {
PhysicalHeap heap;
heap.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0xA0000000,
0x20000000, 64 * 1024, &parent);
uint32_t addr = 0;
bool ok = heap.Alloc(0x10000, 0x10000, kMemoryAllocationReserve,
kMemoryProtectRead, false, &addr);
REQUIRE(ok);
REQUIRE(addr != 0);
REQUIRE(addr % 0x10000 == 0);
REQUIRE(addr >= 0xA0000000);
REQUIRE(addr < 0xC0000000);
}
SECTION("multiple allocations with different alignments") {
PhysicalHeap heap;
heap.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0xA0000000,
0x20000000, 64 * 1024, &parent);
for (uint32_t alignment : {0x10000u, 0x20000u, 0x40000u, 0x100000u}) {
uint32_t addr = 0;
bool ok = heap.Alloc(alignment, alignment, kMemoryAllocationReserve,
kMemoryProtectRead, false, &addr);
REQUIRE(ok);
REQUIRE(addr % alignment == 0);
}
}
}
TEST_CASE("PhysicalHeap::AllocRange alignment", "[memory]") {
VirtualHeap parent;
parent.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0x00000000,
0x20000000, 4096);
SECTION("returned address respects alignment within range") {
PhysicalHeap heap;
heap.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0xA0000000,
0x20000000, 64 * 1024, &parent);
uint32_t addr = 0;
bool ok = heap.AllocRange(0xA0000000, 0xBFFFFFFF, 0x10000, 0x10000,
kMemoryAllocationReserve, kMemoryProtectRead,
false, &addr);
REQUIRE(ok);
REQUIRE(addr % 0x10000 == 0);
REQUIRE(addr >= 0xA0000000);
REQUIRE(addr <= 0xBFFFFFFF);
}
SECTION("large alignment preserved through translation") {
PhysicalHeap heap;
heap.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0xC0000000,
0x20000000, 16 * 1024 * 1024, &parent);
uint32_t addr = 0;
bool ok = heap.AllocRange(0xC0000000, 0xDFFFFFFF, 0x1000000, 0x1000000,
kMemoryAllocationReserve, kMemoryProtectRead,
false, &addr);
REQUIRE(ok);
REQUIRE(addr % 0x1000000 == 0);
}
}
TEST_CASE("PhysicalHeap::AllocFixed alignment", "[memory]") {
VirtualHeap parent;
parent.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0x00000000,
0x20000000, 4096);
PhysicalHeap heap;
heap.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0xA0000000,
0x20000000, 64 * 1024, &parent);
// AllocFixed at a specific aligned address must succeed
bool ok = heap.AllocFixed(0xA0100000, 0x10000, 0x10000,
kMemoryAllocationReserve, kMemoryProtectRead);
REQUIRE(ok);
}
TEST_CASE("PhysicalHeap vE0000000 alignment", "[memory]") {
VirtualHeap parent;
parent.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0x00000000,
0x20000000, 4096);
PhysicalHeap heap;
heap.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0xE0000000,
0x1FD00000, 4096, &parent);
// The 0xE0000000 heap always has a 0x1000 physical offset, so the
// translation offset is 0xE0000000 - 0x1000 = 0xDFFFF000, which is
// 4KB-aligned but not 64KB-aligned. This is true on all platforms.
uint32_t physical_base = heap.GetPhysicalAddress(heap.heap_base());
REQUIRE(physical_base == 0x1000);
SECTION("page-size allocation preserves alignment") {
uint32_t addr = 0;
bool ok = heap.Alloc(0x1000, 0x1000, kMemoryAllocationReserve,
kMemoryProtectRead, false, &addr);
REQUIRE(ok);
REQUIRE(addr % 0x1000 == 0);
REQUIRE(addr >= 0xE0000000);
}
SECTION("translation offset is 4KB-aligned") {
uint32_t translation_offset = heap.heap_base() - physical_base;
REQUIRE(translation_offset % heap.page_size() == 0);
}
SECTION("alloc with alignment larger than page_size is rejected") {
// The translation offset (0xDFFFF000) is only 4KB-aligned, so a
// 64KB alignment request produces a misaligned translated address.
//
// Without the fix: BaseHeap::AllocFixed receives a misaligned address,
// hitting assert_true in debug or silently corrupting in release.
// With the fix: PhysicalHeap::Alloc detects the misalignment and
// returns false cleanly.
uint32_t alignment = 0x10000; // 64KB
uint32_t addr = 0;
bool ok = heap.Alloc(0x10000, alignment, kMemoryAllocationReserve,
kMemoryProtectRead, false, &addr);
REQUIRE_FALSE(ok);
}
}
TEST_CASE("PhysicalHeap vE0000000 AllocRange alignment", "[memory]") {
VirtualHeap parent;
parent.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0x00000000,
0x20000000, 4096);
PhysicalHeap heap;
heap.Initialize(nullptr, nullptr, HeapType::kGuestPhysical, 0xE0000000,
0x1FD00000, 4096, &parent);
SECTION("page-aligned AllocRange succeeds") {
uint32_t addr = 0;
bool ok = heap.AllocRange(0xE0000000, 0xFFFCFFFF, 0x1000, 0x1000,
kMemoryAllocationReserve, kMemoryProtectRead,
false, &addr);
REQUIRE(ok);
REQUIRE(addr % 0x1000 == 0);
}
SECTION("AllocRange rejects misaligned translation") {
// Same scenario as Alloc: 64KB alignment on a heap whose translation
// offset (0xDFFFF000) is not 64KB-aligned.
uint32_t alignment = 0x10000;
uint32_t addr = 0;
bool ok = heap.AllocRange(0xE0000000, 0xFFFCFFFF, 0x10000, alignment,
kMemoryAllocationReserve, kMemoryProtectRead,
false, &addr);
REQUIRE_FALSE(ok);
}
}
} // namespace test
} // namespace xe

View File

@@ -1645,6 +1645,13 @@ void PhysicalHeap::Initialize(Memory* memory, uint8_t* membase,
BaseHeap::Initialize(memory, membase, heap_type, heap_base, heap_size, BaseHeap::Initialize(memory, membase, heap_type, heap_base, heap_size,
page_size, host_address_offset); page_size, host_address_offset);
parent_heap_ = parent_heap; parent_heap_ = parent_heap;
// The physical base offset (host_address_offset) must be a multiple of
// page_size. Otherwise, aligned parent allocations become misaligned after
// translation back to virtual addresses (parent_address + heap_base_ -
// GetPhysicalAddress(heap_base_) loses alignment).
xenia_assert(host_address_offset % page_size == 0);
system_page_size_ = uint32_t(xe::memory::page_size()); system_page_size_ = uint32_t(xe::memory::page_size());
xenia_assert(xe::is_pow2(system_page_size_)); xenia_assert(xe::is_pow2(system_page_size_));
system_page_shift_ = xe::log2_floor(system_page_size_); system_page_shift_ = xe::log2_floor(system_page_size_);
@@ -1685,6 +1692,14 @@ bool PhysicalHeap::Alloc(uint32_t size, uint32_t alignment,
// Given the address we've reserved in the parent heap, pin that here. // Given the address we've reserved in the parent heap, pin that here.
// Shouldn't be possible for it to be allocated already. // Shouldn't be possible for it to be allocated already.
uint32_t address = heap_base_ + parent_address - parent_heap_start; uint32_t address = heap_base_ + parent_address - parent_heap_start;
if (address % alignment != 0) {
XELOGE(
"PhysicalHeap::Alloc translated address {:08X} misaligned "
"(alignment {:08X}, physical base offset {:08X})",
address, alignment, parent_heap_start);
parent_heap_->Release(parent_address);
return false;
}
if (!BaseHeap::AllocFixed(address, size, alignment, allocation_type, if (!BaseHeap::AllocFixed(address, size, alignment, allocation_type,
protect)) { protect)) {
XELOGE( XELOGE(
@@ -1721,6 +1736,14 @@ bool PhysicalHeap::AllocFixed(uint32_t base_address, uint32_t size,
// Shouldn't be possible for it to be allocated already. // Shouldn't be possible for it to be allocated already.
uint32_t address = uint32_t address =
heap_base_ + parent_base_address - GetPhysicalAddress(heap_base_); heap_base_ + parent_base_address - GetPhysicalAddress(heap_base_);
if (address % alignment != 0) {
XELOGE(
"PhysicalHeap::AllocFixed translated address {:08X} misaligned "
"(alignment {:08X}, physical base offset {:08X})",
address, alignment, GetPhysicalAddress(heap_base_));
parent_heap_->Release(parent_base_address);
return false;
}
if (!BaseHeap::AllocFixed(address, size, alignment, allocation_type, if (!BaseHeap::AllocFixed(address, size, alignment, allocation_type,
protect)) { protect)) {
XELOGE( XELOGE(
@@ -1762,6 +1785,14 @@ bool PhysicalHeap::AllocRange(uint32_t low_address, uint32_t high_address,
// Shouldn't be possible for it to be allocated already. // Shouldn't be possible for it to be allocated already.
uint32_t address = uint32_t address =
heap_base_ + parent_address - GetPhysicalAddress(heap_base_); heap_base_ + parent_address - GetPhysicalAddress(heap_base_);
if (address % alignment != 0) {
XELOGE(
"PhysicalHeap::AllocRange translated address {:08X} misaligned "
"(alignment {:08X}, physical base offset {:08X})",
address, alignment, GetPhysicalAddress(heap_base_));
parent_heap_->Release(parent_address);
return false;
}
if (!BaseHeap::AllocFixed(address, size, alignment, allocation_type, if (!BaseHeap::AllocFixed(address, size, alignment, allocation_type,
protect)) { protect)) {
XELOGE( XELOGE(