From 48277759e90ed81fb843987bfc47aef516b4502a Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 3 Oct 2026 23:25:43 +0200 Subject: [PATCH 1/5] heap_growth_windows: the windows across a burst that grows the kernel heap A T14 row for the reading the heap-growth fix is judged by. The kernel heap grows by one 2 MiB frame at a time inside the allocation that found it full, and a thread's 128 KiB kernel stack comes off that heap, so `test_rs_heap_growth` creates 256 threads that stay alive: 32 MiB of stacks, each growth inside the `SYS_THREAD_SPAWN` that needed it, with interrupts masked for the whole syscall. A `mask-windows` kernel reports every CPU's longest windows at each process exit, so the burst is bracketed by two. `echo` exits before it and takes the load's spawn (1.4 ms on the spawning CPU in the T14's windowscase boots) and its child's; the child, parked on its stdin since before `echo` ran, exits right after it, and is the first exit recorded under the load's name, which is the report `windows_under` reads. No thread ends inside the bracket: in the herd's report of `rename-full-r2-readback/windowscase/kernel.log` the threads' ends read 7.7 ms on every CPU beside `tlb: ... max=7742us`. `windows_on_metal` takes the load and the word it prints the reading under, so the herd's row prints `herd irqs_off_ns=` as `issues/kernel/syscall-preemption-is-incidental.md` cites it. The row is a boot of its own: the herd's teardown would land in the burst's report, and the heap the herd grew would leave the burst nothing to grow. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- tests/checks.rs | 2 +- tests/toyos-rust-tests/src/bin/heap_growth.rs | 67 +++++++++++++++++++ tests/toyos.rs | 48 ++++++++++--- 3 files changed, 107 insertions(+), 10 deletions(-) create mode 100644 tests/toyos-rust-tests/src/bin/heap_growth.rs diff --git a/tests/checks.rs b/tests/checks.rs index 88cd2bdc418..98352bb333e 100644 --- a/tests/checks.rs +++ b/tests/checks.rs @@ -427,7 +427,7 @@ mod checks { let opening = [report((100, 100), read_back), exit("pwd")].concat(); let own = [report((3_000_000, 1_100_000), (700_000, 600_000)), exit(WINDOWS_LOAD)].concat(); let good = [hold.as_str(), &first, &opening, &own].concat(); - let exited = windows_load_exited(); + let exited = load_exited(WINDOWS_LOAD); mask_windows(&good, 2).map_err(|e| format!("the good capture was refused: {e}"))?; let unpaired = |what: &str, capture: &str, says: &str| match mask_windows(capture, 2) { diff --git a/tests/toyos-rust-tests/src/bin/heap_growth.rs b/tests/toyos-rust-tests/src/bin/heap_growth.rs new file mode 100644 index 00000000000..c2676281836 --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/heap_growth.rs @@ -0,0 +1,67 @@ +//! **A burst of thread creations that grows the kernel heap, in a windows +//! report of its own.** +//! +//! A thread costs the kernel a 128 KiB stack off its heap, so [`THREADS`] +//! alive at once grow it by whole 2 MiB frames, each growth inside the +//! `SYS_THREAD_SPAWN` that needed it. A `mask-windows` kernel reports every +//! CPU's longest windows at each process exit, so two exits bracket the burst: +//! `echo`'s before it, which takes this process's spawn and its child's, and +//! the child's after it, the first exit recorded under this binary's name, +//! whose report `heap_growth_windows` reads. No thread ends inside the +//! bracket: an ending thread's TLS unmap is a shootdown, a window on every CPU. + +use std::io::{Read, Write}; +use std::process::{Command, Stdio}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::thread; + +const SELF_PATH: &str = "/system/bin/test_rs_heap_growth"; + +/// 32 MiB of kernel stacks. +const THREADS: usize = 256; + +static RELEASED: AtomicBool = AtomicBool::new(false); + +fn main() { + if std::env::args().nth(1).as_deref() == Some("bracket") { + let mut stdout = std::io::stdout(); + stdout.write_all(b"!").and_then(|()| stdout.flush()).expect("the bracket says it started"); + std::io::stdin().read_to_end(&mut Vec::new()).expect("the bracket waits for its stdin to close"); + return; + } + + let mut bracket = Command::new(SELF_PATH) + .arg("bracket") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .expect("spawn the bracket"); + // Its start, too, is in `echo`'s report rather than the burst's. + let mut started = [0u8]; + bracket.stdout.take().expect("the bracket's stdout").read_exact(&mut started).expect("the bracket started"); + let opened = Command::new("/system/bin/echo").status().expect("run echo"); + assert!(opened.success(), "echo: {opened}"); + + let parked: Vec<_> = (0..THREADS) + .map(|_| { + thread::spawn(|| { + while !RELEASED.load(Ordering::Acquire) { + thread::park(); + } + }) + }) + .collect(); + + drop(bracket.stdin.take()); + let closed = bracket.wait().expect("wait for the bracket"); + assert!(closed.success(), "the bracket: {closed}"); + + RELEASED.store(true, Ordering::Release); + for thread in &parked { + thread.thread().unpark(); + } + for thread in parked { + thread.join().expect("a parked thread"); + } + println!("heap_growth: {THREADS} threads alive at once between two process exits"); +} diff --git a/tests/toyos.rs b/tests/toyos.rs index 486edcdcd90..86834d04a4f 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -80,6 +80,10 @@ const RUST_SKIP: &[&str] = &[ // its 256 threads' stacks would crowd every member after it in one boot. // The `mask_windows` metal row runs it. "ring_park_herd", + // Its product is the windows across its burst, and its 256 threads crowd + // every member after it as the herd's do. The `heap_growth_windows` metal + // row runs it. + "heap_growth", // It asserts nothing: it is half a second of an idle machine, which the // same row reads the windows across. "idle_span", @@ -259,7 +263,16 @@ const METAL: &[(&str, metal::Metal)] = &[ // The windows on the machine that owes them: every CPU reported beside // its census, and a held window read back. "mask_windows", - metal::Metal { arms: WINDOWSCASE, judge: |b| windows_on_metal(b[0]) }, + metal::Metal { arms: WINDOWSCASE, judge: |b| windows_on_metal(b[0], WINDOWS_LOAD, "herd") }, + ), + ( + // The same reading across a burst that grows the kernel heap by whole + // frames, each growth inside the thread spawn that needed it. + "heap_growth_windows", + metal::Metal { + arms: HEAPGROWCASE, + judge: |b| windows_on_metal(b[0], HEAP_GROWTH_LOAD, "burst"), + }, ), ( "mkdir_cap", @@ -732,9 +745,25 @@ const WINDOWSCASE: &[metal::Arm] = &[metal::Arm { /// The load `mask_windows` reads the windows under. const WINDOWS_LOAD: &str = "test_rs_ring_park_herd"; -/// The head of the kernel's record of [`WINDOWS_LOAD`]'s exit. -fn windows_load_exited() -> String { - format!("{}{} pid=", toyos_build::bootlog::EXIT, toyos_build::bootlog::recorded_name(WINDOWS_LOAD)) +/// [`WINDOWSCASE`] with [`HEAP_GROWTH_LOAD`] for its load, on a boot of its +/// own: the herd's teardown would be in the burst's report, and the heap the +/// herd grew would leave the burst nothing to grow. +const HEAPGROWCASE: &[metal::Arm] = &[metal::Arm { + features: toyos_build::build::MASK_WINDOWS_KERNEL, + ..metal::once( + "heapgrowcase", + "tests/testcases", + &[], + &["test_rs_idle_span", "pwd", HEAP_GROWTH_LOAD], + ) +}]; + +/// The load `heap_growth_windows` reads the windows under. +const HEAP_GROWTH_LOAD: &str = "test_rs_heap_growth"; + +/// The head of the kernel's record of a process of `load`'s exit. +fn load_exited(load: &str) -> String { + format!("{}{} pid=", toyos_build::bootlog::EXIT, toyos_build::bootlog::recorded_name(load)) } /// A `logkeeper` that leaves soundserver's ring unread until the job says the tone played. @@ -2536,17 +2565,18 @@ fn irq_census(capture: &str) -> Result<(), String> { Ok(()) } -/// The T14's windows: [`mask_windows`]' verdict with every CPU reporting, and +/// The T14's windows under `load`, its reading printed as `what`'s: +/// [`mask_windows`]' verdict with every CPU reporting, and /// `common::irqcensus::windows_under`'s. Its durations are printed and none is /// recorded: one boot's longest window is no baseline for the next. -fn windows_on_metal(boot: &metal::Readback) -> Result<(), String> { - boot.job_passed(WINDOWS_LOAD)?; +fn windows_on_metal(boot: &metal::Readback, load: &str, what: &str) -> Result<(), String> { + boot.job_passed(load)?; let kernel = boot.kernel(); let cpus = boot.cpus()?; mask_windows(kernel.text(), cpus)?; - let read = common::irqcensus::windows_under(kernel.text(), cpus, &windows_load_exited())?; + let read = common::irqcensus::windows_under(kernel.text(), cpus, &load_exited(load))?; eprintln!( - " [windows] held irqs_off_ns={} preempt_off_ns={}; herd irqs_off_ns={} preempt_off_ns={}", + " [windows] held irqs_off_ns={} preempt_off_ns={}; {what} irqs_off_ns={} preempt_off_ns={}", read.held.0, read.held.1, read.load.0, read.load.1 ); Ok(()) From 76c01e36f4e8fdf158df005d361791834bbb784d Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 3 Oct 2026 23:28:34 +0200 Subject: [PATCH 2/5] The kernel heap acquires and zeroes no frame while its lock is held When dlmalloc ran out of room it called `KernelPageSource::alloc` inside `dlmalloc.lock()`, which called `pmm::alloc_page`: a linear scan of the frame bitmap under the PMM's lock, nested in the heap's, and then a 2 MiB `write_bytes` of the frame. The growing CPU did that with interrupts masked whenever it was in a syscall or the timer path's pass, and every other CPU that wanted the heap spun with preemption off for as long. The zeroing bought nothing: dlmalloc reads `allocates_zeros` only for mmapped chunks (`calloc_must_clear`), which this port never makes, and `GlobalAlloc::alloc_zeroed`'s default writes its own zeros. The page source now grows the heap only by the frame its lock holder offered (`offer`, set and taken back inside one hold), and refuses a growth when there is none. `KernelAllocator::alloc` asks once without a frame; on a null it takes one off the PMM with the lock released (`pmm::alloc_heap_page`, the scan without the zeroing) and asks again, offering it, and a frame that second hold did not take goes back to the PMM after the lock drops. A null from that second ask is a refusal, as a null always was: dlmalloc calls its page source at most once per `malloc` (`sys_alloc` from `malloc`, `memalign` through `malloc`), and the offer is that hold's alone, so a CPU growing the heap meanwhile can neither take it nor make the retry need a second. `allocates_zeros` now says false, which is what the frames are. Why this shape over its alternatives: a refill reserve kept ahead of need would acquire a frame on paths that never grow and would still need a slow path when it runs dry; a page source that dropped and retook the lock from inside dlmalloc would run with dlmalloc's state half-updated. Retrying after the lock drops touches the PMM exactly when a growth was refused, and leaves dlmalloc's state whole between the two holds: `sys_alloc` returns before it writes anything when its page source answers null. A frame now reaches the heap as its last owner left it, which can be another process's bytes. The heap answers uninitialized memory either way, and a chunk it reuses already holds whatever was freed into it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- kernel/src/mm/alloc.rs | 60 ++++++++++++++++++++++++++++++------------ kernel/src/mm/pmm.rs | 20 +++++++++----- 2 files changed, 57 insertions(+), 23 deletions(-) diff --git a/kernel/src/mm/alloc.rs b/kernel/src/mm/alloc.rs index 8f2f610cf44..0e58fcc7ce6 100644 --- a/kernel/src/mm/alloc.rs +++ b/kernel/src/mm/alloc.rs @@ -1,4 +1,14 @@ +//! The kernel heap: `dlmalloc` behind one ticket lock, grown by whole 2 MiB +//! frames. +//! +//! **No frame is acquired or zeroed while the heap lock is held.** A growth +//! takes only the frame its hold was offered, and a hold is offered one only +//! after a hold without one failed: `KernelAllocator::alloc` then takes a +//! frame off the PMM with the lock released and asks again, and a frame the +//! second hold did not take goes back once the lock is released. + use core::alloc::{GlobalAlloc, Layout}; +use core::cell::Cell; use core::sync::atomic::{AtomicU8, Ordering}; use super::MAX_HEAP_ALLOC; @@ -7,24 +17,26 @@ use super::PAGE_2M; use super::pmm; /// Invariant: no method here may panic — it runs inside `dlmalloc.lock()`, which cannot unwind. -struct KernelPageSource; +struct KernelPageSource { + /// Set and taken back inside one hold of `dlmalloc.lock()`. + offer: Cell>, +} -// SAFETY: `alloc` only ever hands out whole `PAGE_2M` pages from `pmm::alloc_page`, and `free` is the only path that returns one. +// SAFETY: `alloc` only ever hands out the whole `PAGE_2M` frame `offer` holds, and `free` is the only path that returns one. unsafe impl dlmalloc::Allocator for KernelPageSource { fn alloc(&self, size: usize) -> (*mut u8, usize, u32) { if size > PAGE_2M as usize { return (core::ptr::null_mut(), 0, 0); } - if let Some(page) = pmm::alloc_page(pmm::Category::KernelHeap) { - let ptr = page.direct_map().as_mut_ptr::(); - #[expect(clippy::disallowed_methods, reason = "dlmalloc owns the page from here on")] - core::mem::forget(page); - #[cfg(feature = "heap-sweep")] - pages::add(ptr as u64); - (ptr, PAGE_2M as usize, 0) - } else { - (core::ptr::null_mut(), 0, 0) - } + let Some(page) = self.offer.take() else { + return (core::ptr::null_mut(), 0, 0); + }; + let ptr = page.direct_map().as_mut_ptr::(); + #[expect(clippy::disallowed_methods, reason = "dlmalloc owns the page from here on")] + core::mem::forget(page); + #[cfg(feature = "heap-sweep")] + pages::add(ptr as u64); + (ptr, PAGE_2M as usize, 0) } fn remap(&self, _ptr: *mut u8, _oldsize: usize, _newsize: usize, _can_move: bool) -> *mut u8 { @@ -48,7 +60,7 @@ unsafe impl dlmalloc::Allocator for KernelPageSource { } fn allocates_zeros(&self) -> bool { - true + false } fn page_size(&self) -> usize { @@ -509,7 +521,9 @@ use crate::sync::Lock; impl KernelAllocator { const fn new() -> Self { Self { - dlmalloc: Lock::new(dlmalloc::Dlmalloc::new_with_allocator(KernelPageSource)), + dlmalloc: Lock::new(dlmalloc::Dlmalloc::new_with_allocator(KernelPageSource { + offer: Cell::new(None), + })), phase: AtomicU8::new(PHASE_UNINIT), } } @@ -531,10 +545,22 @@ unsafe impl GlobalAlloc for KernelAllocator { layout.size(), MAX_HEAP_ALLOC); // Bands are armed after the lock drops, so a failing band never fires inside `dlmalloc.lock()`. let outer = tripwire::outer(layout); - let base = { - let mut dlm = self.dlmalloc.lock(); - dlm.malloc(outer.size(), outer.align()) + let malloc = |frame| { + let (base, untaken) = { + let mut dlm = self.dlmalloc.lock(); + dlm.allocator_mut().offer.set(frame); + let base = dlm.malloc(outer.size(), outer.align()); + (base, dlm.allocator_mut().offer.take()) + }; + drop(untaken); + base }; + let mut base = malloc(None); + if base.is_null() { + if let Some(frame) = pmm::alloc_heap_page() { + base = malloc(Some(frame)); + } + } tripwire::arm(base, layout) } // Any byte other than the three phases is corrupt state — fail loudly rather than serve it as READY. diff --git a/kernel/src/mm/pmm.rs b/kernel/src/mm/pmm.rs index fe23dfc66fe..390dd3bbbb6 100644 --- a/kernel/src/mm/pmm.rs +++ b/kernel/src/mm/pmm.rs @@ -264,6 +264,20 @@ pub(super) fn init(entries: &[MemoryMapEntry], reserved: &[Region]) { /// Allocate one 2MB physical page. Does not heap-allocate (safe to call from the allocator). pub fn alloc_page(cat: Category) -> Option { + let page = claim(cat)?; + // SAFETY: `claim` just took the frame off the bitmap, so it is unaliased, and the direct map covers every address the bitmap can name. + unsafe { + core::ptr::write_bytes(page.direct_map().as_mut_ptr::(), 0, PAGE_2M as usize); + } + Some(page) +} + +/// One 2MB physical page for the kernel heap, holding what its last owner left in it: the heap answers uninitialized memory, and `alloc_zeroed` writes its own zeros. +pub(super) fn alloc_heap_page() -> Option { + claim(Category::KernelHeap) +} + +fn claim(cat: Category) -> Option { let mut bm = BITMAP.lock(); if bm.free_count == 0 { return None; } let start = bm.next_hint; @@ -275,12 +289,6 @@ pub fn alloc_page(cat: Category) -> Option { bm.next_hint = if idx + 1 < bm.page_count { idx + 1 } else { 0 }; let phys = bm.idx_to_phys(idx); drop(bm); - // SAFETY: `idx` was just claimed under `BITMAP`'s lock, so it is unaliased, and the direct map covers every address the bitmap can name. - unsafe { - core::ptr::write_bytes( - DirectMap::from_phys(phys).as_mut_ptr::(), 0, PAGE_2M as usize, - ); - } CATEGORY_STATS[cat as usize].alloc_pages.fetch_add(1, Ordering::Relaxed); return Some(PhysPage { phys, category: cat as u8 }); } From 2c7a264171d758829c74e1b771362afab30c06d8 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 3 Oct 2026 23:31:34 +0200 Subject: [PATCH 3/5] issues: a TLS block is zeroed twice with interrupts masked Found reading the windows `heap_growth_windows` reads: `build_combined` takes its frames from `pmm::alloc_contiguous`, which zeroes each one, and zeroes the block again with `write_bytes`, inside a syscall whose interrupts are masked throughout. Filed rather than fixed: it is not the heap's growth. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...-is-zeroed-twice-with-interrupts-masked.md | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 issues/kernel/a-tls-block-is-zeroed-twice-with-interrupts-masked.md diff --git a/issues/kernel/a-tls-block-is-zeroed-twice-with-interrupts-masked.md b/issues/kernel/a-tls-block-is-zeroed-twice-with-interrupts-masked.md new file mode 100644 index 00000000000..0bbae91ec65 --- /dev/null +++ b/issues/kernel/a-tls-block-is-zeroed-twice-with-interrupts-masked.md @@ -0,0 +1,22 @@ +--- +status: open +kind: defect +opened: 2026-10-03 +--- + +# A TLS block is zeroed twice with interrupts masked + +`kernel/src/loader/tls.rs`'s `build_combined` takes its frames from +`PageAlloc::new`, which is `pmm::alloc_contiguous`, which zeroes every 2 MiB +frame it hands out; then it zeroes the whole block again with `write_bytes`. +Every `SYS_THREAD_SPAWN` (`process::spawn_thread`) and every `SYS_SPAWN` +(`loader`) builds one, and a syscall runs with interrupts masked from entry to +exit (`issues/kernel/syscall-preemption-is-incidental.md`), so each spawn pays +two writes of the block, at least 2 MiB each, in one interrupts-off window. + +By reading, not measured. The T14's `heap_growth_windows` row reads the longest +window across 256 thread spawns, of which these writes are part. + +Owner: `issues/kernel/toyos-beats-linuxs-latency-on-the-t14.md`. + +**Exit**: `build_combined` zeroes no byte the PMM already zeroed. From c9b9811acc70f04cc5ca31094a1effdbcf785c32 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 00:10:39 +0200 Subject: [PATCH 4/5] Answer review round 1: the heap_growth_windows row goes, and claim carries the dirty-frame contract The T14 read both arms of heap_growth_windows at 2c7a26417 and the judge passed on each: base 953,617 ns, fix 957,092 ns, both on cpu7. The row records no duration, so it cannot fail on any claim the fix makes, and its reading did not see the fix. Its load, its arm, its RUST_SKIP entry and the load/what parameters it added to windows_on_metal go with it; tests/toyos.rs and tests/checks.rs are main's again. pmm: alloc_heap_page was a one-caller wrapper over claim(KernelHeap). claim is now pub(super) and carries the dirty-frame contract and the no-heap-allocation clause, which named the allocator as alloc_page's caller after this branch removed that call. The TLS issue no longer cites the deleted row. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...-is-zeroed-twice-with-interrupts-masked.md | 3 +- kernel/src/mm/alloc.rs | 2 +- kernel/src/mm/pmm.rs | 10 +-- tests/checks.rs | 2 +- tests/toyos-rust-tests/src/bin/heap_growth.rs | 67 ------------------- tests/toyos.rs | 48 +++---------- 6 files changed, 15 insertions(+), 117 deletions(-) delete mode 100644 tests/toyos-rust-tests/src/bin/heap_growth.rs diff --git a/issues/kernel/a-tls-block-is-zeroed-twice-with-interrupts-masked.md b/issues/kernel/a-tls-block-is-zeroed-twice-with-interrupts-masked.md index 0bbae91ec65..94f55dcdfe0 100644 --- a/issues/kernel/a-tls-block-is-zeroed-twice-with-interrupts-masked.md +++ b/issues/kernel/a-tls-block-is-zeroed-twice-with-interrupts-masked.md @@ -14,8 +14,7 @@ Every `SYS_THREAD_SPAWN` (`process::spawn_thread`) and every `SYS_SPAWN` exit (`issues/kernel/syscall-preemption-is-incidental.md`), so each spawn pays two writes of the block, at least 2 MiB each, in one interrupts-off window. -By reading, not measured. The T14's `heap_growth_windows` row reads the longest -window across 256 thread spawns, of which these writes are part. +By reading, not measured. Owner: `issues/kernel/toyos-beats-linuxs-latency-on-the-t14.md`. diff --git a/kernel/src/mm/alloc.rs b/kernel/src/mm/alloc.rs index 0e58fcc7ce6..0bb60a5bb98 100644 --- a/kernel/src/mm/alloc.rs +++ b/kernel/src/mm/alloc.rs @@ -557,7 +557,7 @@ unsafe impl GlobalAlloc for KernelAllocator { }; let mut base = malloc(None); if base.is_null() { - if let Some(frame) = pmm::alloc_heap_page() { + if let Some(frame) = pmm::claim(pmm::Category::KernelHeap) { base = malloc(Some(frame)); } } diff --git a/kernel/src/mm/pmm.rs b/kernel/src/mm/pmm.rs index 390dd3bbbb6..9be7bd4919d 100644 --- a/kernel/src/mm/pmm.rs +++ b/kernel/src/mm/pmm.rs @@ -262,7 +262,7 @@ pub(super) fn init(entries: &[MemoryMapEntry], reserved: &[Region]) { ); } -/// Allocate one 2MB physical page. Does not heap-allocate (safe to call from the allocator). +/// Allocate one 2MB physical page. pub fn alloc_page(cat: Category) -> Option { let page = claim(cat)?; // SAFETY: `claim` just took the frame off the bitmap, so it is unaliased, and the direct map covers every address the bitmap can name. @@ -272,12 +272,8 @@ pub fn alloc_page(cat: Category) -> Option { Some(page) } -/// One 2MB physical page for the kernel heap, holding what its last owner left in it: the heap answers uninitialized memory, and `alloc_zeroed` writes its own zeros. -pub(super) fn alloc_heap_page() -> Option { - claim(Category::KernelHeap) -} - -fn claim(cat: Category) -> Option { +/// One 2MB physical page holding what its last owner left in it. Only the kernel heap takes one as it is: the heap answers uninitialized memory, and `alloc_zeroed` writes its own zeros. Does not heap-allocate: the heap calls it when it is full. +pub(super) fn claim(cat: Category) -> Option { let mut bm = BITMAP.lock(); if bm.free_count == 0 { return None; } let start = bm.next_hint; diff --git a/tests/checks.rs b/tests/checks.rs index 98352bb333e..88cd2bdc418 100644 --- a/tests/checks.rs +++ b/tests/checks.rs @@ -427,7 +427,7 @@ mod checks { let opening = [report((100, 100), read_back), exit("pwd")].concat(); let own = [report((3_000_000, 1_100_000), (700_000, 600_000)), exit(WINDOWS_LOAD)].concat(); let good = [hold.as_str(), &first, &opening, &own].concat(); - let exited = load_exited(WINDOWS_LOAD); + let exited = windows_load_exited(); mask_windows(&good, 2).map_err(|e| format!("the good capture was refused: {e}"))?; let unpaired = |what: &str, capture: &str, says: &str| match mask_windows(capture, 2) { diff --git a/tests/toyos-rust-tests/src/bin/heap_growth.rs b/tests/toyos-rust-tests/src/bin/heap_growth.rs deleted file mode 100644 index c2676281836..00000000000 --- a/tests/toyos-rust-tests/src/bin/heap_growth.rs +++ /dev/null @@ -1,67 +0,0 @@ -//! **A burst of thread creations that grows the kernel heap, in a windows -//! report of its own.** -//! -//! A thread costs the kernel a 128 KiB stack off its heap, so [`THREADS`] -//! alive at once grow it by whole 2 MiB frames, each growth inside the -//! `SYS_THREAD_SPAWN` that needed it. A `mask-windows` kernel reports every -//! CPU's longest windows at each process exit, so two exits bracket the burst: -//! `echo`'s before it, which takes this process's spawn and its child's, and -//! the child's after it, the first exit recorded under this binary's name, -//! whose report `heap_growth_windows` reads. No thread ends inside the -//! bracket: an ending thread's TLS unmap is a shootdown, a window on every CPU. - -use std::io::{Read, Write}; -use std::process::{Command, Stdio}; -use std::sync::atomic::{AtomicBool, Ordering}; -use std::thread; - -const SELF_PATH: &str = "/system/bin/test_rs_heap_growth"; - -/// 32 MiB of kernel stacks. -const THREADS: usize = 256; - -static RELEASED: AtomicBool = AtomicBool::new(false); - -fn main() { - if std::env::args().nth(1).as_deref() == Some("bracket") { - let mut stdout = std::io::stdout(); - stdout.write_all(b"!").and_then(|()| stdout.flush()).expect("the bracket says it started"); - std::io::stdin().read_to_end(&mut Vec::new()).expect("the bracket waits for its stdin to close"); - return; - } - - let mut bracket = Command::new(SELF_PATH) - .arg("bracket") - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .spawn() - .expect("spawn the bracket"); - // Its start, too, is in `echo`'s report rather than the burst's. - let mut started = [0u8]; - bracket.stdout.take().expect("the bracket's stdout").read_exact(&mut started).expect("the bracket started"); - let opened = Command::new("/system/bin/echo").status().expect("run echo"); - assert!(opened.success(), "echo: {opened}"); - - let parked: Vec<_> = (0..THREADS) - .map(|_| { - thread::spawn(|| { - while !RELEASED.load(Ordering::Acquire) { - thread::park(); - } - }) - }) - .collect(); - - drop(bracket.stdin.take()); - let closed = bracket.wait().expect("wait for the bracket"); - assert!(closed.success(), "the bracket: {closed}"); - - RELEASED.store(true, Ordering::Release); - for thread in &parked { - thread.thread().unpark(); - } - for thread in parked { - thread.join().expect("a parked thread"); - } - println!("heap_growth: {THREADS} threads alive at once between two process exits"); -} diff --git a/tests/toyos.rs b/tests/toyos.rs index 86834d04a4f..486edcdcd90 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -80,10 +80,6 @@ const RUST_SKIP: &[&str] = &[ // its 256 threads' stacks would crowd every member after it in one boot. // The `mask_windows` metal row runs it. "ring_park_herd", - // Its product is the windows across its burst, and its 256 threads crowd - // every member after it as the herd's do. The `heap_growth_windows` metal - // row runs it. - "heap_growth", // It asserts nothing: it is half a second of an idle machine, which the // same row reads the windows across. "idle_span", @@ -263,16 +259,7 @@ const METAL: &[(&str, metal::Metal)] = &[ // The windows on the machine that owes them: every CPU reported beside // its census, and a held window read back. "mask_windows", - metal::Metal { arms: WINDOWSCASE, judge: |b| windows_on_metal(b[0], WINDOWS_LOAD, "herd") }, - ), - ( - // The same reading across a burst that grows the kernel heap by whole - // frames, each growth inside the thread spawn that needed it. - "heap_growth_windows", - metal::Metal { - arms: HEAPGROWCASE, - judge: |b| windows_on_metal(b[0], HEAP_GROWTH_LOAD, "burst"), - }, + metal::Metal { arms: WINDOWSCASE, judge: |b| windows_on_metal(b[0]) }, ), ( "mkdir_cap", @@ -745,25 +732,9 @@ const WINDOWSCASE: &[metal::Arm] = &[metal::Arm { /// The load `mask_windows` reads the windows under. const WINDOWS_LOAD: &str = "test_rs_ring_park_herd"; -/// [`WINDOWSCASE`] with [`HEAP_GROWTH_LOAD`] for its load, on a boot of its -/// own: the herd's teardown would be in the burst's report, and the heap the -/// herd grew would leave the burst nothing to grow. -const HEAPGROWCASE: &[metal::Arm] = &[metal::Arm { - features: toyos_build::build::MASK_WINDOWS_KERNEL, - ..metal::once( - "heapgrowcase", - "tests/testcases", - &[], - &["test_rs_idle_span", "pwd", HEAP_GROWTH_LOAD], - ) -}]; - -/// The load `heap_growth_windows` reads the windows under. -const HEAP_GROWTH_LOAD: &str = "test_rs_heap_growth"; - -/// The head of the kernel's record of a process of `load`'s exit. -fn load_exited(load: &str) -> String { - format!("{}{} pid=", toyos_build::bootlog::EXIT, toyos_build::bootlog::recorded_name(load)) +/// The head of the kernel's record of [`WINDOWS_LOAD`]'s exit. +fn windows_load_exited() -> String { + format!("{}{} pid=", toyos_build::bootlog::EXIT, toyos_build::bootlog::recorded_name(WINDOWS_LOAD)) } /// A `logkeeper` that leaves soundserver's ring unread until the job says the tone played. @@ -2565,18 +2536,17 @@ fn irq_census(capture: &str) -> Result<(), String> { Ok(()) } -/// The T14's windows under `load`, its reading printed as `what`'s: -/// [`mask_windows`]' verdict with every CPU reporting, and +/// The T14's windows: [`mask_windows`]' verdict with every CPU reporting, and /// `common::irqcensus::windows_under`'s. Its durations are printed and none is /// recorded: one boot's longest window is no baseline for the next. -fn windows_on_metal(boot: &metal::Readback, load: &str, what: &str) -> Result<(), String> { - boot.job_passed(load)?; +fn windows_on_metal(boot: &metal::Readback) -> Result<(), String> { + boot.job_passed(WINDOWS_LOAD)?; let kernel = boot.kernel(); let cpus = boot.cpus()?; mask_windows(kernel.text(), cpus)?; - let read = common::irqcensus::windows_under(kernel.text(), cpus, &load_exited(load))?; + let read = common::irqcensus::windows_under(kernel.text(), cpus, &windows_load_exited())?; eprintln!( - " [windows] held irqs_off_ns={} preempt_off_ns={}; {what} irqs_off_ns={} preempt_off_ns={}", + " [windows] held irqs_off_ns={} preempt_off_ns={}; herd irqs_off_ns={} preempt_off_ns={}", read.held.0, read.held.1, read.load.0, read.load.1 ); Ok(()) From 242cdc92dea3a951501cfddf3acfeb68a6f2d4b9 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 00:34:26 +0200 Subject: [PATCH 5/5] issues: a fresh kernel heap chunk holds its frame's last owner's bytes Review round 2 on #699: the heap now takes un-zeroed frames from pmm::claim, and that compromise is recorded with an owner and an exit. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...hunk-holds-its-frames-last-owners-bytes.md | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 issues/kernel/a-fresh-kernel-heap-chunk-holds-its-frames-last-owners-bytes.md diff --git a/issues/kernel/a-fresh-kernel-heap-chunk-holds-its-frames-last-owners-bytes.md b/issues/kernel/a-fresh-kernel-heap-chunk-holds-its-frames-last-owners-bytes.md new file mode 100644 index 00000000000..2143a6839ad --- /dev/null +++ b/issues/kernel/a-fresh-kernel-heap-chunk-holds-its-frames-last-owners-bytes.md @@ -0,0 +1,24 @@ +--- +status: open +kind: defect +opened: 2026-10-04 +--- + +# A fresh kernel heap chunk holds its frame's last owner's bytes + +When the kernel heap grows, `KernelAllocator::alloc` (`kernel/src/mm/alloc.rs`) +takes its frame from `pmm::claim` (`kernel/src/mm/pmm.rs`), which does not zero +it, so that no 2 MiB write runs while a CPU waits on the heap lock. A chunk +carved from that frame holds whatever its last owner, a process's page among +them, left there until the kernel writes it. `GlobalAlloc::alloc` promises no +contents either way, but a kernel bug that copies out heap memory it never +wrote now leaks another process's data where it used to leak zeros. + +By reading, not measured: no reachable copy-out of unwritten heap memory is +known. + +Owner: `issues/kernel/the-kernel-is-at-least-as-secure-as-linux-on-every-machine-toyos-supports.md`. + +**Exit**: no byte the kernel heap hands out holds data a previous owner of its +frame wrote, and the zeroing that ensures it runs under no lock another CPU +waits on.