megboyzz/nfsmw-online
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

GuestHeap: add opt-in red zones and free poisoning (task #45)

Browse Source 
First piece of actual verification infrastructure, rather than
symptom-driven debugging. Off by default (kHeapDebugChecks), so a normal
build is byte-for-byte the same arithmetic as before: kRedZoneBytes is 0 and
every guard branch folds away at compile time.

With checks on:
* each block carries a 16-byte guard immediately after the bytes the caller
  asked for - deliberately not at the end of the block, where slack left by
  a reused larger block would hide a small overflow - verified on Free and
  reported with the offending block's address and size;
* freed payloads are poisoned, which matters more since Alloc stopped
  zeroing: without it a freed block keeps plausible stale contents and a
  use-after-free silently keeps "working".

The check is deliberately not fatal. A loud, precise report is the point;
aborting mid-run would make the engine harder to debug than the corruption
being reported.

BlockHeader's former alignment-only `pad` field now carries the user-visible
size, maintained whether or not checks are on, so BlockSize() reports what
the caller actually got and realloc copies the right amount.

Verified in both directions before trusting it: a deliberate one-byte
overflow was caught and attributed to the right block ("guard byte 0 of 16
is 0x41, expected 0xbe"), while a clean neighbouring block produced no
report. Then a full run to the race with checks on: zero violations, zero
rejected frees, zero faults. That is partial evidence - the guard is only
checked when a block is freed, so blocks that live to process exit are never
examined - but it is the first evidence of its kind this project has.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
megboyzz
2026-09-19 13:49:09 +03:00
co-authored by Claude
parent f631a3a0a4
commit 16f1125951
2 changed files with 98 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mpcore/src/main/cpp/emu/guest_heap.cpp
+54 -4
View File
@@ -44,7 +44,40 @@ void GuestHeap::PushFree(GuestAddr dataAddr, uint32_t size) {
uint32_t GuestHeap::BlockSize(GuestAddr addr) const {
if (!IsValidLiveBlock(addr)) return 0;
return HeaderAt(addr)->size;
// The USER size, not the footprint - callers (Shim_realloc) want to know
// how many bytes they were actually given, not how much the guard adds.
return HeaderAt(addr)->user_size;
}
void GuestHeap::WriteRedZone(GuestAddr dataAddr, uint32_t userSize) {
if (!kHeapDebugChecks) return;
std::memset(host_base_ + dataAddr + userSize, kRedZoneFill, kRedZoneBytes);
}
bool GuestHeap::CheckRedZone(GuestAddr dataAddr, uint32_t userSize) const {
if (!kHeapDebugChecks) return true;
const uint8_t* guard = host_base_ + dataAddr + userSize;
for (uint32_t i = 0; i < kRedZoneBytes; i++) {
if (guard[i] == kRedZoneFill) continue;
// Loud and specific: this identifies the block that was overflowed
// AND how far past its end the write reached, which is the whole
// point - the corruption is reported at the offending block rather
// than wherever it happens to surface later.
Log("GuestHeap: RED ZONE VIOLATION - block at 0x%x (user_size=%u) was written past its "
"end: guard byte %u of %u is 0x%02x, expected 0x%02x. Something overflowed THIS "
"block; see guest_heap.h's kHeapDebugChecks comment.",
dataAddr, userSize, i, kRedZoneBytes, guard[i], kRedZoneFill);
return false;
}
return true;
}
void GuestHeap::PoisonPayload(GuestAddr dataAddr, uint32_t userSize) {
if (!kHeapDebugChecks) return;
// Makes a use-after-free read obviously-wrong data. Matters more since
// Alloc stopped zeroing (2026-09-19): without this, a freed block keeps
// its old, plausible contents and a stale pointer keeps "working".
std::memset(host_base_ + dataAddr, kFreePoisonFill, userSize);
}
GuestAddr GuestHeap::Alloc(uint32_t size) {
@@ -52,7 +85,11 @@ GuestAddr GuestHeap::Alloc(uint32_t size) {
if (!host_base_ || size == 0) return 0;
uint64_t callNo = g_allocCalls2.fetch_add(1, std::memory_order_relaxed) + 1;
uint64_t t0 = NowNs2();
uint32_t payload = AlignUp(size, kAlign);
const uint32_t userSize = AlignUp(size, kAlign);
// Everything below allocates in terms of the FOOTPRINT (user bytes plus
// the trailing guard). kRedZoneBytes is 0 unless checks are on, so this
// is identical to the previous arithmetic in normal builds.
const uint32_t payload = userSize + kRedZoneBytes;
auto logIfSampled = [&](const char* how) {
if (callNo <= 5 || callNo % 20000 == 0) {
Log("GuestHeap::Alloc: call #%llu (%s), took %lluns",
@@ -109,9 +146,14 @@ GuestAddr GuestHeap::Alloc(uint32_t size) {
th->magic = kMagic;
th->size = remainder - sizeof(BlockHeader);
th->free = 1;
th->pad = 0;
th->user_size = th->size;
PushFree(tailHeader + sizeof(BlockHeader), th->size);
}
// The guard goes right after the USER bytes, so an overflow of even
// one byte past what the caller asked for is caught - even when the
// block reused was larger and did not get split.
h->user_size = userSize;
WriteRedZone(blockAddr, userSize);
live_bytes_ += h->size;
if (live_bytes_ > peak_live_bytes_) peak_live_bytes_ = live_bytes_;
logIfSampled("reuse");
@@ -133,9 +175,10 @@ GuestAddr GuestHeap::Alloc(uint32_t size) {
h->magic = kMagic;
h->size = payload;
h->free = 0;
h->pad = 0;
h->user_size = userSize;
GuestAddr dataAddr = free_cursor_ + sizeof(BlockHeader);
free_cursor_ += needed;
WriteRedZone(dataAddr, userSize);
live_bytes_ += payload;
if (live_bytes_ > peak_live_bytes_) peak_live_bytes_ = live_bytes_;
logIfSampled("bump-new");
@@ -164,6 +207,13 @@ void GuestHeap::Free(GuestAddr addr) {
return;
}
BlockHeader* h = HeaderAt(addr);
// Verify the guard BEFORE the block goes back on a free list: this is the
// last moment the overflow can still be attributed to this block rather
// than to whoever allocates the memory next. Deliberately not fatal - the
// point is a loud, precise report, and aborting here would make the
// engine less debuggable than the corruption it is reporting.
CheckRedZone(addr, h->user_size);
PoisonPayload(addr, h->user_size);
h->free = 1;
live_bytes_ -= h->size;
// Every block carries its exact payload size, so it goes straight onto
mpcore/src/main/cpp/emu/guest_heap.h
+44 -2
View File
@@ -93,11 +93,43 @@ private:
// payloads are multiples of kAlign), and makes the split arithmetic below
// exact. Costs 4 bytes per block, trivial next to the ~4x this file's
// 2026-09-19 rework removes.
// ---- memory-error detection (2026-09-19, task #45) ----
// OFF by default. The existing magic check on Free catches a wrong or
// stale POINTER, but not the more insidious failure this layer is prone
// to: a shim writing a few bytes past the end of a block and silently
// corrupting whatever live object follows. That has already happened
// once in this project's history (the guest JNIEnv structure was
// overwritten that way - see this class's own comment) and it surfaces
// subsystems later, looking like an unrelated crash.
//
// With checks on, each block gets a trailing guard filled with a known
// pattern, verified when the block is freed, and freed payloads are
// poisoned so a use-after-free reads obviously-wrong data instead of
// plausible stale values. The latter matters more since 2026-09-19,
// when Alloc stopped zeroing.
//
// Both cost memory and time, so this follows the same opt-in discipline
// as every other diagnostic here (see kTraceHeapAllocations in
// import_shims.cpp): flip to true while hunting a corruption bug, never
// leave it on. With it false, kRedZoneBytes is 0 and every guard-related
// branch folds away at compile time.
static constexpr bool kHeapDebugChecks = false;
static constexpr uint32_t kRedZoneBytes = kHeapDebugChecks ? 16 : 0;
static constexpr uint8_t kRedZoneFill = 0xBE; // "guard"
static constexpr uint8_t kFreePoisonFill = 0xDF; // "dead free"
struct BlockHeader {
uint32_t magic; // kMagic if this is a real header GuestHeap itself wrote - see class comment
uint32_t size; // payload size, not including this header
// Total payload footprint this block occupies, INCLUDING the trailing
// guard when checks are on. Free lists, splitting and IsValidLiveBlock
// all work in these terms.
uint32_t size;
uint32_t free; // 1 = free, 0 = in use
uint32_t pad; // alignment only - see above
// What the caller actually asked for (aligned up) - i.e. size minus
// the guard. Was pure alignment padding before; it is maintained
// unconditionally so BlockSize() can report the user-visible size and
// realloc can copy the right amount, whether or not checks are on.
uint32_t user_size;
};
static constexpr uint32_t kMagic = 0x47484B21; // "GuestHeap blocK!" - arbitrary but distinctive
static constexpr uint32_t kAlign = 8;
@@ -175,6 +207,16 @@ private:
// Pushes a free block onto the intrusive list for its exact size.
void PushFree(GuestAddr dataAddr, uint32_t size);
// Guard handling - all no-ops when kHeapDebugChecks is false.
// `dataAddr` is a payload address, `userSize` what the caller asked for;
// the guard lives immediately after the user payload so that a one-byte
// overflow is caught, rather than at the very end of a possibly-larger
// block where slack would hide it.
void WriteRedZone(GuestAddr dataAddr, uint32_t userSize);
// Logs and returns false if the guard was damaged.
bool CheckRedZone(GuestAddr dataAddr, uint32_t userSize) const;
void PoisonPayload(GuestAddr dataAddr, uint32_t userSize);
BlockHeader* HeaderAt(GuestAddr addr) const {
return reinterpret_cast<BlockHeader*>(host_base_ + addr - sizeof(BlockHeader));
}