Fix use-after-free in sk_repl_idle()'s idle-tick VM resolution (FABRIC-3.md §XIII)
Build / build-amd64-iso (push) Waiting to run
Build / build-aarch64-iso (push) Waiting to run
Build / build-riscv64-img (push) Waiting to run

Root-caused a heap-corruption bug that reliably failed WIREBIND identity
attach on the third attach/detach cycle in one boot. sk_console_readline()
and sk_console_getkey() captured `active_vm` once from their caller and
kept passing that same (possibly long-stale) pointer to sk_repl_idle() on
every idle tick serviced while blocked waiting for input. If the VM it
pointed at was killed (WIREBIND detach) mid-block, the existing bailout
only checked a generic "is anyone attached" boolean -- masked as soon as a
different identity attached next -- so blk_vm_flush_all() kept writing
into a freed VM struct sitting on kmalloc's own free list, corrupting the
free list's linked-list metadata itself.

Both idle branches now re-resolve the live active VM fresh from
g_repl_active_vm on every tick, matching the dispatch-side fix already
made for the sibling bug in §XII.3.

Verified: rebuilt amd64, reran the exact three-cycle repro that reliably
corrupted the heap before the fix -- free-list census stayed stable
through the same idle window that previously collapsed to zero. All three
architectures (amd64/aarch64/riscv64) boot clean to the zuse)ok> prompt.

kmalloc_debug_census()/kmalloc_debug_census_bytes() kept as permanent
diagnostic infrastructure; every other temporary probe added during the
investigation was reverted.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EXieurDfDSsDFdnSyusuWo
This commit is contained in:
Robert Allan James
2026-09-10 20:30:41 -04:00
co-authored by Claude Sonnet 5
parent 70421bdd43
commit 9142dda2d6
38 changed files with 236983 additions and 3 deletions
+101
View File
@@ -2140,3 +2140,104 @@ produced one interesting, unconfirmed data point before the first panic: `-123 4
`DOUBLE-OVERFLOW` via `D.` on amd64, identically for `zuse`, `rajames`, and `00` -- worth checking `DOUBLE-OVERFLOW` via `D.` on amd64, identically for `zuse`, `rajames`, and `00` -- worth checking
against the other two architectures once the campaign completes; not yet root-caused or reported against the other two architectures once the campaign completes; not yet root-caused or reported
as a bug on its own. as a bug on its own.
## XIII. Heap corruption under repeated WIREBIND attach/detach cycling — root-caused and CLOSED
2026-09-10
Surfaced while running the std79 exerciser campaign: cycling identities through one boot
(attach → `USE` → exercise → detach → attach next) on amd64 reliably failed after the *third*
WIREBIND identity cycle in a boot -- `xhci: USB MSC block-subsystem attach failed`, and the
target identity never births (`USE: NN not found`).
**Ruled out along the way, with live evidence, not just inspection:**
- **Not the image files** -- a failing identity's own thumbdrive attaches and births cleanly
from a cold boot in isolation.
- **Not a timing race between WIREBIND bind and the hotplug/IRQ event.**
- **Not heap fragmentation in the ordinary sense, and not something a defragmenter would fix.**
`kmalloc.c`'s allocator was read in full: real block splitting (`allocate_from_block()`/
`can_split()`) and real bidirectional coalescing on every `kfree()` (`coalesce_neighbors()`),
both correct. A free-list census (`kmalloc_debug_census()`, kept as permanent debug
infrastructure -- see below) plus a second census summing total free/used *bytes*
(`kmalloc_debug_census_bytes()`, added specifically to discriminate this investigation's two
competing hypotheses) confirmed that genuine cross-VM fragmentation is real but benign at this
scale: `free_blocks` climbs by roughly 1,000-1,100 per birth/kill cycle (interleaved
`DictEntry` allocations from other live VMs prevent `coalesce_neighbors()` from merging a
killed VM's freed entries back together), while `total_free_bytes` stayed flat at ~497-509 MB
and `largest_free` never dropped below ~492 MB across two full cycles. A third, later boot with
three full cycles (00/01/02) reproduced the real failure signature under the same probes:
`total_free_bytes` collapsed from ~508 MB to **literally 0** between two consecutive idle
ticks, with `used_blocks` staying a sane ~7,300 throughout -- proving this was never
fragmentation, it was the free list's own linked-list metadata being overwritten.
- **Not a garbage/oversized BAM allocation from a bad device-geometry read** -- geometry probes
around `blkio_info()`/`blk_format_or_load_disk()` showed clean, identical values every
successful cycle, and the failing cycle never even reached that code.
- **Not `capsule_wirebind_try_attach()`'s own body, including cert verification (BINDSTEP)** --
directly tested by bypassing the BINDSTEP block entirely; the corruption still occurred at the
same point, one cycle later, ruling the whole cert-verification/X.509 path out.
- **Not the MSG-TICK pump's own `vm_interpret()` dispatch, `blk_migration_idle_check()`, or
`capsule_wirebind_overflow_idle_check()`** -- each individually bracketed with pre/post census
probes; all read clean at the exact tick the corruption appeared elsewhere.
**Root cause, confirmed via bisection down to the exact tick and line:** `sk_repl_idle()`'s
`blk_vm_flush_all(active_vm)` call was the exact point where `total_free_bytes` collapsed
(`TD:flush pre lf=492362056 fb=1140``TD:flush post lf=0 fb=0`, same tick, no intervening
code). `blk_vm_flush_all()` (`block_words.c`) reads and writes `vm->blk_vm_lbn[]`,
`vm->blk_vm_cbuf[]`, `vm->blk_vm_dirty[]`, and `vm->blk_vm_epoch` on whatever `VM *` it's given
-- and the `active_vm` it was given traced back to a genuine use-after-free in
`src/starkernel/repl.c`, sibling to the bug already fixed once in §XII.3 (2026-09-09/10) but not
fully closed by that fix:
- `sk_repl_run()`'s main loop resolves `active = g_repl_active_vm ? g_repl_active_vm : vm;` once
per line, then calls `sk_console_readline(input, sizeof(input), active, 1)` -- which can block
for an arbitrarily long real-world time waiting for the next character.
- **While blocked**, `sk_console_readline()`'s own idle branch called
`sk_repl_idle(active_vm)` every `SK_IDLE_BEAT_INTERVAL` using that *same parameter*, captured
once, for the entire duration of the block (`repl.c`, was line 978).
- The bailout meant to catch a mid-read logout (`if (reanchor_prompt && n == 0 &&
!sk_console_identity_present()) return -1;`) checks a **generic "is anyone attached at all"
boolean**, not "is the specific identity `active_vm` belonged to still attached" -- the exact
anti-pattern already flagged once before in this codebase (see
`feedback_boolean_guards_vs_identity_comparison.md`). So: identity A is the active VM when this
read begins and gets WIREBIND-detached (killed, freed) while the read sits idle;
`sk_console_identity_present()` reads **false** for one tick, but before this idle branch ever
observes that false and bails, identity B attaches (a normal, unrelated next cycle) and
presence flips back to **true** -- permanently masking that the *original* `active_vm` (A) is
long gone. Every subsequent idle tick inside this same still-blocked read keeps calling
`sk_repl_idle(active_vm)` -> `blk_vm_flush_all(active_vm)` on A's freed `VM` struct.
- A's freed struct is, at that point, sitting on kmalloc's own free list.
`blk_vm_check_epoch()`'s unconditional field writes (`vm->blk_vm_lbn[i] = 0;` etc., inside
`blk_vm_flush_all()`'s own epoch check) land inside that free block's memory -- and if any of
`VM`'s `blk_vm_*` fields happen to overlap where `kmalloc.c`'s own `heap_block_t` stores
`next`/`size`/`free`, the write corrupts the free list's own linked-list metadata. This exactly
explains the observed signature: a walk that used to see ~2,100 healthy free blocks suddenly
sees 0, because the `next` pointer chaining them together got overwritten with zeros.
Reproducibly on the *third* identity cycle specifically because that's the point at which the
"gone, then something else attaches" masking sequence first lines up with a still-blocked read
from an *earlier* cycle's identity.
**Fix (commit follows this entry):** re-resolve the live active VM fresh on every idle tick,
inside both `sk_console_readline()`'s and `sk_console_getkey()`'s idle branches, exactly
matching the pattern already used at the dispatch site (§XII.3's fix): `VM *live_active =
g_repl_active_vm ? g_repl_active_vm : (VM *)sk_get_mama_vm();` then `sk_repl_idle(live_active);`
-- never trust the `active_vm` parameter captured before the block began. `sk_get_mama_vm()`
(Hera) is the safe fallback, matching `sk_repl_run()`'s own `g_repl_active_vm ? g_repl_active_vm
: vm` shape. Both functions' `active_vm` parameters are now unused for this purpose (silenced
with `(void)active_vm;`, kept in the signature since `repl.h` declares it and other callers still
pass one) -- no fix on the *bailout* boolean at line ~1029 was needed or made, since the memory
corruption is now impossible regardless of whether that cosmetic prompt-reanchor logic still has
its own generic-boolean flaw.
**Verified:** rebuilt amd64, reran the exact three-cycle repro (00 attach/USE/detach, 01
attach/USE/detach, 02 attach/USE/detach) that reliably corrupted the heap before the fix, waited
through the same idle window that previously collapsed `total_free_bytes` to 0 -- census stayed
rock solid (`lf=497605128 fb=2401 tfb=508518296`, unchanged) across dozens of subsequent idle
ticks and `blk_vm_flush_all()` calls.
**Debug infrastructure kept, not reverted:** `kmalloc_debug_census()` and
`kmalloc_debug_census_bytes()` (`src/starkernel/memory/kmalloc.c` /
`include/starkernel/kmalloc.h`) -- both small, cheap, read-only free-list walks, kept as
permanent reusable diagnostic infrastructure per the existing `kmalloc_debug_census()`
precedent. Every other probe added during this investigation (`block_words.c`,
`block_subsystem.c`, `capsule_birth.c`, `vm_core.c`, `mama_forth_words.c`, and the rest of
`repl.c`'s temporary brackets) was reverted after use, per the established "write, run once,
capture, revert" discipline.
+1 -1
View File
@@ -1,5 +1,5 @@
# Capsule Block Manifest — Auto-generated # Capsule Block Manifest — Auto-generated
<!-- Generated by mkcapsule --manifest 2026-09-10T21:16:32Z --> <!-- Generated by mkcapsule --manifest 2026-09-11T00:25:07Z -->
<!-- DO NOT EDIT — re-run mkcapsule --manifest to refresh. --> <!-- DO NOT EDIT — re-run mkcapsule --manifest to refresh. -->
<!-- Hand-written justifications and immutability notes live --> <!-- Hand-written justifications and immutability notes live -->
<!-- in MANIFEST.md alongside this auto-generated index. --> <!-- in MANIFEST.md alongside this auto-generated index. -->
BIN
View File
Binary file not shown.
+17
View File
@@ -66,4 +66,21 @@ kmalloc_stats_t kmalloc_get_stats(void);
uintptr_t kmalloc_heap_base_addr(void); uintptr_t kmalloc_heap_base_addr(void);
uintptr_t kmalloc_heap_end_addr(void); uintptr_t kmalloc_heap_end_addr(void);
/* Debug/diagnostic probe -- free-list census (largest free block, count of
free blocks, count of allocated blocks, total blocks). Kept as permanent
infrastructure (FABRIC-3.md SXIII, 2026-09-10): a cheap O(n) walk over
the free list, useful for any future heap-shape investigation, not just
the one that introduced it. Not called anywhere in the normal boot path
today -- callers add their own log_message() call sites when debugging. */
void kmalloc_debug_census(size_t *out_largest_free, size_t *out_free_count,
size_t *out_used_count, size_t *out_total_blocks);
/* Same precedent as kmalloc_debug_census() above -- sum of free-block bytes
and used-block bytes across the whole free list. The pairing of the two
(block-count census + byte-sum census) is what let FABRIC-3.md SXIII tell
real heap corruption apart from ordinary fragmentation: fragmentation
grows free_count while total_free_bytes stays flat; corruption drops
both together. */
void kmalloc_debug_census_bytes(size_t *out_total_free_bytes, size_t *out_total_used_bytes);
#endif /* STARKERNEL_KMALLOC_H */ #endif /* STARKERNEL_KMALLOC_H */
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
+37
View File
@@ -339,3 +339,40 @@ kmalloc_stats_t kmalloc_get_stats(void)
{ {
return heap_stats; return heap_stats;
} }
void kmalloc_debug_census(size_t *out_largest_free, size_t *out_free_count,
size_t *out_used_count, size_t *out_total_blocks)
{
size_t largest = 0, free_count = 0, used_count = 0, total = 0;
heap_block_t *cur = heap_head;
while (cur) {
total++;
if (cur->free) {
free_count++;
if (cur->size > largest) largest = cur->size;
} else {
used_count++;
}
cur = cur->next;
}
if (out_largest_free) *out_largest_free = largest;
if (out_free_count) *out_free_count = free_count;
if (out_used_count) *out_used_count = used_count;
if (out_total_blocks) *out_total_blocks = total;
}
void kmalloc_debug_census_bytes(size_t *out_total_free_bytes, size_t *out_total_used_bytes)
{
size_t free_bytes = 0, used_bytes = 0;
heap_block_t *cur = heap_head;
while (cur) {
if (cur->free) {
free_bytes += cur->size;
} else {
used_bytes += cur->size;
}
cur = cur->next;
}
if (out_total_free_bytes) *out_total_free_bytes = free_bytes;
if (out_total_used_bytes) *out_total_used_bytes = used_bytes;
}
+40 -2
View File
@@ -749,6 +749,11 @@ static int g_console_pending_key = -1;
* the caller's job, same as any standard KEY implementation. */ * the caller's job, same as any standard KEY implementation. */
int sk_console_getkey(VM *active_vm) int sk_console_getkey(VM *active_vm)
{ {
/* No longer used directly -- the idle branch below re-resolves the
* live active VM itself (FABRIC-3.md SXIII, 2026-09-10) rather than
* trusting this parameter, which can go stale mid-block. Kept in the
* signature: repl.h declares it, other callers still pass one. */
(void)active_vm;
for (;;) { for (;;) {
int c; int c;
if (g_console_pending_key >= 0) { if (g_console_pending_key >= 0) {
@@ -763,7 +768,13 @@ int sk_console_getkey(VM *active_vm)
uint64_t now = heartbeat_ticks(); uint64_t now = heartbeat_ticks();
if (now - g_last_beat_tick >= SK_IDLE_BEAT_INTERVAL) { if (now - g_last_beat_tick >= SK_IDLE_BEAT_INTERVAL) {
g_last_beat_tick = now; g_last_beat_tick = now;
sk_repl_idle(active_vm); /* Same use-after-free class fixed in sk_console_readline()'s
* idle branch (FABRIC-3.md SXIII, 2026-09-10) -- `active_vm`
* is a parameter captured once by the caller before this
* (possibly long) blocking wait for a key began, and can be
* killed mid-wait. Re-resolve fresh every tick instead. */
VM *live_active = g_repl_active_vm ? g_repl_active_vm : (VM *)sk_get_mama_vm();
sk_repl_idle(live_active);
} }
arch_relax(); arch_relax();
} }
@@ -837,6 +848,11 @@ int sk_console_key_available(void)
int sk_console_readline(char* buf, int size, VM* active_vm, int reanchor_prompt) int sk_console_readline(char* buf, int size, VM* active_vm, int reanchor_prompt)
{ {
/* No longer used directly -- see sk_console_getkey()'s matching comment;
* the idle branch below re-resolves the live active VM itself instead
* of trusting this parameter across a potentially long block. Kept in
* the signature: repl.h declares it, other callers still pass one. */
(void)active_vm;
int n = 0; int n = 0;
/* TX counter value right after the caller printed its prompt. Any /* TX counter value right after the caller printed its prompt. Any
* console output that lands while this readline blocks (heartbeat * console output that lands while this readline blocks (heartbeat
@@ -889,7 +905,29 @@ int sk_console_readline(char* buf, int size, VM* active_vm, int reanchor_prompt)
* design already tolerates. */ * design already tolerates. */
if (n == 0 && now - g_last_beat_tick >= SK_IDLE_BEAT_INTERVAL) { if (n == 0 && now - g_last_beat_tick >= SK_IDLE_BEAT_INTERVAL) {
g_last_beat_tick = now; g_last_beat_tick = now;
sk_repl_idle(active_vm); /* Found live 2026-09-10 (FABRIC-3.md SXIII): `active_vm` is
* this call's parameter, captured once by the caller before
* this (possibly very long) block began -- see the matching
* comment at this function's dispatch-side fix, below, for
* the full mechanism. That fix re-resolves `active` fresh
* right before dispatch, but every idle tick serviced
* *during* this same blocked call used to pass the stale
* parameter straight into sk_repl_idle() -> blk_vm_flush_all(),
* which reads and writes vm->blk_vm_lbn[]/cbuf[]/dirty[]/
* epoch on whatever `active_vm` points at. If that VM was
* killed (WIREBIND detach) while this call sat idle, those
* fields live in a kmalloc block already back on the free
* list -- and blk_vm_check_epoch()'s unconditional field
* writes silently corrupt that block's own free-list
* metadata (next/size/free), observed live as free_blocks/
* largest_free collapsing to 0 a tick or two after a third
* WIREBIND identity attached following two prior attach/
* detach cycles. Re-resolve fresh from the global here too,
* same pattern as the dispatch-side fix -- sk_get_mama_vm()
* is the safe fallback (never freed) matching sk_repl_run()'s
* own `g_repl_active_vm ? g_repl_active_vm : vm` shape. */
VM *live_active = g_repl_active_vm ? g_repl_active_vm : (VM *)sk_get_mama_vm();
sk_repl_idle(live_active);
} }
/* Blink the cursor while idle (no key ready this iteration), /* Blink the cursor while idle (no key ready this iteration),