fix: critical privilege escalation + resource leak hardening

CRITICAL — MapPage privilege escalation:
  handle_map_page allowed mapping kernel memory (VA >= 0x80000000)
  into userspace. Any process with a Process capability could read/
  write kernel data structures. Fixed: reject src_va >= USER_VA_LIMIT.

HIGH — resource leaks:
  handle_kill: leaked pages (used cleanup_threads directly instead
    of destroy_process). Now uses destroy_process for full cleanup.
  handle_create_process: orphaned process on cap grant failure.
    Now destroys process on rollback.
  handle_create_endpoint: orphaned endpoint on cap grant failure.
    Now destroys endpoint on rollback.
  Thread ID array: hardcoded [None; 4] replaced with
    [None; MAX_THREADS_PER_PROCESS] slice. cleanup_threads now
    takes &[Option<ThreadId>] slice.

MEDIUM — quality fixes:
  handle_request_pages: replaced unreachable!() with proper error
    return + page pool rollback.
This commit is contained in:
2026-10-18 22:39:16 +02:00
parent d009818caf
commit 06863c84fa
4 changed files with 41 additions and 39 deletions

View File

@@ -218,12 +218,13 @@ impl<P: Platform> Kernel<P> {
} }
pub(crate) fn destroy_process(&mut self, pid: ProcessId) { pub(crate) fn destroy_process(&mut self, pid: ProcessId) {
// Collect thread IDs before cleanup use domain::process::MAX_THREADS_PER_PROCESS;
let thread_ids: [Option<ThreadId>; 4] = match self.processes.get(pid) {
let thread_ids: [Option<ThreadId>; MAX_THREADS_PER_PROCESS] = match self.processes.get(pid) {
Ok(p) => { Ok(p) => {
let mut ids = [None; 4]; let mut ids = [None; MAX_THREADS_PER_PROCESS];
for (i, tid) in p.thread_ids().enumerate() { for (i, tid) in p.thread_ids().enumerate() {
if i < 4 { ids[i] = Some(tid); } if i < MAX_THREADS_PER_PROCESS { ids[i] = Some(tid); }
} }
ids ids
} }
@@ -250,7 +251,7 @@ impl<P: Platform> Kernel<P> {
self.process_memory[pid.raw() as usize] = None; self.process_memory[pid.raw() as usize] = None;
} }
pub(crate) fn cleanup_threads(&mut self, thread_ids: &[Option<ThreadId>; 4]) { pub(crate) fn cleanup_threads(&mut self, thread_ids: &[Option<ThreadId>]) {
use domain::scheduler::Scheduler; use domain::scheduler::Scheduler;
for slot in thread_ids { for slot in thread_ids {
if let Some(tid) = slot { if let Some(tid) = slot {

View File

@@ -25,7 +25,10 @@ impl<P: Platform> Kernel<P> {
all_perms, all_perms,
) { ) {
Ok(slot) => SyscallOutcome::Complete(SyscallResult::Slot(slot)), Ok(slot) => SyscallOutcome::Complete(SyscallResult::Slot(slot)),
Err(e) => SyscallOutcome::Complete(SyscallResult::Error(e)), Err(e) => {
let _ = self.endpoints.destroy(ep_id);
SyscallOutcome::Complete(SyscallResult::Error(e))
}
} }
} }

View File

@@ -1,3 +1,4 @@
use domain::process::MAX_THREADS_PER_PROCESS;
use platform::Platform; use platform::Platform;
use super::{SyscallOutcome, SyscallResult}; use super::{SyscallOutcome, SyscallResult};
@@ -14,7 +15,7 @@ impl<P: Platform> Kernel<P> {
use domain::thread::SavedContext; use domain::thread::SavedContext;
let priority = types::bounded::Priority::DEFAULT; let priority = types::bounded::Priority::DEFAULT;
let time_slice = types::time::TimeSlice::DEFAULT; let time_slice = types::time::TimeSlice::new(100);
let context = SavedContext::with_entry_and_stack(entry, stack); let context = SavedContext::with_entry_and_stack(entry, stack);
let tid = match self.threads.spawn_with_context(pid, priority, time_slice, None, context) { let tid = match self.threads.spawn_with_context(pid, priority, time_slice, None, context) {
@@ -38,8 +39,6 @@ impl<P: Platform> Kernel<P> {
create_cap: types::bounded::CapSlot, create_cap: types::bounded::CapSlot,
_image: types::address::DataRef, _image: types::address::DataRef,
) -> SyscallOutcome { ) -> SyscallOutcome {
use domain::scheduler::Scheduler;
if let Err(e) = self.check_cap(caller_pid, create_cap, types::permission::Permission::Execute) { if let Err(e) = self.check_cap(caller_pid, create_cap, types::permission::Permission::Execute) {
return SyscallOutcome::Complete(SyscallResult::Error(e)); return SyscallOutcome::Complete(SyscallResult::Error(e));
} }
@@ -55,18 +54,21 @@ impl<P: Platform> Kernel<P> {
self.process_memory[new_pid.raw() as usize] = self.process_memory[new_pid.raw() as usize] =
Some(domain::memory::process_memory::ProcessMemory::new(new_pid, 1024)); Some(domain::memory::process_memory::ProcessMemory::new(new_pid, 1024));
let slot = match self.grant_cap( match self.grant_cap(
caller_pid, caller_pid,
domain::resource::Resource::Process(new_pid), domain::resource::Resource::Process(new_pid),
types::permission::PermissionSet::empty() types::permission::PermissionSet::empty()
.with(types::permission::Permission::Execute) .with(types::permission::Permission::Execute)
.with(types::permission::Permission::Read), .with(types::permission::Permission::Read),
) { ) {
Ok(s) => s, Ok(slot) => SyscallOutcome::Complete(SyscallResult::Slot(slot)),
Err(e) => return SyscallOutcome::Complete(SyscallResult::Error(e)), Err(e) => {
}; // Rollback: destroy orphaned process
let _ = self.processes.destroy(new_pid);
SyscallOutcome::Complete(SyscallResult::Slot(slot)) self.process_memory[new_pid.raw() as usize] = None;
SyscallOutcome::Complete(SyscallResult::Error(e))
}
}
} }
pub(crate) fn handle_spawn_thread_in( pub(crate) fn handle_spawn_thread_in(
@@ -97,7 +99,7 @@ impl<P: Platform> Kernel<P> {
}; };
let priority = types::bounded::Priority::DEFAULT; let priority = types::bounded::Priority::DEFAULT;
let time_slice = types::time::TimeSlice::DEFAULT; let time_slice = types::time::TimeSlice::new(100);
let context = SavedContext::with_entry_and_stack(entry, stack); let context = SavedContext::with_entry_and_stack(entry, stack);
let tid = match self.threads.spawn_with_context(target_pid, priority, time_slice, None, context) { let tid = match self.threads.spawn_with_context(target_pid, priority, time_slice, None, context) {
@@ -133,23 +135,7 @@ impl<P: Platform> Kernel<P> {
Err(e) => return SyscallOutcome::Complete(SyscallResult::Error(e)), Err(e) => return SyscallOutcome::Complete(SyscallResult::Error(e)),
}; };
let thread_ids: [Option<types::id::ThreadId>; 4] = { self.destroy_process(target_pid);
match self.processes.get(target_pid) {
Ok(p) => {
let mut ids = [None; 4];
for (i, tid) in p.thread_ids().enumerate() {
if i < 4 { ids[i] = Some(tid); }
}
ids
}
Err(e) => return SyscallOutcome::Complete(SyscallResult::Error(e)),
}
};
self.cleanup_threads(&thread_ids);
let _ = self.processes.destroy(target_pid);
self.process_memory[target_pid.raw() as usize] = None;
SyscallOutcome::Complete(SyscallResult::Ok) SyscallOutcome::Complete(SyscallResult::Ok)
} }
} }

View File

@@ -4,6 +4,8 @@ use types::address::{PhysicalAddress, VirtualAddress};
use super::{SyscallOutcome, SyscallResult}; use super::{SyscallOutcome, SyscallResult};
use crate::Kernel; use crate::Kernel;
const USER_VA_LIMIT: usize = 0x80000000;
impl<P: Platform> Kernel<P> { impl<P: Platform> Kernel<P> {
pub(crate) fn handle_request_pages( pub(crate) fn handle_request_pages(
&mut self, &mut self,
@@ -12,7 +14,6 @@ impl<P: Platform> Kernel<P> {
) -> SyscallOutcome { ) -> SyscallOutcome {
let pages = count.value() as usize; let pages = count.value() as usize;
// Check limits first (separate borrow)
match &mut self.process_memory[pid.raw() as usize] { match &mut self.process_memory[pid.raw() as usize] {
Some(pm) => { Some(pm) => {
if pm.grant(pages).is_err() { if pm.grant(pages).is_err() {
@@ -26,7 +27,6 @@ impl<P: Platform> Kernel<P> {
)), )),
} }
// Allocate physical pages from pool
let phys_addr = match self.page_pool.allocate(pages) { let phys_addr = match self.page_pool.allocate(pages) {
Some(a) => a, Some(a) => a,
None => { None => {
@@ -39,24 +39,26 @@ 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] { let user_va = match &mut self.process_memory[pid.raw() as usize] {
Some(pm) => { Some(pm) => {
let va = pm.alloc_user_va(pages); let va = pm.alloc_user_va(pages);
pm.record_allocation(phys_addr, pages); pm.record_allocation(phys_addr, pages);
va va
} }
None => unreachable!(), None => {
self.page_pool.free(phys_addr, pages);
return SyscallOutcome::Complete(SyscallResult::Error(
domain::error::KernelError::InvalidProcess(pid),
));
}
}; };
// Map pages
for i in 0..pages { for i in 0..pages {
let page_va = VirtualAddress::from_raw(user_va.raw() + i * 4096); let page_va = VirtualAddress::from_raw(user_va.raw() + i * 4096);
let page_pa = PhysicalAddress::from_raw(phys_addr.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); self.platform.map_user_page(pid, page_va, page_pa, true, false);
} }
// Track region for ReleasePages
let region = domain::memory::MemoryRegion::new( let region = domain::memory::MemoryRegion::new(
types::id::RegionId::from_raw(0), types::id::RegionId::from_raw(0),
pid, pid,
@@ -77,6 +79,16 @@ impl<P: Platform> Kernel<P> {
dst_va: VirtualAddress, dst_va: VirtualAddress,
executable: bool, executable: bool,
) -> SyscallOutcome { ) -> SyscallOutcome {
// Prevent mapping kernel memory into userspace
if src_va.raw() >= USER_VA_LIMIT {
return SyscallOutcome::Complete(SyscallResult::Error(
domain::error::KernelError::PermissionDenied {
slot: process_cap,
needed: types::permission::Permission::Read,
},
));
}
let target_pid = match self.check_cap( let target_pid = match self.check_cap(
caller_pid, caller_pid,
process_cap, process_cap,