Skip to content

Commit 06eddd8

Browse files
riccajasonclaude
andcommitted
reaper: defer terminating task's active page-table root + kernel stack (ADR-034 Phase A)
Fixes the confirmed clean-exit triple-fault on x86_64 + riscv64: a dying task freed the page-table root still loaded in its own CR3/satp before it yielded, and zero-on-free (318c1cd) made that fatal — freeing the active PML4 zeroes the kernel half and the next instruction triple-faults. The self-referential set a task cannot free from its own running context — the page-table root + intermediates and the kernel stack — is now captured by value and freed later by a per-CPU reaper running in a clean context. What: - New src/reaper.rs: `ReclaimItem` { root_phys, kstack_top } + a bounded per-CPU `ReclaimQueue` (`RECLAIM_QUEUE_CAPACITY` = MAX_TASKS, SCAFFOLDING- tagged: one slot per task slot makes overflow structurally impossible, ADR-034 §5) + `drain_local()`, which pops items and frees the root via `reclaim_process_page_tables` then the kernel stack via `dealloc`. - src/lib.rs: `pub mod reaper`; `PER_CPU_RECLAIM_QUEUE` (`[IrqSpinlock<ReclaimQueue>; MAX_CPUS]`) + `local_reclaim_queue()` for all three arches. IrqSpinlock (not plain Spinlock) because the pusher is a task in the Terminated-but-still-running state; isolated "Additional lock domain", never nested with a hierarchy lock. - src/microkernel/main.rs: `reaper::drain_local()` wired into all five per-CPU idle loops (BSP x86/aarch64 microkernel_loop, BSP riscv, and the three AP loops) — normal context, interrupts enabled, SCHEDULER not held. - src/process.rs: `destroy_process` no longer frees page tables inline; it returns `Option<u64>` (the root to defer), `#[must_use]`. VMA + heap + generation bump stay inline, unchanged. Stale-id test strengthened to assert the deferral return. - src/syscalls/dispatcher.rs (`handle_exit`): captures the kernel stack + transition-winner under SCHEDULER, captures the deferred root from destroy_process, then enqueues { root, kstack } under the per-CPU queue lock alone (every hierarchy lock released) immediately before the terminal yield. Enqueue-once is gated on winning the Running->Terminated transition; a full queue degrades to a logged bounded leak, never a panic or block. Safe to enqueue here: purge_task cleared current_task, so the timer ISR takes its no-switch path and the dying task is never involuntarily switched away before the item is durable. - src/memory/paging.rs (x86) + src/memory/mod.rs (non-x86): release-build active-root guard at the top of `reclaim_process_page_tables` — reads CR3 / satp / TTBR0 and refuses (frees nothing, audits, debug-asserts) if asked to free the active root. Defense-in-depth: post-Phase-A the reaper only ever frees roots the owner has yielded off, so a hit means a regression reintroduced the self-free. - src/audit/mod.rs: `AuditEventKind::ReapWouldFreeActiveRoot = 21` + builder — the runtime witness for the guard above. Purely additive (no exhaustive match over AuditEventKind exists outside the audit module). - src/loader/mod.rs: `KERNEL_STACK_SIZE` made `pub` (the reaper rebuilds the exact dealloc Layout); fixed a stale SAFETY comment (8192 -> 32768). - docs/ASSUMPTIONS.md: `RECLAIM_QUEUE_CAPACITY` SCAFFOLDING row. - tools/check-*-baseline.txt: line-number resyncs for entries my edits shifted + the descriptive "deferred" tokens inherent to a module about deferred reclamation (same treatment as the ADR-029/031/033 tokens). No new untagged bound and no new Convention-9 deferral. The two stale boot_modules.rs assumptions entries are preserved (their cleanup remains the separately-flagged Phase-0 item). Why: ADR-034 (ratified 62c59ba). The active-root self-free is the triple-fault Tier C boot-smoke surfaced; the kernel stack ("can't free the stack you're running on") was a standing leak with the same root cause. One mechanism closes both. Out of scope: - Task-slot leak (HV1): Phase B — TaskId generation + TaskExitRing + WaitTask rewrite + H5 co-fix. The slot is intentionally not deferred here. - The dedicated per-CPU reaper *task* vehicle: Phase A drains from the existing idle loops, which is the exact normal-context the ADR requires; building a ring-0 kernel-task trampoline now is new arch-specific unsafe of the same triple-fault class we are fixing, for no Phase-A correctness gain (overflow is structurally impossible; slots leak in Phase A anyway). The v1-shape drain_local() is reused verbatim by any future vehicle; promotion is deferred to ADR-034's latency trigger. - STATUS.md (kernel-stack-leak known-issue resolution, subsystem-row sync): flagged for separate application to respect the parallel-session boundary and the index-isolation gate. Verification: - make check-all: x86_64 + aarch64 + riscv64 build cleanly. - cargo test --lib: 892 passed, 0 failed. - check-assumptions / check-deferrals / check-unsafe-coverage / check-boot-panics: all pass (0 new). clippy: no new warnings, no missing_safety_doc error. - Boot before/after (x86_64, make run-quiet): the pre-fix tree HANGS at `[FDE-MOUNT] ... skipping FDE unlock` (run-quiet exit 2) — fde-mount's boot-time `sys::exit` frees the active PML4 and triple-faults. The post-fix tree sails past that exact point to `CambiOS Shell v0.1` (exit 0). Only difference between the trees is this change. Staged files: docs/ASSUMPTIONS.md src/audit/mod.rs src/lib.rs src/loader/mod.rs src/memory/mod.rs src/memory/paging.rs src/microkernel/main.rs src/process.rs src/reaper.rs src/syscalls/dispatcher.rs tools/check-assumptions-baseline.txt tools/check-deferrals-baseline.txt Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 62c59ba commit 06eddd8

12 files changed

Lines changed: 479 additions & 49 deletions

File tree

docs/ASSUMPTIONS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ These are the ones that will need to grow as the system matures. They are correc
7878
|---|---|---|---|---|
7979
| `MAX_TASKS` (per CPU) | 256 | [src/scheduler/mod.rs:126](../src/scheduler/mod.rs#L126), [src/lib.rs:157](../src/lib.rs#L157) | Heap-allocated, per-CPU. Originally raised from 32 to support multi-core workloads. | Current workloads sit far below 256. Revisit when a single CPU is regularly seeing >100 active tasks, or when AI inference services start spawning per-request worker tasks. |
8080
| `REPLY_ENDPOINT` (size) | 256 | [src/lib.rs](../src/lib.rs) | `[AtomicU32; 256]` — one slot per process, indexed by `ProcessId::slot()`. Stores the first endpoint each process registered; `handle_write` uses it as the `from` field of outgoing messages so receivers can route replies via `msg.from_endpoint()`. Sized to match `MAX_TASKS`; 1 KiB in `.bss`. Landed in Phase 4b to fix the pid-slot-as-reply-address bug that broke all service-reply paths. | Process/task slot model changes (e.g., slot count goes above 256, or multi-endpoint-per-process routing becomes a real feature rather than implicitly "first wins"). |
81+
| `RECLAIM_QUEUE_CAPACITY` (per CPU) | 256 (= `MAX_TASKS`) | [src/reaper.rs:66](../src/reaper.rs#L66) | Depth of each per-CPU deferred-reclaim queue ([ADR-034 § Decision 5](adr/034-deferred-task-resource-reclamation.md)). One slot per per-CPU task slot makes overflow structurally impossible: a CPU can hold at most `MAX_TASKS` Terminated-but-unreaped tasks at once (a slot cannot be reused until reaped), so a queue this deep always accepts the death path's push — the path never panics, blocks, or frees inline. Tracks `MAX_TASKS`, not an independent guess. Memory cost: 256 × 16 B = 4 KiB per CPU, × `MAX_CPUS` = 1 MiB zero-init `.bss` fleet-wide. | `MAX_TASKS` changes (keep them equal), or a design ever lets one CPU hold more than `MAX_TASKS` pending reclaims (it cannot today). Per-CPU high-watermark >25% under any test workload is the ADR-034 Open-Problems trigger to revisit reaper throughput, not this bound. |
8182
| `MAX_CPUS` | 256 | [src/lib.rs:91](../src/lib.rs#L91), [src/arch/x86_64/percpu.rs:23](../src/arch/x86_64/percpu.rs#L23), [src/arch/aarch64/percpu.rs:23](../src/arch/aarch64/percpu.rs#L23), [src/arch/aarch64/gic.rs:30](../src/arch/aarch64/gic.rs#L30) | Matches xAPIC 8-bit APIC ID space; statically-sized per-CPU arrays. | x2APIC support (32-bit IDs) or > 256-core targets. Not a v1 concern. |
8283
| `MAX_ENDPOINTS` | 32 | [src/ipc/mod.rs:22](../src/ipc/mod.rs#L22) | Historically matched `MAX_PROCESSES` (one endpoint per service). Sharded IPC has one shard per endpoint; static array. | `MAX_PROCESSES` is gone as of Phase 3.2a — the process table now scales with tier policy. This endpoint cap should eventually move too (tie it to `config::num_slots()` or a new `MAX_ENDPOINTS_PER_SLOT * num_slots()` computation). First Phase 3 service that needs >32 endpoints is the trigger. |
8384
| `MAX_CAPS_PER_PROCESS` (per-process capability table) | 64 (16 under `cfg(kani)`) | [src/ipc/capability.rs:133](../src/ipc/capability.rs#L133) | Bounded set for verification; cache-line-friendly linear scan. Bumped 32 → 64 at A-v.d.3 in lockstep with `MAX_ENDPOINTS` (the boot manifest grants one cap per endpoint, so the two must match or trailing endpoints silently fail their grant — the original fde-mount=32 IPC bug). `verification/capability-proofs` references this const directly. Under `cfg(kani)` it shrinks to 16: P3.7 symbolically fills a full table and `grant`'s per-call scan makes that ~O(N²), so at 64 the CBMC formula exhausts CI-runner memory (the first Tier B GitHub Actions run OOM'd). The proven "full table rejects further grants" property is size-independent, so 16 proves the same guarantee — same technique as `MAX_FRAMES`'s `cfg(kani)` shrink. | The policy service holds one capability per service it mediates, the audit consumer holds one per producer; 64 will get tight when those land at scale. |

src/audit/mod.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,16 @@ pub enum AuditEventKind {
100100
/// - `arg0`: kind discriminant (0 = Close, 1 = Revoke)
101101
/// - `arg1`: number of pages freed
102102
ChannelTeardownCompleted = 20,
103+
/// Defense-in-depth witness (ADR-034 §3): `reclaim_process_page_tables`
104+
/// was asked to free a page-table root that is *still this CPU's active
105+
/// CR3/satp/TTBR0*. Post-Phase-A the reaper only frees roots the owning
106+
/// task has yielded off, so this never fires in correct operation —
107+
/// emitting it means a regression reintroduced the active-root self-free.
108+
/// The reclaim refuses (frees nothing) in both build profiles; a
109+
/// `debug_assert!` additionally fast-fails dev builds.
110+
/// - `subject_pid`: 0 (kernel-context event)
111+
/// - `arg0`: the active root physical address the free was refused for
112+
ReapWouldFreeActiveRoot = 21,
103113
}
104114

105115
/// ARCHITECTURAL: `cluster_revoked` event's `arg2` discriminant for
@@ -643,6 +653,30 @@ impl RawAuditEvent {
643653
)
644654
}
645655

656+
/// `REAP_WOULD_FREE_ACTIVE_ROOT`: defense-in-depth witness that a
657+
/// page-table reclaim was refused because the root is the active
658+
/// CR3/satp/TTBR0 (ADR-034 §3). Kernel-context event (no subject pid).
659+
///
660+
/// - `arg0`: the active root physical address the free was refused for
661+
pub fn reap_would_free_active_root(
662+
active_root_phys: u64,
663+
timestamp: u64,
664+
sequence: u32,
665+
) -> Self {
666+
Self::build(
667+
AuditEventKind::ReapWouldFreeActiveRoot,
668+
0,
669+
sequence,
670+
timestamp,
671+
0,
672+
0,
673+
active_root_phys,
674+
0,
675+
0,
676+
0,
677+
)
678+
}
679+
646680
/// `POLICY_QUERY`: when the policy service is consulted (sampled).
647681
///
648682
/// - `subject_pid`: the process being queried about

src/lib.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ pub mod arch;
3333
pub mod loader;
3434
pub mod syscalls;
3535
pub mod process;
36+
pub mod reaper;
3637
pub mod acpi;
3738
pub mod fs;
3839
pub mod crypto;
@@ -724,6 +725,44 @@ pub fn local_frame_cache() -> &'static Spinlock<FrameCache> {
724725
&PER_CPU_FRAME_CACHE[cpu_id]
725726
}
726727

728+
/// Per-CPU deferred-reclaim queues (ADR-034 §3). Each CPU owns
729+
/// `PER_CPU_RECLAIM_QUEUE[cpu_id]`. A dying task's death path pushes its
730+
/// `{ page-table root, kernel stack }` here (under this lock alone — never
731+
/// nested with a hierarchy lock); the per-CPU idle-loop reaper
732+
/// (`reaper::drain_local`) pops and frees them from a clean context.
733+
///
734+
/// `IrqSpinlock`, not a plain `Spinlock`, because the pusher is a task in the
735+
/// `Terminated`-but-still-running state: disabling interrupts across the brief
736+
/// push removes any question of a timer-ISR or idle-drain reentrancy on this
737+
/// CPU's queue while the push is in flight. Isolated "Additional lock domain"
738+
/// (like `PER_CPU_FRAME_CACHE`) — see CLAUDE.md § Lock Ordering.
739+
pub static PER_CPU_RECLAIM_QUEUE: [IrqSpinlock<reaper::ReclaimQueue>; MAX_CPUS] =
740+
[const { IrqSpinlock::new(reaper::ReclaimQueue::new()) }; MAX_CPUS];
741+
742+
/// Get the current CPU's deferred-reclaim queue lock.
743+
#[cfg(target_arch = "x86_64")]
744+
pub fn local_reclaim_queue() -> &'static IrqSpinlock<reaper::ReclaimQueue> {
745+
// SAFETY: GS base initialized at boot; cpu_id is a pure read.
746+
let cpu_id = unsafe { arch::x86_64::percpu::current_percpu().cpu_id() } as usize;
747+
&PER_CPU_RECLAIM_QUEUE[cpu_id]
748+
}
749+
750+
/// Get the current CPU's deferred-reclaim queue lock (AArch64).
751+
#[cfg(target_arch = "aarch64")]
752+
pub fn local_reclaim_queue() -> &'static IrqSpinlock<reaper::ReclaimQueue> {
753+
// SAFETY: TPIDR_EL1 initialized at boot; cpu_id is a pure read.
754+
let cpu_id = unsafe { arch::aarch64::percpu::current_percpu().cpu_id() } as usize;
755+
&PER_CPU_RECLAIM_QUEUE[cpu_id]
756+
}
757+
758+
/// Get the current hart's deferred-reclaim queue lock (RISC-V).
759+
#[cfg(target_arch = "riscv64")]
760+
pub fn local_reclaim_queue() -> &'static IrqSpinlock<reaper::ReclaimQueue> {
761+
// SAFETY: `tp` initialized at boot; cpu_id is a pure read.
762+
let cpu_id = unsafe { arch::riscv64::percpu::current_percpu().cpu_id() } as usize;
763+
&PER_CPU_RECLAIM_QUEUE[cpu_id]
764+
}
765+
727766
/// Allocate a physical frame, preferring the local CPU's cache.
728767
///
729768
/// Fast path (no global lock): pop from per-CPU cache.

src/loader/mod.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,11 @@ use core::mem::size_of;
4242
/// observed usage and costs MAX_TASKS × 32 KiB ≈ 8 MiB of kernel memory.
4343
/// Replace when: per-kstack guard-page mapping lands (then this becomes a
4444
/// HARDWARE-bounded value driven by the page-fault report, not a guess).
45-
const KERNEL_STACK_SIZE: usize = 32 * 1024;
45+
///
46+
/// `pub` so `reaper::free_kernel_stack` can reconstruct the exact `Layout`
47+
/// when freeing a terminated task's stack (ADR-034 Phase A) — the deferred
48+
/// free must use the same size this allocation used.
49+
pub const KERNEL_STACK_SIZE: usize = 32 * 1024;
4650

4751
/// SCAFFOLDING: default user stack 16 pages (64 KiB).
4852
/// Why: conservative default that fits all current services.
@@ -690,7 +694,7 @@ pub fn load_elf_process(
690694
let kstack_layout = Layout::from_size_align(KERNEL_STACK_SIZE, 16)
691695
.map_err(|_| LoaderError::KernelStackAllocationFailed)?;
692696

693-
// SAFETY: Layout is valid (KERNEL_STACK_SIZE=8192, align=16).
697+
// SAFETY: Layout is valid (KERNEL_STACK_SIZE=32768, align=16).
694698
let kstack_base = unsafe { alloc(kstack_layout) };
695699
if kstack_base.is_null() {
696700
return Err(LoaderError::KernelStackAllocationFailed);

src/memory/mod.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,35 @@ pub mod paging {
443443
return 0;
444444
}
445445

446+
// ADR-034 §3 release-build active-root guard. Freeing the L0 root that
447+
// is still this hart's active satp / this CPU's active TTBR0 unmaps the
448+
// running address space (the confirmed triple-fault on RISC-V; on
449+
// AArch64 it would brick the TTBR0 regime). Post-Phase-A the reaper
450+
// only frees roots the owning task has yielded off, so a hit here means
451+
// a regression reintroduced the active-root self-free. Refuse (free
452+
// nothing) in BOTH build profiles. See x86_64 `paging.rs` for the full
453+
// rationale.
454+
#[cfg(not(test))]
455+
{
456+
// SAFETY: HHDM is set and a valid root exists by the time any
457+
// process page table is reclaimed; reading the root register
458+
// (satp / TTBR0_EL1) is a pure read.
459+
let active = unsafe { ap::active_root() };
460+
if active == l0_phys {
461+
crate::audit::emit(crate::audit::RawAuditEvent::reap_would_free_active_root(
462+
l0_phys,
463+
crate::audit::now(),
464+
0,
465+
));
466+
debug_assert!(
467+
false,
468+
"reclaim_process_page_tables called on the active root {:#x}",
469+
l0_phys
470+
);
471+
return 0;
472+
}
473+
}
474+
446475
let mut freed = 0usize;
447476

448477
// SAFETY: l0_phys is a valid L0 page table from

src/memory/paging.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,35 @@ pub fn reclaim_process_page_tables(
292292
return 0;
293293
}
294294

295+
// ADR-034 §3 release-build active-root guard. Freeing the PML4 that is
296+
// still this CPU's active CR3 unmaps the running address space — the
297+
// next TLB miss faults on an unmapped #PF handler → #DF → triple fault
298+
// (the confirmed bug this whole mechanism exists to prevent). Post-Phase-A
299+
// the reaper only frees roots the owning task has yielded off, so a hit
300+
// here means a regression reintroduced the active-root self-free. Refuse
301+
// (free nothing) in BOTH build profiles; the read is invisible cost on a
302+
// path that only runs at teardown. `debug_assert!` adds dev fast-fail.
303+
#[cfg(not(test))]
304+
{
305+
let cr3: u64;
306+
// SAFETY: reading CR3 is a pure, ring-0-legal read; no memory touched.
307+
unsafe { core::arch::asm!("mov {}, cr3", out(reg) cr3, options(nostack, nomem)); }
308+
// CR3 bits[51:12] hold the PML4 base; mask off PCD/PWT/PCID low bits.
309+
if (cr3 & 0x000F_FFFF_FFFF_F000) == pml4_phys {
310+
crate::audit::emit(crate::audit::RawAuditEvent::reap_would_free_active_root(
311+
pml4_phys,
312+
crate::audit::now(),
313+
0,
314+
));
315+
debug_assert!(
316+
false,
317+
"reclaim_process_page_tables called on the active CR3 root {:#x}",
318+
pml4_phys
319+
);
320+
return 0;
321+
}
322+
}
323+
295324
let hhdm = crate::hhdm_offset();
296325
let mut freed = 0usize;
297326

src/microkernel/main.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,9 @@ unsafe extern "C" fn kmain_riscv64(hart_id: u64, dtb_phys: u64) -> ! {
907907
// another task, the trap vector's `mv sp, a0 ; sret` swaps out
908908
// this stack; we resume here when the scheduler picks idle again.
909909
loop {
910+
// ADR-034 §3: reclaim exited tasks' page-table roots + kernel stacks
911+
// from this hart's deferred-reclaim queue, in normal context.
912+
cambios_core::reaper::drain_local();
910913
// SAFETY: wfi is always legal in S-mode. On a hart with SIE
911914
// set, it blocks until the next interrupt (timer or external)
912915
// fires, at which point control re-enters the trap vector.
@@ -1037,6 +1040,8 @@ unsafe extern "C" fn kmain_riscv64_ap(cpu_index: u64, hart_id: u64) -> ! {
10371040
// SAFETY: trap vector installed; PerCpu/timer/scheduler all live.
10381041
unsafe { cambios_core::arch::riscv64::trap::enable_interrupts(); }
10391042
loop {
1043+
// ADR-034 §3: drain this hart's deferred-reclaim queue.
1044+
cambios_core::reaper::drain_local();
10401045
// SAFETY: wfi is always legal in S-mode with SIE set.
10411046
unsafe { core::arch::asm!("wfi", options(nostack, nomem, preserves_flags)); }
10421047
}
@@ -2186,6 +2191,8 @@ unsafe extern "C" fn ap_entry(cpu: &limine::mp::Cpu) -> ! {
21862191
x86_64::instructions::interrupts::enable();
21872192

21882193
loop {
2194+
// ADR-034 §3: drain this CPU's deferred-reclaim queue.
2195+
cambios_core::reaper::drain_local();
21892196
hlt();
21902197
}
21912198
}
@@ -2275,6 +2282,8 @@ unsafe extern "C" fn ap_entry(cpu: &limine::mp::Cpu) -> ! {
22752282
}
22762283

22772284
loop {
2285+
// ADR-034 §3: drain this CPU's deferred-reclaim queue.
2286+
cambios_core::reaper::drain_local();
22782287
cambios_core::wfi();
22792288
}
22802289
}
@@ -2637,6 +2646,12 @@ fn microkernel_loop() -> ! {
26372646
// (internally throttled to once per BALANCE_INTERVAL_TICKS)
26382647
cambios_core::try_load_balance();
26392648

2649+
// ADR-034 §3: drain this CPU's deferred-reclaim queue — free the
2650+
// page-table roots + kernel stacks of tasks that exited on this CPU,
2651+
// now that they have yielded off them. Normal context, no hierarchy
2652+
// lock held: the safe place to take FRAME_ALLOCATOR + free.
2653+
cambios_core::reaper::drain_local();
2654+
26402655
// Halt/wait CPU until next interrupt (timer, keyboard, etc.)
26412656
#[cfg(target_arch = "x86_64")]
26422657
hlt();

src/process.rs

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -992,33 +992,48 @@ impl ProcessTable {
992992
/// Kernel stack deallocation is deferred to a separate cleanup
993993
/// pass (bounded leak, requires scheduler-level deferred-free
994994
/// mechanism). See STATUS.md.
995+
/// Reclaim a terminating process's inline-freeable resources and return
996+
/// the page-table root to be freed **later** by the per-CPU reaper.
997+
///
998+
/// VMA-tracked frames and the heap are freed inline here. The page-table
999+
/// root (PML4 / satp L0 / TTBR0 L0) is **not** — it is the dying task's
1000+
/// *active* root, and freeing it inline unmaps the running address space
1001+
/// (the confirmed triple-fault, ADR-034 Context). Instead the root is
1002+
/// returned so the caller can hand it to `reaper::drain_local`, which
1003+
/// frees it from a clean context after the task has yielded off it.
1004+
///
1005+
/// Returns `Some(root_phys)` for a process with a per-process page table,
1006+
/// `None` for a kernel task (`cr3 == 0`) or a stale / unknown / unoccupied
1007+
/// `process_id` (nothing to defer).
1008+
#[must_use = "the returned page-table root must be enqueued for the reaper, \
1009+
or it leaks"]
9951010
pub fn destroy_process(
9961011
&mut self,
9971012
process_id: ProcessId,
9981013
frame_alloc: &mut FrameAllocator,
999-
) {
1014+
) -> Option<u64> {
10001015
let idx = process_id.slot() as usize;
10011016
if idx >= self.processes.len() {
1002-
return;
1017+
return None;
10031018
}
10041019
// Reject a stale `process_id`: destroying by slot alone would let a
10051020
// stale id reap the slot's *current* occupant (and bump its
10061021
// generation), a confused-deputy that revokes a live process.
10071022
if self.generations[idx] != process_id.generation() {
1008-
return;
1023+
return None;
10091024
}
10101025
if let Some(mut desc) = self.processes[idx].take() {
10111026
// Step 1: Reclaim VMA-tracked user-space regions.
10121027
// Unmaps pages from the process page table and frees
1013-
// physical frames. Must precede page table reclaim.
1028+
// physical frames. Must precede page table reclaim, and runs
1029+
// while the page table is still intact (and still the dying
1030+
// task's active root).
10141031
let _vma_count = desc.reclaim_user_vmas(frame_alloc);
10151032

1016-
// Step 2: Reclaim page table frames (PML4 + intermediates).
1017-
// Only for processes with a per-process page table (cr3 != 0).
1018-
#[cfg(not(test))]
1019-
if desc.cr3 != 0 {
1020-
paging::reclaim_process_page_tables(desc.cr3, frame_alloc);
1021-
}
1033+
// Step 2: Page-table frames (root + intermediates) are NOT freed
1034+
// here — see the doc comment. Capture the root for the reaper.
1035+
// `None` when cr3 == 0 (kernel task, no per-process table).
1036+
let deferred_root = if desc.cr3 != 0 { Some(desc.cr3) } else { None };
10221037

10231038
// Step 3: Reclaim the heap region.
10241039
let _ = desc.reclaim_heap(frame_alloc);
@@ -1027,6 +1042,10 @@ impl ProcessTable {
10271042
// slot gets a distinct ProcessId. Wrapping is intentional —
10281043
// u32 gives ~4 billion reuses per slot before wrap.
10291044
self.generations[idx] = self.generations[idx].wrapping_add(1);
1045+
1046+
deferred_root
1047+
} else {
1048+
None
10301049
}
10311050
}
10321051

@@ -1522,18 +1541,28 @@ mod tests {
15221541
table.generations[0] = 5;
15231542
let mut fa = FrameAllocator::new();
15241543

1525-
// A stale id (generation 4) must not reap the live occupant (gen 5).
1544+
// A stale id (generation 4) must not reap the live occupant (gen 5),
1545+
// and must defer no page-table root.
15261546
let stale = ProcessId::new(0, 4);
1527-
table.destroy_process(stale, &mut fa);
1547+
assert_eq!(
1548+
table.destroy_process(stale, &mut fa),
1549+
None,
1550+
"stale id must defer nothing"
1551+
);
15281552
assert!(
15291553
table.processes[0].is_some(),
15301554
"stale id reaped the live occupant"
15311555
);
15321556
assert_eq!(table.generations[0], 5, "stale destroy must not bump gen");
15331557

1534-
// The current id destroys it and bumps the generation.
1558+
// The current id destroys it, bumps the generation, and returns the
1559+
// page-table root for the reaper to free (ADR-034 Phase A).
15351560
let current = ProcessId::new(0, 5);
1536-
table.destroy_process(current, &mut fa);
1561+
assert_eq!(
1562+
table.destroy_process(current, &mut fa),
1563+
Some(0x2000_0000),
1564+
"destroy must defer the descriptor's cr3 root"
1565+
);
15371566
assert!(table.processes[0].is_none());
15381567
assert_eq!(table.generations[0], 6);
15391568
}

0 commit comments

Comments
 (0)