Skip to content

x86 free-path custody hardening follow-ups (from #470 PR-3 reviews) #556

Description

@ryanbreen

Summary

PR #470 PR-3 (x86 free-path custody, aarch64/x86 root+leaf leak closure) went through two review
rounds (r1, final) plus a bounded fix-diff confirmation pass. All blocking findings from both rounds
were closed before merge. This issue consolidates the surviving non-blocking notes — real,
disclosed residuals that are safe to ship but worth hardening in follow-up work — so they aren't lost
in scratch review files. Grouped by theme; deduplicated across the three review passes. References
#545 and #554 where a note's mechanism ties to those open issues.

Sources (temporary scratchpad, content inlined below so this issue is self-contained):
review-pr3-r1.md, review-pr3-final.md, review-fixdiff.md.


Theme 1: Shadow-root CR3 slot (saved_process_cr3) can block forever or skip a restore

Origin: r1 finding 4 → still valid per final review finding 9 and fixdiff N1 (extends r1-4's
mechanism to superseded roots).

  • saved_process_cr3 (gs:80) is written on every userspace→kernel entry
    (syscall/entry.asm:66-68, timer_entry.asm:66-68) but is only ever read by
    shadow_root_is_live — nothing clears it on thread death, process exit, or a switch to a
    kernel/idle thread, unlike next_cr3 (gs:64), which the asm clears on consumption. If a dying
    root's Shadow leg is the last one standing (all other legs cleared, but no other userspace thread
    ever re-enters the kernel on that CPU), the leg blocks permanently. Shadow blockers do not
    increment proof_failures (only Cached | LiveRow do), so the receipt is never parked — it is
    requeued forever, and idle_loop's drain runs a full proof pass on it at ~1000 Hz indefinitely.
  • clear_shadow_root (process_task.rs:166-172) was added in the fix round and closes the simple
    case, but only clears for page_table, not old_page_tables — a set PR-3's own D3 change made
    non-empty in x86 production. A process that execs (superseding a root into
    pending_old_page_tables) and dies before re-entering userspace still has saved_process_cr3
    naming the old root, reproducing the same permanent-stall shape (fixdiff N1).
  • Separately (final review finding 9), the safety argument for writing zero to the slot — "the
    consuming asm skips its mov cr3 when the slot is zero, and CR3 already holds the same root, so a
    zero is a no-op" — holds only while nothing moves CR3 between the clear and the return. Any kernel
    path that calls switch_to_kernel_page_table(), does not context-switch, and returns to ring 3 on
    that CPU would IRETQ with the kernel PML4 loaded — the x86: not-present fault (0x14) at user RIP 0x4000e2bc on first entry to userspace in http_test #553 instruction-fetch fault class that
    79c4818a was written to eliminate. This is asserted, not measured: the O2/P2 proof leg that would
    exercise the shadow slot never runs on x86 (see Theme 4).

Suggested hardening: clear saved_process_cr3 for the whole root set (page_table
old_page_tables) on defer; add a debug-level counter or fail-closed re-stamp on the
non-context-switching return path so a stale zero-write is checkable rather than only argued in a
comment.


Theme 2: Reclaim-queue lock/stall exposure under fault or contention

Origin: r1 finding 5, final review finding 7 (half-closed), final review finding 8, fixdiff N7/N8.

  • RECLAIM_DRAIN_ACTIVE (added in the fix round) stops idle_loop's drain from re-entering an
    abandoned pass, but does not release the spin::Mutex (PENDING_PROCESS_RECLAIMS /
    PARKED_PROCESS_RECLAIMS) the abandoned pass was holding if a kernel fault occurs mid-drain (e.g.
    a page fault in release_mapped_leaves's walk_mapped_pages over a corrupted table). The next
    process exit calls enqueue_process_reclaimpush_pending_or_abandon, which spins on that same
    abandoned mutex with interrupts masked — an unrecoverable hard hang on the exit path, on a queue
    that did not exist on main. park_reclaim/unpark_sweep have the same exposure.
  • RECLAIM_CONTEXT_VIOLATIONS is overloaded: both the genuine context violation (PM/scheduler scope
    held) and a healthy concurrent drain increment the same counter. On a busy system where idle
    rarely runs, the exit-path drain leg (the one D1's forward-progress argument relies on) is disabled
    exactly when needed, and the counter that would reveal this is indistinguishable from a real
    lock-discipline bug. The cohort's RECLAIM_CONTEXT_VIOLATIONS == 0 assertion can't see this because
    the boot fixture owns the queues alone.
  • The fix round's try_lock conversion in unpark_sweep_with_snapshot (was lock()/lock()) is the
    correct direction per campaign law (leak beats hang beats over-free) but changes a would-be loud
    deadlock into a silent rise in PT_ROOT_ABANDONED_NO_ARCH, whose healthy-zero expectation is
    asserted by the gates but not observable on a production boot otherwise.
  • The "owning drain will take it" comment justifying disabled nested drains has a genuine but narrow
    race window: between the owning drain reading None from its selection loop and storing false
    into RECLAIM_DRAIN_ACTIVE, a receipt pushed in that window is taken by neither drain and waits for
    idle's next iteration. Forward progress still holds, but "idle runs again" is a weaker guarantee
    than the comment states.

Suggested hardening: split the context-violation counter from the healthy-nesting counter; add a
ratchet anchor for RECLAIM_DRAIN_ACTIVE (currently untested per Theme 5 gap 1) so its claim/release
discipline can't silently regress.


Theme 3: OOM fail-closed coverage is partial

Origin: r1 finding 7 (closed in code, unratcheted), final review section E finding 23 gap 2.

The initial try_reserve(1) guard on queue growth is real, but only one of several growth sites is
guarded. The fix round moved four queue-growth sites onto try_reserve
(push_pending_or_abandon, requeue-after-blocker, requeue-after-budget, park_reclaim,
unpark_sweep_with_snapshot), which closes the immediately reachable gap, but
PROCESS_PAGE_TABLE_ABANDON_SITES only pins abandon_unqueued_reclaim's two reasons — a future
push added back without a reservation is unratcheted and would silently reintroduce an
unreserved-allocation abort path under real memory pressure.

Suggested hardening: add a ratchet anchor that enumerates every queue-growth call site and asserts
each goes through try_reserve.


Theme 4: x86 O2 injection-gate matrix (E/F/I/J/P1-P4/Q) never executes

Origin: r1 finding 3 (partly closed), final review finding 1 + finding 2 + section F (matrix).

The RCA (RCA-cohort-mmap.md) established that no registry-registered x86 test can ever run — the x86
boot thread is the idle thread, and its continuation is discarded at the first post-RING3_SMOKE
preemption. The fix round gave only the retire cohort a direct call site
(main.rsrun_x86_retire_cohort_gate); reclaim_progress_gate_test — spec §4.3's entire O2
matrix (contention loss, budget requeue, mid-retire drop, idempotence, and all four proof-leg forcing
cases: Hardware, Shadow, LiveRow, Epoch) — never got one and still never executes on x86. The only two
x86 gates that do run (O3, the retire cohort) both drive quiescent, uninjected address spaces to a
clean retire; neither exercises a blocked-then-cleared leg, a requeue, a mid-retire drop, a contended
loss, or the defer-refusal fallback.

Mutation coverage reflects the same gap: of the spec's designated x86 mutation rows, only 3 were run,
and one (M1, "stub the root return out of retire_bounded") is not the designated mutation — the
version actually run left the per-PID counters healthy and only the whole-boot balance oracle (O4)
caught it, leaving the per-PID equality (returned == recorded + 1) itself unproven per the
campaign's own "an oracle that doesn't fail on its designated mutation is not an oracle" rule. A
second (M3) is the inverse of its designated mutation (forces the Hardware leg to always block
rather than always pass), proving the downstream balance oracle notices a stall but not that the P1
sub-case can detect a leg that stopped blocking.

Suggested hardening: give reclaim_progress_gate_test (and the still-dead retirement_fence_gate
on x86) a direct call site analogous to the retire cohort's, then run the full designated
mutation/injection matrix from PR3-SPEC.md §4.2/§4.3/§4.5 against it (see the unproven-row table
below).

Unproven designated rows (from the final review, still open): M7-x86→O1 per-PID equality;
M8-x86 (remove cohort sentinel mappings)→O1; M9-x86 (reclaim_bounded skips
release_mapped_leaves)→O1/O4; O2 Hardware-leg-always-pass→P1; O2 retire_bounded returns Complete
without returning the root→E/F; O2 sub-cases E, F, I, J, P1, P2, P3, P4, Q — all currently
gate-unreachable on x86.


Theme 5: Ratchet/anchor gaps and decorative pins

Origin: r1 finding 24 (still valid), final review findings 21, 23, 24.

  • Deviation record staleness. The single deviation record (deviation-record-pr3.md) that
    TRAP-LIST-1a.md requires be stamped at the final SHA and pasted verbatim into the PR body was nine
    commits stale at final review, and its "NOT verified here" caveats had become false by that point
    (understating actual evidence). Several live deviations existed only in scattered fix-notes rather
    than the one record: the new PT_ROOT_SLOT_REFUSED counter (COUNTER_COUNT 70→71, contradicting
    spec §3.8's explicit "no new counters"), the retire cohort's direct call site and its placement
    deviation from the RCA plan, the dead x86 reclaim_progress_gate registration, the create_process
    remap deletion, and all six rider fixes.
  • Gap 1 — RECLAIM_DRAIN_ACTIVE has no ratchet anchor. Its claim/release discipline (Theme 2) can
    be deleted with every existing ratchet still green.
  • Gap 2 — OOM fail-closed set has no membership anchor (Theme 3).
  • Decorative pins persist. tests/teardown_structure.rs still requires the harness shell script
    to contain the literal strings advance_stage_marker_only and [TESTS_COMPLETE:0/0] — both of
    which appear only inside a comment in docker/qemu/run-x86-boot-tests.sh. Editing the script's
    actual gating logic while leaving the comment intact keeps the ratchet green. Similarly,
    run-x86-boot-tests.sh echoes the expected literal ($PT_CUSTODY_LITERAL / $PT_COHORT_LITERAL)
    in its transcript rather than the observed serial line; the grep -F -x -c … -eq 1 assertions above
    it make the gate sound, but the pasted-into-PR transcript is a placeholder where a measurement
    belongs.
  • D4 leaf-timing oracle's x86 arm is runtime-vacuous. TEARDOWN_MASKED_FRAMES_WALKED's x86
    producer (drain_old_page_tables_counted) only fires for a process that has exec'd
    (pending_old_page_tables non-empty); the retire cohort's 64 synthetic children fork and exit
    without ever exec'ing, so the counter has no producer in the oracle's window. A future regression
    reintroducing an under-PM frame walk on the x86 exit path would still read delta == 0 and pass.
  • Cohort gate double-run risk. If a future fix (x86: boot thread wedges in the RING3_SMOKE disk read while holding the process-manager lock, blocking every first ring-3 entry #554-class work) makes kernel_main_continue's
    tail reachable again, the dormant fork_exit_defer_reclaim_pairing_test registry entry goes live
    and the retire cohort runs twice per boot, breaking the harness's exactly-once grep -c … -eq 1
    assertions — a bug fix would break the custody gate and look like a custody defect. Either
    re-#[cfg(target_arch = "aarch64")] the registry entry, or change the harness to count >= 1.

Suggested hardening: add ratchet anchors for RECLAIM_DRAIN_ACTIVE and the OOM-guarded
queue-growth set; replace the decorative comment-string pins with anchors on real gating logic; make
the shell gate transcript print the observed line, not the expected literal; guard the cohort registry
entry (or the harness count) against the double-run scenario.


Theme 6: Tier-2 interrupt-path cost beyond the authorized epoch stamp

Origin: final review findings 13, 14 (new in final review, not in r1); confirmed still present in
fixdiff N2 (partially mitigated — hoisted, but into the common branch).

PR3-SPEC.md §2.5 authorizes exactly one Tier-2 addition on the interrupt-return path: a single
relaxed atomic increment (the epoch stamp). The shipped diff also added, on every preemption:
old_thread_is_user (a second scheduler::with_thread_mut — blocking lock_scheduler()), and
saved_context_is_kernel_frame, called unconditionally before the idle/kernel-thread branches (two
more blocking SCHEDULER acquisitions plus, when the caller has no guard, a crate::process::try_manager()
and a process-table scan). The fix round hoisted this into the branch that consumes it (the common
case for a userspace thread preempted in userspace), which is real progress but still runs on
essentially every userspace preemption, and no cycle measurement exists for any of it (D1 says so
explicitly). The ratchet that pins the epoch stamp's minimality only covers what precedes the stamp,
so nothing structurally prevents further growth here.

Separately, saved_context_is_kernel_frame fails open under PM contention: if the PM lock is held
elsewhere (the #554 condition, observed live in ~1/3 of boots in that investigation), the helper
returns false for a user thread whose saved context actually is a kernel frame, falls through to
restore_userspace_thread_context, and the new RestoreError::KernelFrame check fires — terminating
the process with SIGSEGV rather than resuming it correctly. The fail-closed backstop works but the
outcome under lock contention is a killed process, uncounted beyond a log::error!.

Suggested hardening: get a cycle measurement for the added interrupt-path cost; consider hoisting
saved_context_is_kernel_frame below the is_idle/!is_user_thread branches to remove most of it;
add a counter for the fail-open-under-contention SIGSEGV path so it's distinguishable from a genuine
corrupted-context kill.


Theme 7: Miscellaneous disclosed residuals (low-risk, worth tracking)

  • RSP0 publish inconsistency (final review finding 11, fixdiff N3/N4). The abort-recovery helper
    was moved off gdt::set_kernel_stack (TSS-only write, stale per-CPU cache) onto
    per_cpu::update_tss_rsp0, but setup_first_userspace_entry's successful first-entry path still
    uses the TSS-only writer — the same class of staleness the abort-path fix was written to eliminate.
    Additionally, per_cpu::update_tss_rsp0 silently no-ops (rather than panicking, like the deleted
    gdt::set_kernel_stack did) when the per-CPU TSS pointer is null — not reachable today, but a
    fail-loud→fail-silent conversion on the exact path x86: not-present fault (0x14) at user RIP 0x4000e2bc on first entry to userspace in http_test #553 was written for. The RSP0-coherence ratchet
    is scoped only to context_switch.rs; gdt::set_tss_rsp0 survives with four other live callers
    outside that file, so a new TSS-only publisher added elsewhere reintroduces the pre-fix divergence
    with every ratchet still green.
  • Fixture unrealism in the retire cohort (r1 finding 12, still valid but weakened; r1 finding 13).
    The 64 synthetic children are inserted into the live ProcessManager as Ready rows with an
    unbacked kernel_stack_top = 0x0080_0000; safe today only because the fixture never enqueues them
    on the scheduler. Related: the fixture's ThreadPrivilege::Kernel choice exists specifically to
    keep the O4 balance oracle from measuring the pre-existing, disclosed Box::leak in
    complete_fork for user-privilege children — that leak remains untouched, has no issue number, and
    is absent from "what Reclaim active process page-table hierarchy on exit #470 still owes."
  • x86 old_page_tables retire arm is dead in production (r1 finding 14, still valid). D3
    discloses reclaim.old_page_tables is always empty on x86 outside boot fixtures, so the arm never
    executes, yet it occupies a live call to the unbounded legacy descriptor walk on the drain path's
    source — dead-in-production code carried across the merge.
  • Leaf-release/stack-drop ordering inversion (r1 finding 15, still valid; relates to Owner-side GuardedStack reclamation of External user-stack frames (#470 residual leaf leak) #546).
    Both x86 defer sites now do defer_process_resources(process); drop(process.stack.take()), with
    release_mapped_leaves() running later in the drain — the reverse of main's prior order. Should
    a frame be owned by both GuardedStack and leaf custody, fail-closed custody would refuse the
    second return (a refusal-count regression, not an over-free) but no oracle covers this ordering and
    no x86 run has produced a refusal count. Owner-side GuardedStack reclamation of External user-stack frames (#470 residual leaf leak) #546 is the tracked issue for the underlying ownership
    split.
  • debug_assert!-only guard on page-table retention (r1 finding 16, still valid).
    release_process_resources on x86 substitutes debug_assert!(process.page_table.is_none()) for
    the prior cleanup_cow_frames/page_table.take()/abandon sequence — a no-op in --release, the
    only profile shipped. Currently safe because both callers call defer_process_resources first, but
    a future caller or reordering would silently abandon a live page table with no counter at all.
  • Epoch-leg guaranteed-block on every x86 exit (r1 finding 17, still valid). The exit-site drain
    runs microseconds after the receipt's grace target is set two epochs out, so it blocks on the Epoch
    leg with certainty on every x86 process exit; ROOT_PROOF_BLOCKED_EPOCH grows 1:1 with exits,
    contradicting the design's "small, nonzero expected" characterization for that counter. Real forward
    progress on x86 rests entirely on idle_loop.
  • ForceLiveReclaimTestGuard::arm's x86 arm is a fake read to suppress an unused-static warning
    (r1 finding 22, still valid). CLAUDE.md's "honest fixes only" policy treats this shape as
    suppression; the honest form is #[cfg(target_arch = "aarch64")] on the static and its guard.
  • Leaf-walk half of the "bounded" drain is unbounded (r1 finding 23, still valid).
    reclaim_bounded runs release_mapped_leaves() (a full four-level walk_mapped_pages with a
    FREE_FRAMES.try_lock per released leaf) to completion before retire_bounded consumes any of
    RETIRE_FRAME_BUDGET. Pre-existing on aarch64; PR-3 puts the same unbounded unit of work on x86's
    idle_loop. Fixdiff N9 extends this: the x86 exec-root walk cleanup_for_exec() newly made live by
    D3 also charges only 1 budget unit per table regardless of the address space's actual size (contrast
    the aarch64 arm, which threads &mut budget all the way through), so this leg's per-pass work is
    latency-unbounded, not merely dead-code-adjacent as in r1.
  • Uncommitted local patches on the beast x86 evidence host (final review finding 6). All beast x86
    builds/gates were produced with a locally-applied KVM-accel QEMU-launcher patch and a symlink
    repoint not present in the committed tree — if acceleration timing changes boot-order-dependent
    free-list state (the mechanism behind x86 VirtIO virtqueue assumes contiguous frames and continues when they are not (silent DMA into unowned memory) #552), a clean checkout could produce a different pinned
    custody vector and fail the gate with nobody able to reproduce the green.

Related issues

#545 — pre-existing x86 TCP-recv loopback-delivery hang (unrelated networking path, PR-2-independent,
surfaced by the same custody-gate hardening work).
#554 — referenced above (Theme 4/6) as the class of fix that would make the dormant x86 registry
runner and PM-contention window live/reachable; the RECLAIM_CONTEXT_VIOLATIONS observability
(Theme 2) and saved_context_is_kernel_frame fail-open path (Theme 6) both interact with #554's
condition.

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