fix: kernel hardening pass 2 — page tracking, queue overflow, region tracking
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.
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -234,12 +234,8 @@ impl<P: Platform> Kernel<P> {
|
||||
|
||||
// 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);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -99,7 +99,12 @@ impl<P: Platform> Kernel<P> {
|
||||
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<P: Platform> Kernel<P> {
|
||||
)),
|
||||
};
|
||||
|
||||
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<P: Platform> Kernel<P> {
|
||||
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,
|
||||
|
||||
@@ -13,9 +13,8 @@ impl<P: Platform> Kernel<P> {
|
||||
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<P: Platform> Kernel<P> {
|
||||
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) {
|
||||
|
||||
@@ -12,21 +12,21 @@ impl<P: Platform> Kernel<P> {
|
||||
) -> 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<P: Platform> Kernel<P> {
|
||||
}
|
||||
};
|
||||
|
||||
// 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()))
|
||||
}
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user