Fix real WIREBIND crash: stale active-VM pointer dispatched after blocking read (FABRIC-3.md §XII.3)
The interpreter_enabled guard added in the previous commit (662ef44) was a
real but incomplete fix -- re-running the exact repro against it still
panicked (this time as a raw #PF page fault), proving something deeper
was wrong.
Root cause, found via targeted console_puts probes (not GDB --
starkernel_kernel.elf's symbols don't correspond to the actual running
starkernel_loader.efi binary for this monolithic build, same gotcha
already on record from the 2026-08-18 aarch64 investigation):
sk_repl_run()'s main loop captures `active` once, before calling
sk_console_readline(), which then blocks for the next full line. If the
identity `active` points at is killed while that read is still blocked,
the bailout meant to catch this (sk_console_identity_present()) only
checks a generic "is anyone attached" boolean, not "is the specific
identity active belonged to still attached" -- a fast detach of one
identity followed by attach of a different one never produces an
observable gap in that boolean, so the bailout never fires. The stale
`active`, now pointing at freed memory, gets dispatched into.
Fix: re-resolve `active` fresh from g_repl_active_vm immediately before
dispatch, right after sk_console_readline() returns. One line, no
registry lookup, no dereference of the stale pointer -- closes the race
regardless of whether the bailout catches it first.
Verified: rebuilt amd64 clean, reproduced the exact same attach/USE/
detach/attach/USE sequence against the fixed build -- clean switch, no
fault, exerciser runs correctly afterward.
FABRIC-3.md §XII.2 also corrected to stop claiming the interpreter_
enabled guard alone closed the crash -- it didn't, per the above.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EXieurDfDSsDFdnSyusuWo
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
662ef44e59
commit
70421bdd43
+57
-21
@@ -2070,37 +2070,73 @@ its own caller, not something `capsule_exec_init()` did on every caller's behalf
|
||||
`capsule_birth_mama()` calls `vm_exec_fn` directly (not `capsule_exec_init()`), so this fix has no
|
||||
effect on Mama's own boot path.
|
||||
|
||||
### XII.2 — `USE` into a WIREBIND identity before its interpreter is enabled panics the whole
|
||||
machine — CLOSED
|
||||
### XII.2 — `USE` into a WIREBIND identity before its interpreter is enabled — real guard added,
|
||||
but NOT what was actually crashing the machine (see §XII.3) — CLOSED as its own narrow fix
|
||||
|
||||
Found live: attaching identity `03` via QMP hotplug, then immediately sending
|
||||
`S" 03" USE` the moment the log printed `WIREBIND: 03 attached and ready -- USE it to begin`,
|
||||
produced not an ACL fault but a full kernel panic --
|
||||
`[StarKernel HAL] PANIC: interpreter invoked before bootstrap completion` -- and the machine
|
||||
halted completely (`System halted.`, QEMU process left spinning uselessly). Root cause: a WIREBIND
|
||||
identity's VM starts with `interpreter_enabled = 0` (`vm_bootstrap.c:162`) and only gets
|
||||
`vm_enable_interpreter()` called as a later, separate step of the same birth sequence. The
|
||||
"attached and ready" message prints as soon as the VM is registered, before that later step is
|
||||
guaranteed to have run. `mama_word_use()` (`USE`) checked the target for dead/stillborn state but
|
||||
never checked interpreter readiness before redirecting the whole console's input to it -- the very
|
||||
next line typed would hit `vm_assert_interpreter_enabled()` inside `vm_interpret()` and call
|
||||
`host->panic()`, which is a hard, unrecoverable halt, not the per-session-recoverable ACL-fault
|
||||
path a redirected VM otherwise gets (§XI.3's fix). This is a real race independent of automation
|
||||
speed: a human typing `USE` right after seeing "ready" hits it exactly the same way.
|
||||
halted completely (`System halted.`, QEMU process left spinning uselessly).
|
||||
|
||||
**Fix:** `mama_word_use()` (`mama_forth_words.c`) now checks `((VM *)entry.vm_ptr)->interpreter_
|
||||
**First-pass diagnosis (correct as far as it went, but incomplete):** a WIREBIND identity's VM
|
||||
starts with `interpreter_enabled = 0` (`vm_bootstrap.c:162`), and `mama_word_use()` (`USE`)
|
||||
checked the target for dead/stillborn state but never checked interpreter readiness before
|
||||
redirecting. Added a guard: `mama_word_use()` now checks `((VM *)entry.vm_ptr)->interpreter_
|
||||
enabled` before redirecting, refusing with `USE: <name> not ready yet -- still bootstrapping, try
|
||||
again` instead of performing the redirect. The caller just retries a moment later. Did not touch
|
||||
the birth sequence itself (i.e., did not try to make the "attached and ready" message wait for
|
||||
`vm_enable_interpreter()` to actually run first) -- narrower, lower-risk fix at the point of actual
|
||||
failure.
|
||||
again` instead of performing the redirect. This guard is real, correct, and worth keeping -- it
|
||||
closes a genuine (if narrow) gap in `USE`'s own target-readiness check.
|
||||
|
||||
**Both fixes verified:** amd64 rebuild clean, zero new warnings.
|
||||
**But re-running the identical repro against the fixed build panicked again** -- same message,
|
||||
different symptom the second time (a raw `#PF` page fault at `RIP=CR2=0xa0000`, not a clean
|
||||
`panic()` call) -- proving the guard above was not the actual root cause of what the campaign was
|
||||
hitting. See §XII.3 for the real root cause, found via targeted live debug probes (not GDB --
|
||||
`starkernel_kernel.elf` is not the running binary for this monolithic build, its symbols do not
|
||||
correspond to `starkernel_loader.efi`'s runtime addresses, the same gotcha already recorded from
|
||||
the 2026-08-18 aarch64 investigation; `console_puts`-based probes compiled into the actual running
|
||||
image instead).
|
||||
|
||||
**Status:** exerciser campaign resuming with the fixed build. The 24-case exerciser itself (see
|
||||
### XII.3 — Real root cause: `active` VM pointer captured before a blocking read, dispatched after
|
||||
the VM it points at was freed — CLOSED
|
||||
|
||||
**Reproduced deterministically:** attach `00`, `USE` into it (confirmed live), detach `00` (kills
|
||||
both its console and user VMs, `sf_free()`s the structs), immediately attach `01`, then send
|
||||
`S" 01" USE` -- panics or page-faults depending on exactly what memory got reused, every time.
|
||||
|
||||
**Root cause, confirmed via three targeted `console_puts` probes (added, used, reverted -- per
|
||||
standing practice):** `sk_repl_run()`'s main loop captures `active = g_repl_active_vm ? ... : vm`
|
||||
**once**, at the top of each iteration, *before* calling `sk_console_readline()` -- which then
|
||||
blocks, potentially for a long time, waiting for a full line of input. If the identity `active`
|
||||
pointed at is killed while that read is still blocked (`n == 0`, nothing typed yet), the intended
|
||||
safety net -- `sk_console_readline()`'s own `n < 0` bailout -- checks `sk_console_identity_
|
||||
present()`, a **generic "is anyone currently attached" boolean**, not "is the specific identity
|
||||
`active` belonged to still attached." A fast detach-of-one-identity-then-attach-of-a-different-one
|
||||
while the read is blocked never produces an observable gap in that boolean -- presence reads true
|
||||
continuously (`00` present, gap, `01` present, polled too coarsely or too late to ever see the
|
||||
gap) -- so the bailout never fires. The stale `active`, now pointing at freed memory, gets passed
|
||||
straight to `sk_repl_dispatch_line()`. Confirmed live: a probe at the top of the loop showed
|
||||
`g_repl_active_vm` correctly reset to `NULL` the instant `00`'s teardown ran (the kill-path fix
|
||||
from FABRIC-2.md §F.10, still correct), but the *local* `active` in the loop iteration that was
|
||||
already blocked in `sk_console_readline()` never re-read it -- it dispatched the stale snapshot
|
||||
once the next line finally arrived, regardless of what had happened to the global in the meantime.
|
||||
|
||||
**Fix:** `sk_repl_run()` (`repl.c`) now re-resolves `active` fresh from `g_repl_active_vm`
|
||||
immediately before `sk_repl_dispatch_line()`, right after `sk_console_readline()` returns --
|
||||
one extra line, no registry lookup, no risk of touching the freed struct at all (it's a plain
|
||||
global re-read, not a dereference of the stale pointer). This closes the race regardless of
|
||||
whether the `n < 0` bailout catches it first; that bailout's own generic-boolean weakness is
|
||||
untouched here and could still misfire for other purposes, but nothing downstream of it can crash
|
||||
the machine anymore -- worth its own follow-up if it comes up again.
|
||||
|
||||
**Verified:** rebuilt amd64 clean (zero new warnings), reproduced the exact same attach/USE/detach/
|
||||
attach/USE sequence against the fixed build -- `USE: now using 01` succeeds cleanly, no fault, no
|
||||
panic, the 24-case exerciser runs correctly against `01` afterward with identical output to `00`
|
||||
and `zuse`.
|
||||
|
||||
**Status:** exerciser campaign resuming with both fixes in place. The 24-case exerciser itself (see
|
||||
scratchpad `std79-exerciser.fth`, not yet committed -- FORTH-79 standard words only, biased toward
|
||||
mixed/double-precision arithmetic and shift/negate boundary cases per cross-ISA risk) had already
|
||||
produced one interesting, unconfirmed data point before the panic: `-123 456 M*` printed
|
||||
`DOUBLE-OVERFLOW` via `D.` on amd64, identically for both `zuse` and `rajames` -- worth checking
|
||||
produced one interesting, unconfirmed data point before the first panic: `-123 456 M*` printed
|
||||
`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
|
||||
as a bug on its own.
|
||||
|
||||
Reference in New Issue
Block a user