Skip to content

Stop brk growing through its neighbors - #395

Merged
jserv merged 2 commits into
mainfrom
heap-corrupt
Sep 26, 2026
Merged

jserv merged 2 commits into
mainfrom
heap-corrupt

Conversation

@jserv

@jserv jserv commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

sys_brk bounded a new break only by guest_size. Everything from the heap up to MMAP_RX_BASE is one premapped RW span and a grow re-maps what it crosses, so an unbounded grow did not fault: it extended page tables over the guest stack, zeroed the range it had just handed the guest, and returned success. The guest saw its own stack replaced by zeroes, which surfaces later and elsewhere as corrupted heap metadata or a jump through a null pointer.

A grow now stops at the first tracked region above the break and returns the unchanged break, which glibc and musl read as "no more heap" before falling back to mmap. Nothing is exempt from that, the heap's own record included: two drafts exempted it by name and both turned out to be reachable, because mremap keeps the source region's name.

Four more defects around it were found the same way and are fixed here. A heap hole left the pieces above it stranded, wedging the heap for the life of the process in 48 measured shapes. A growth adopted a sealed neighbour's prot, so the next mprotect took its same-prot fast path and left the pages writable. The fallback record spanned pages that had no page-table entries. The stale-tracker flag outlived the execve that rebuilt the table it doubted, which static glibc does not survive. Image placement also leaves a minimum window above the break and is bounded by MMAP_RX_BASE, so an ET_EXEC linked near 250 MiB is refused rather than placing its stack on the mmap RX region, and execve asks for that placement before its point of no return so the guest gets ENOEXEC instead of the VM going down with exit(128).

A terminating EL0 fault now reports its address, PC and ESR at warn level rather than only under --verbose, for SIGILL as well as SIGSEGV. That is the diagnostics gap #320 also named: the reporter had to rebuild with the line promoted before they could see it.

Left out deliberately: a capacity pre-merge in guest_region_add_ex_owned_gpa was written and then removed, because no guest-reachable path exercised it. Five follow-ups are filed rather than folded in, including a fatal signal that ends only the faulting thread instead of the thread group, and the 104 MiB heap window itself, which this bounds rather than widens.

Closes #320


Summary by cubic

Fixes sys_brk growing through its neighbors, which could overwrite the guest stack and silently corrupt heap metadata (issue #320).

  • A grow now stops at the first tracked region above the break and returns the unchanged break; glibc and musl fall back to mmap.
  • Fixes four related defects: a heap hole that wedged the heap for the process's lifetime, a growth that adopted a sealed neighbor's prot and left grown pages writable, a fallback record spanning pages without page-table entries, and a stale-tracker flag that outlived execve.
  • Heap and stack placement now leaves a minimum window above the break and is bounded by the mmap RX region; execve refuses images that leave no room with ENOEXEC instead of exit(128).
  • A terminating EL0 fault now logs its address, PC, and ESR at warn level for both SIGILL and SIGSEGV, not only under --verbose; the SIGILL line describes the branch as a non-abort exception and carries the EC rather than claiming an undefined instruction.
  • Verified on macOS 26 / Apple silicon: make check, the full test matrix, make verify-brk (11 of 11 obligations), and the new test-brk-stack suite all pass.

Written for commit 4f200a9. Summary will update on new commits.

Review in cubic

cubic-dev-ai[bot]

This comment was marked as resolved.

cubic-dev-ai[bot]

This comment was marked as resolved.

sys_brk bounded a new break only by guest_size. Everything from the heap
up to MMAP_RX_BASE is one premapped RW span and a grow re-maps what it
crosses, so an unbounded grow did not fault: it extended page tables
over the guest stack, zeroed the range it had just handed the guest, and
returned success. The guest saw its own stack replaced by zeroes,
surfacing later as corrupted heap metadata or a jump through a null
pointer. Reproduced at about 104 MiB of malloc, which is where the heap
reaches the stack.

A grow now stops at the first tracked region above the break, with
g->stack_base as the bound at the stack, and returns the unchanged
break, which glibc and musl read as no more heap before falling back to
mmap. Nothing is exempt from that, the heap's own record included: two
drafts exempted it by name and both were reachable, because mremap keeps
the source region's name.

regions[] is start-sorted and non-overlapping, so ends are monotonic and
guest_region_first_end_above answers in one search. The record it lands
on is the first that can be in the way, a straddler included, and the
record a growth extends is the one below it. Walking every record below
the break instead answers the same question and costs 1.47 ns per
record: 6221 ns against 1823 once 3000 heap records sit below it.

Four defects around it were found the same way and fixed here. A heap
hole left the pieces above it stranded, which wedged the heap for the
life of the process in 48 measured shapes. A growth adopted a sealed
neighbor's prot, so the next mprotect took its same-prot fast path and
left the pages writable. The fallback record spanned pages that had no
page-table entries. The stale-tracker flag outlived the execve that
rebuilt the table it doubted, which static glibc does not survive, since
its pre-TLS __libc_setup_tls writes errno before the thread pointer
exists.

Image placement leaves BRK_MIN_WINDOW above the break, so an image whose
load_max is 2 MiB-aligned no longer gets a heap of zero bytes, and is
bounded by MMAP_RX_BASE rather than the infra reserve, so an ET_EXEC
linked near 250 MiB is refused instead of placing its stack on the mmap
RX region. execve asks for that placement before its point of no return
and answers ENOEXEC, where the post-PNR refusal would have taken the VM
down with exit(128).

A terminating EL0 fault now reports its address, PC and ESR at warn
level rather than only under --verbose, for SIGILL as well as SIGSEGV.
That is the diagnostics gap the issue also named: the reporter had to
rebuild with the line promoted to see it at all.

src/proved/brk.h carries the bound arithmetic under ACSL contracts, and
make verify-brk discharges 11 of 11 obligations with both mutants
caught. tests/test-brk-stack.c covers every neighbor shape in 26 checks,
each verified to fail against the defect it names, and runs in the
elfuse-aarch64 and qemu-aarch64 lanes so a real kernel adjudicates it.

Closes #320
The catch-all branch in vcpu_handle_el0_fault delivers SIGILL for every
EC that is not a lower-EL abort, and its message named one of them. That
was harmless while the line only appeared under --verbose; now that a
terminating fault reports at warn level, a guest killed by an alignment
fault or an illegal execution state gets a default-level line claiming
an undefined instruction.

Say what the branch actually covers and keep the EC, which is what
identifies the class. The comment above it needed the same treatment for
a different reason: it listed 0x18 and 0x3C among the classes landing
here, but src/core/shim.S dispatches those to HVC #12 and HVC #10 before
the catch-all, so neither ever arrives.
@jserv
jserv merged commit 5027e21 into main Sep 26, 2026
19 checks passed
@jserv
jserv deleted the heap-corrupt branch September 26, 2026 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Guest heap corruption in a program exec'd by a large parent (dpkg maintainer script)

1 participant