Skip to content

Heap detectors miss exec.library/Allocate, which is where C malloc actually lives #83

Description

@sidick

--sanitize's heap detectors (#65, #66) wrap AllocMem/AllocVec/AllocPooled. They do not reach exec.library/Allocate, and that turns out to be where C programs' malloc actually lives — so the heap detectors miss the most common heap bug in the most common kind of program.

Measured

A deliberately buggy C program built with the real m68k-amigaos-gcc:

static void clobber(char *p) { p[32] = 'X'; }   /* one past a 32-byte buffer */
int main(void) {
    char *buf = malloc(32);
    clobber(buf);
    ...
}

Under --sanitize, volamos reports nothing. The equivalent bug written against AllocMem (fixtures/memtest overrun) is caught immediately.

Tracing the allocation calls that program makes explains why:

11 exec.library(-186) -> Allocate
 3 exec.library(-684) -> AllocVec
12 exec.library(-564) -> ObtainSemaphore

libnix's malloc takes a few large chunks via AllocVec and then sub-allocates from its own MemHeader with Allocate — eleven Allocate calls for one malloc(32). So the guest's 32-byte buffer sits in the middle of a chunk volamos handed out as one big block, and buf[32] lands in libnix's own pool rather than in any redzone volamos placed.

Stack detection is unaffected, which is why a gcc-built strcpy stack smash is caught (see #74) — this is specific to the heap detectors.

Why this is fixable

crate::execchunk implements the real Allocate/Deallocate algorithm itself, walking the guest's MemChunk chain. So volamos already sees every call, the returned address, and the size — the information the detectors need is in hand; it simply isn't used.

Worth being clear about what differs from AllocMem: there, volamos owns the heap and can freely reserve extra space for redzones. Here the guest owns the pool, so any change to block sizes has to keep the guest's own chunk accounting consistent.

Tier 1 — no layout change (cheap, do this first)

Mark the shadow map on the existing calls, without altering a single byte of the guest's pool:

  • On Allocate: mark the returned range Uninit (unless it is a cleared path), exactly as execmem's handlers do.
  • On Deallocate: mark the freed range Unaddressable with PoisonReason::Freed.

That immediately buys use-after-free and uninitialised-read detection for malloc'd memory, which is most of the value, for a handful of lines and no behavioural risk. Note the ordering rule from #80 applies: writes heal shadow bytes, so poison after any zeroing the handler does, not before.

Tier 2 — redzones (harder, needs care)

Catching overruns needs guard bytes, which means Allocate taking more from the chunk than the caller asked for and returning an inner pointer, with a host-side map from returned pointer to true extent so Deallocate can release the real block. GuestHeap already does exactly this for AllocMem (alloc_with_requested/extent_of_live_alloc), so the shape is proven.

Two things to think through before attempting it:

  • A guest that walks its own mh_First chain to compute free memory will see different numbers. That is probably acceptable — it is the same trade AvailMem already makes under --sanitize (see guestmem's docs) — but it is a genuine guest-visible change, whereas Tier 1 is not, so it deserves its own decision and possibly its own flag.
  • Deallocate takes the size the caller believes it allocated, so the pointer-to-extent map is load-bearing: get it wrong and the guest's pool corrupts. execmem's FreeMem size-mismatch check is the precedent for failing loudly rather than silently mis-freeing.

Scope

  • Tier 1: mark Uninit on Allocate, Freed on Deallocate. Verify a use-after-free and an uninitialised read through malloc in a gcc-built C program are both reported.
  • A C fixture (or a documented local test, given gcc isn't in CI) exercising malloc use-after-free and an uninitialised malloc read.
  • Decide separately whether Tier 2 redzones are worth the guest-visible change, and whether they need their own flag.
  • Re-run the false-positive corpus: real pLhA, PhxAss and SAS/C sc must stay clean, plus the gcc binaries now in play.
  • Document in the CLI reference which allocator the heap detectors cover — right now it says "heap" without qualifying that malloc in a C runtime may not be reached.

Related

#65 (the sanitizer), #66 (heap redzones and the quarantine), #68 (uninitialised reads), #80 (--dirty-heap, whose fill has the same Allocate blind spot), #74 (where this was found, and why stack detection is unaffected).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions