From d009818caf5385db3fd9fecf78b7fd1cca254047 Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Sun, 18 Oct 2026 22:31:16 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20kernel=20hardening=20pass=202=20?= =?UTF-8?q?=E2=80=94=20page=20tracking,=20queue=20overflow,=20region=20tra?= =?UTF-8?q?cking?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Page lifecycle: - ProcessMemory now tracks (PhysicalAddress, count) pairs for each allocation (up to 16). destroy_process iterates and frees all pages back to the pool. No more permanent page leaks on exit/fault. - RequestPages re-adds MemoryRegion entries so ReleasePages works. Release test un-ignored, passes. - RequestPages: VA allocation moved after pool allocation succeeds, preventing VA leak on OOM. IPC hardening: - WaitQueue overflow (>16 waiters): returns WouldBlock error instead of silently dropping the thread. Thread is not blocked if enqueue fails. - debug_assert on pending_messages[sender].take() — invariant that a sender in wait queue must have a pending message. Scheduler: - remove() now clears current thread if the removed thread was current on its core. - add() asserts push succeeded; bitmap bit only set if thread was actually added to RunList. Cleanup: - Priority::DEFAULT replaces Priority::new(16).unwrap() in spawn_thread and spawn_thread_in. 36 kernel tests pass, 0 ignored. --- crates/domain/src/memory/process_memory.rs | 18 +++++++- crates/domain/src/scheduler/multi_core.rs | 6 ++- crates/domain/src/scheduler/priority_queue.rs | 7 +++- crates/kernel/src/lib.rs | 8 +--- crates/kernel/src/syscall/ipc.rs | 21 ++++++---- crates/kernel/src/syscall/lifecycle.rs | 10 ++--- crates/kernel/src/syscall/memory.rs | 41 ++++++++++++++----- crates/kernel/src/tests/memory.rs | 1 - 8 files changed, 78 insertions(+), 34 deletions(-) diff --git a/crates/domain/src/memory/process_memory.rs b/crates/domain/src/memory/process_memory.rs index 7ecab8f..454878e 100644 --- a/crates/domain/src/memory/process_memory.rs +++ b/crates/domain/src/memory/process_memory.rs @@ -1,16 +1,19 @@ -use types::address::VirtualAddress; +use types::address::{PhysicalAddress, VirtualAddress}; use types::id::ProcessId; use crate::error::KernelError; const USER_HEAP_START: usize = 0x40000; const PAGE_SIZE: usize = 4096; +const MAX_ALLOCATIONS: usize = 16; pub struct ProcessMemory { process: ProcessId, page_count: usize, limit: usize, next_heap_va: usize, + allocations: [(PhysicalAddress, usize); MAX_ALLOCATIONS], + alloc_count: usize, } impl ProcessMemory { @@ -20,6 +23,8 @@ impl ProcessMemory { page_count: 0, limit, next_heap_va: USER_HEAP_START, + allocations: [(PhysicalAddress::from_raw(0), 0); MAX_ALLOCATIONS], + alloc_count: 0, } } @@ -29,6 +34,17 @@ impl ProcessMemory { VirtualAddress::from_raw(va) } + pub fn record_allocation(&mut self, pa: PhysicalAddress, count: usize) { + if self.alloc_count < MAX_ALLOCATIONS { + self.allocations[self.alloc_count] = (pa, count); + self.alloc_count += 1; + } + } + + pub fn allocations(&self) -> &[(PhysicalAddress, usize)] { + &self.allocations[..self.alloc_count] + } + pub fn process(&self) -> ProcessId { self.process } diff --git a/crates/domain/src/scheduler/multi_core.rs b/crates/domain/src/scheduler/multi_core.rs index 53e7a54..80927aa 100644 --- a/crates/domain/src/scheduler/multi_core.rs +++ b/crates/domain/src/scheduler/multi_core.rs @@ -68,7 +68,11 @@ impl Scheduler for MultiCoreScheduler { fn remove(&mut self, thread: ThreadId) { let idx = thread.raw() as usize; if let Some(placement) = self.placements[idx].take() { - self.cores[placement.core.raw() as usize].remove(thread, placement.priority); + let core_idx = placement.core.raw() as usize; + self.cores[core_idx].remove(thread, placement.priority); + if self.cores[core_idx].current() == Some(thread) { + self.cores[core_idx].clear_current(); + } } } diff --git a/crates/domain/src/scheduler/priority_queue.rs b/crates/domain/src/scheduler/priority_queue.rs index f9c345a..9fc84cb 100644 --- a/crates/domain/src/scheduler/priority_queue.rs +++ b/crates/domain/src/scheduler/priority_queue.rs @@ -77,8 +77,11 @@ impl PriorityQueue { pub fn add(&mut self, thread: ThreadId, priority: Priority) { let idx = priority.value() as usize; - self.queues[idx].push(thread); - self.bitmap |= 1 << idx; + let added = self.queues[idx].push(thread); + debug_assert!(added, "scheduler run list full for priority {}", idx); + if added { + self.bitmap |= 1 << idx; + } } pub fn remove(&mut self, thread: ThreadId, priority: Priority) { diff --git a/crates/kernel/src/lib.rs b/crates/kernel/src/lib.rs index 0960a2a..f0fc6a3 100644 --- a/crates/kernel/src/lib.rs +++ b/crates/kernel/src/lib.rs @@ -234,12 +234,8 @@ impl Kernel

{ // Free allocated pages back to pool if let Some(pm) = &self.process_memory[pid.raw() as usize] { - let page_count = pm.page_count(); - if page_count > 0 { - // Pages were allocated from the pool but we don't track individual - // addresses. The page pool uses contiguous allocation, so we can't - // free without knowing the base address. Mark as returned for accounting. - // Full page tracking is a future improvement. + for &(pa, count) in pm.allocations() { + self.page_pool.free(pa, count); } } diff --git a/crates/kernel/src/syscall/ipc.rs b/crates/kernel/src/syscall/ipc.rs index fc171be..9da25a2 100644 --- a/crates/kernel/src/syscall/ipc.rs +++ b/crates/kernel/src/syscall/ipc.rs @@ -99,7 +99,12 @@ impl Kernel

{ domain::error::KernelError::InvalidEndpoint(ep_id), )), }; - let _ = endpoint.enqueue_sender(caller_tid); + if endpoint.enqueue_sender(caller_tid).is_none() { + self.pending_messages[caller_tid.raw() as usize] = None; + return SyscallOutcome::Complete(SyscallResult::Error( + domain::error::KernelError::WouldBlock, + )); + } let caller = match self.threads.get_mut(caller_tid) { Ok(t) => t, @@ -149,11 +154,9 @@ impl Kernel

{ )), }; - let message = self.pending_messages[sender_tid.raw() as usize] - .take() - // A sender in the wait queue must have a pending message. - // If not, the kernel state is inconsistent. - .unwrap_or_else(|| domain::message::Message::empty()); + let message = self.pending_messages[sender_tid.raw() as usize].take(); + debug_assert!(message.is_some(), "sender in wait queue had no pending message"); + let message = message.unwrap_or_else(|| domain::message::Message::empty()); self.pending_results[sender_tid.raw() as usize] = Some(SyscallResult::Ok); @@ -174,7 +177,11 @@ impl Kernel

{ domain::error::KernelError::InvalidEndpoint(ep_id), )), }; - let _ = endpoint.enqueue_receiver(caller_tid); + if endpoint.enqueue_receiver(caller_tid).is_none() { + return SyscallOutcome::Complete(SyscallResult::Error( + domain::error::KernelError::WouldBlock, + )); + } let caller = match self.threads.get_mut(caller_tid) { Ok(t) => t, diff --git a/crates/kernel/src/syscall/lifecycle.rs b/crates/kernel/src/syscall/lifecycle.rs index 05d7484..ec1345f 100644 --- a/crates/kernel/src/syscall/lifecycle.rs +++ b/crates/kernel/src/syscall/lifecycle.rs @@ -13,9 +13,8 @@ impl Kernel

{ use domain::scheduler::Scheduler; use domain::thread::SavedContext; - let priority = types::bounded::Priority::new(16) - .unwrap_or_else(|| unreachable!()); - let time_slice = types::time::TimeSlice::new(100); + let priority = types::bounded::Priority::DEFAULT; + let time_slice = types::time::TimeSlice::DEFAULT; let context = SavedContext::with_entry_and_stack(entry, stack); let tid = match self.threads.spawn_with_context(pid, priority, time_slice, None, context) { @@ -97,9 +96,8 @@ impl Kernel

{ Err(e) => return SyscallOutcome::Complete(SyscallResult::Error(e)), }; - let priority = types::bounded::Priority::new(16) - .unwrap_or_else(|| unreachable!()); - let time_slice = types::time::TimeSlice::new(100); + let priority = types::bounded::Priority::DEFAULT; + let time_slice = types::time::TimeSlice::DEFAULT; let context = SavedContext::with_entry_and_stack(entry, stack); let tid = match self.threads.spawn_with_context(target_pid, priority, time_slice, None, context) { diff --git a/crates/kernel/src/syscall/memory.rs b/crates/kernel/src/syscall/memory.rs index c2a89bc..0c6f677 100644 --- a/crates/kernel/src/syscall/memory.rs +++ b/crates/kernel/src/syscall/memory.rs @@ -12,21 +12,21 @@ impl Kernel

{ ) -> SyscallOutcome { let pages = count.value() as usize; - let pm = match &mut self.process_memory[pid.raw() as usize] { - Some(pm) => pm, + // Check limits first (separate borrow) + match &mut self.process_memory[pid.raw() as usize] { + Some(pm) => { + if pm.grant(pages).is_err() { + return SyscallOutcome::Complete(SyscallResult::Error( + domain::error::KernelError::OutOfMemory, + )); + } + } None => return SyscallOutcome::Complete(SyscallResult::Error( domain::error::KernelError::InvalidProcess(pid), )), - }; - - if pm.grant(pages).is_err() { - return SyscallOutcome::Complete(SyscallResult::Error( - domain::error::KernelError::OutOfMemory, - )); } - let user_va = pm.alloc_user_va(pages); - + // Allocate physical pages from pool let phys_addr = match self.page_pool.allocate(pages) { Some(a) => a, None => { @@ -39,12 +39,33 @@ impl Kernel

{ } }; + // Allocate user VA and record allocation (only after pool succeeds) + let user_va = match &mut self.process_memory[pid.raw() as usize] { + Some(pm) => { + let va = pm.alloc_user_va(pages); + pm.record_allocation(phys_addr, pages); + va + } + None => unreachable!(), + }; + + // Map pages for i in 0..pages { let page_va = VirtualAddress::from_raw(user_va.raw() + i * 4096); let page_pa = PhysicalAddress::from_raw(phys_addr.raw() + i * 4096); self.platform.map_user_page(pid, page_va, page_pa, true, false); } + // Track region for ReleasePages + let region = domain::memory::MemoryRegion::new( + types::id::RegionId::from_raw(0), + pid, + phys_addr, + count, + types::generation::Generation::initial(), + ); + let _ = self.regions.insert(region); + SyscallOutcome::Complete(SyscallResult::Value(user_va.raw())) } diff --git a/crates/kernel/src/tests/memory.rs b/crates/kernel/src/tests/memory.rs index db4dbd5..e96b041 100644 --- a/crates/kernel/src/tests/memory.rs +++ b/crates/kernel/src/tests/memory.rs @@ -27,7 +27,6 @@ fn request_pages_reduces_pool() { } #[test] -#[ignore] // RequestPages no longer creates regions — ReleasePages needs redesign fn release_pages_frees_and_restores_pool() { let (mut k, _) = kernel_with_process(); let before = k.free_page_count();