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. 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..94f55dcdfe0 --- /dev/null +++ b/issues/kernel/a-tls-block-is-zeroed-twice-with-interrupts-masked.md @@ -0,0 +1,21 @@ +--- +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. + +Owner: `issues/kernel/toyos-beats-linuxs-latency-on-the-t14.md`. + +**Exit**: `build_combined` zeroes no byte the PMM already zeroed. diff --git a/kernel/src/mm/alloc.rs b/kernel/src/mm/alloc.rs index 8f2f610cf44..0bb60a5bb98 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::claim(pmm::Category::KernelHeap) { + 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..9be7bd4919d 100644 --- a/kernel/src/mm/pmm.rs +++ b/kernel/src/mm/pmm.rs @@ -262,8 +262,18 @@ 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. + unsafe { + core::ptr::write_bytes(page.direct_map().as_mut_ptr::(), 0, PAGE_2M as usize); + } + Some(page) +} + +/// 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; @@ -275,12 +285,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 }); }