diff --git a/crates/adapter-riscv/linker.ld b/crates/adapter-riscv/linker.ld index d0da499..754e5b9 100644 --- a/crates/adapter-riscv/linker.ld +++ b/crates/adapter-riscv/linker.ld @@ -1,8 +1,11 @@ OUTPUT_ARCH(riscv) ENTRY(_start) +/* Every boundary below is page aligned: the kernel maps each range with + its own permissions (W^X), see mmu::init_kernel_page_table. */ SECTIONS { . = 0x80200000; + _text_start = .; .text : { *(.text.entry) @@ -10,13 +13,22 @@ SECTIONS { } . = ALIGN(4096); + _text_end = .; + .rodata : { *(.rodata .rodata.*) + *(.srodata .srodata.*) + } + .eh_frame : { + *(.eh_frame) } . = ALIGN(4096); + _rodata_end = .; + .data : { *(.data .data.*) + *(.sdata .sdata.*) } . = ALIGN(4096); @@ -27,7 +39,12 @@ SECTIONS { _bss_end = .; } + /* Left unmapped, so overflowing the kernel stack faults instead of + silently overwriting .bss. */ . = ALIGN(4096); + _stack_guard = .; + . += 4096; + _stack_bottom = .; . += 0x80000; _stack_top = .; diff --git a/crates/adapter-riscv/src/boot/hardware.rs b/crates/adapter-riscv/src/boot/hardware.rs index 25824a9..ec26b1a 100644 --- a/crates/adapter-riscv/src/boot/hardware.rs +++ b/crates/adapter-riscv/src/boot/hardware.rs @@ -16,6 +16,26 @@ pub fn init(uart: &Uart) -> &'static mut mmu::PageTable { let kernel_pt = mmu::init_kernel_page_table(); mmu::activate(kernel_pt); uart.write_str("k-os: MMU enabled (Sv39)\n"); + match mmu::check_wx(kernel_pt) { + Ok(code_pages) => { + uart.write_str("k-os: W^X verified, "); + uart.write_dec(code_pages); + uart.write_str(" code pages\n"); + } + Err(v) => { + let (what, va) = match v { + mmu::page_table::WxViolation::WritableCode(va) => ("writable code", va), + mmu::page_table::WxViolation::ExecutableData(va) => ("executable data", va), + mmu::page_table::WxViolation::MappedHole(va) => ("mapped hole", va), + }; + uart.write_str("k-os: W^X violation: "); + uart.write_str(what); + uart.write_str(" at "); + uart.write_hex(va); + uart.put_char(b'\n'); + crate::halt(uart, "kernel page table violates W^X"); + } + } trap::init(); crate::hw::timer::init(); diff --git a/crates/adapter-riscv/src/mmu/mod.rs b/crates/adapter-riscv/src/mmu/mod.rs index ba983c4..0791863 100644 --- a/crates/adapter-riscv/src/mmu/mod.rs +++ b/crates/adapter-riscv/src/mmu/mod.rs @@ -3,6 +3,6 @@ pub mod page_table; pub use allocator::init_allocator; pub use page_table::{ - PAGE_SIZE, PageTable, Sv39Flags, USER_VA_END, USER_VA_START, activate, active_root, + PAGE_SIZE, PageTable, Sv39Flags, USER_VA_END, USER_VA_START, activate, active_root, check_wx, create_user_root, init_kernel_page_table, map_user_page, unmap_user_page, }; diff --git a/crates/adapter-riscv/src/mmu/page_table.rs b/crates/adapter-riscv/src/mmu/page_table.rs index 6862a84..42962a5 100644 --- a/crates/adapter-riscv/src/mmu/page_table.rs +++ b/crates/adapter-riscv/src/mmu/page_table.rs @@ -22,9 +22,15 @@ pub struct Sv39Flags { } impl Sv39Flags { - pub fn kernel_rwx() -> Self { + pub fn kernel_rx() -> Self { Self { - bits: PTE_R | PTE_W | PTE_X | PTE_G, + bits: PTE_R | PTE_X | PTE_G, + } + } + + pub fn kernel_ro() -> Self { + Self { + bits: PTE_R | PTE_G, } } @@ -140,16 +146,33 @@ pub fn init_kernel_page_table() -> &'static mut PageTable { ) }; - let (va, pa) = identity(board::RAM_BASE); - map( - root, - Mapping { - va, - pa, - pages: board::RAM_SIZE / PAGE_SIZE, - flags: Sv39Flags::kernel_rwx(), - }, - ); + // The kernel image, then everything after it, each with only the + // access it needs: code is never writable, data never executable. + // RAM below the image belongs to the firmware and stays unmapped, as + // does the guard page under the kernel stack. + let layout = KernelLayout::from_linker(); + let ram_end = board::RAM_BASE + board::RAM_SIZE; + for (start, end, flags) in [ + (layout.text_start, layout.text_end, Sv39Flags::kernel_rx()), + (layout.text_end, layout.rodata_end, Sv39Flags::kernel_ro()), + ( + layout.rodata_end, + layout.stack_guard, + Sv39Flags::kernel_rw(), + ), + (layout.stack_bottom, ram_end, Sv39Flags::kernel_rw()), + ] { + let (va, pa) = identity(start); + map( + root, + Mapping { + va, + pa, + pages: (end - start) / PAGE_SIZE, + flags, + }, + ); + } let (va, pa) = identity(board::UART_BASE); map( @@ -198,6 +221,102 @@ pub fn init_kernel_page_table() -> &'static mut PageTable { root } +/// Page-aligned boundaries of the kernel image, from linker.ld. +pub struct KernelLayout { + pub text_start: usize, + pub text_end: usize, + pub rodata_end: usize, + pub stack_guard: usize, + pub stack_bottom: usize, +} + +impl KernelLayout { + pub fn from_linker() -> Self { + unsafe extern "C" { + static _text_start: u8; + static _text_end: u8; + static _rodata_end: u8; + static _stack_guard: u8; + static _stack_bottom: u8; + } + // SAFETY: only the symbols' addresses are taken, never their values. + unsafe { + Self { + text_start: &_text_start as *const u8 as usize, + text_end: &_text_end as *const u8 as usize, + rodata_end: &_rodata_end as *const u8 as usize, + stack_guard: &_stack_guard as *const u8 as usize, + stack_bottom: &_stack_bottom as *const u8 as usize, + } + } + } +} + +/// What `check_wx` found wrong with a kernel page table, if anything. +pub enum WxViolation { + /// A page that is both writable and executable. + WritableCode(usize), + /// An executable page outside the kernel's text. + ExecutableData(usize), + /// A mapped page under the firmware, or the stack guard. + MappedHole(usize), +} + +/// Walks every leaf of `root` and checks the W^X layout: nothing is both +/// writable and executable, only kernel text is executable, and the +/// firmware and the stack guard are unmapped. Returns the number of +/// executable pages. +pub fn check_wx(root: &PageTable) -> Result { + let layout = KernelLayout::from_linker(); + let text = layout.text_start..layout.text_end; + let guard = layout.stack_guard..layout.stack_bottom; + let firmware = board::RAM_BASE..layout.text_start; + let mut executable = 0; + for (va, bits) in leaves(root) { + if bits & PTE_W != 0 && bits & PTE_X != 0 { + return Err(WxViolation::WritableCode(va)); + } + if bits & PTE_X != 0 { + if !text.contains(&va) { + return Err(WxViolation::ExecutableData(va)); + } + executable += 1; + } + if guard.contains(&va) || firmware.contains(&va) { + return Err(WxViolation::MappedHole(va)); + } + } + Ok(executable) +} + +/// Every 4 KiB leaf in `root` as (virtual address, PTE flag bits). +/// The kernel maps nothing larger. +fn leaves(root: &PageTable) -> impl Iterator + '_ { + let next = |e: &PageTableEntry| table_at((e.ppn() << 12) as usize) as &PageTable; + root.entries + .iter() + .enumerate() + .filter(|(_, e)| e.is_valid()) + .flat_map(move |(i2, e2)| { + next(e2) + .entries + .iter() + .enumerate() + .filter(|(_, e)| e.is_valid()) + .flat_map(move |(i1, e1)| { + next(e1) + .entries + .iter() + .enumerate() + .filter(|(_, e)| e.is_valid()) + .map(move |(i0, e0)| { + let va = (i2 << 30) | (i1 << 21) | (i0 << 12); + (va, e0.0 & 0x3ff) + }) + }) + }) +} + pub fn activate(root: &PageTable) { let ppn = (root as *const PageTable as u64) >> 12; let satp = (8u64 << 60) | ppn; diff --git a/crates/adapter-riscv/src/trap/mod.rs b/crates/adapter-riscv/src/trap/mod.rs index 053cbf2..24df8a3 100644 --- a/crates/adapter-riscv/src/trap/mod.rs +++ b/crates/adapter-riscv/src/trap/mod.rs @@ -123,6 +123,8 @@ const CAUSE_INTERRUPT_BIT: usize = 1 << 63; const CAUSE_S_TIMER: usize = 5; const CAUSE_S_EXTERNAL: usize = 9; const CAUSE_ECALL_U: usize = 8; +/// Set in `sstatus` when the trap came from supervisor mode. +const SSTATUS_SPP: usize = 1 << 8; pub fn init() { unsafe extern "C" { @@ -347,6 +349,22 @@ fn handle_exception(frame: &mut TrapFrame, scause: usize) { } } } + _ if frame.sstatus & SSTATUS_SPP != 0 => { + // The kernel itself faulted (a W^X violation, the stack guard, + // a bad pointer). Its state can no longer be trusted, and no + // user thread is to blame. + let uart = crate::hw::uart::Uart::new(); + uart.write_str("KERNEL FAULT: scause="); + uart.write_hex(scause); + let stval: usize; + unsafe { core::arch::asm!("csrr {0}, stval", out(reg) stval) }; + uart.write_str(" stval="); + uart.write_hex(stval); + uart.write_str(" sepc="); + uart.write_hex(frame.sepc); + uart.put_char(b'\n'); + crate::halt(&uart, "fault in kernel mode"); + } _ => { let uart = crate::hw::uart::Uart::new(); uart.write_str("FAULT: scause="); diff --git a/crates/domain/src/error.rs b/crates/domain/src/error.rs index 0bf29ac..8392b9e 100644 --- a/crates/domain/src/error.rs +++ b/crates/domain/src/error.rs @@ -31,6 +31,9 @@ pub enum KernelError { /// The target virtual range already holds a mapping. AddressInUse, InvalidRegion, + /// The region is mapped executable somewhere and the request would map + /// it writable, or the other way round (W^X across processes). + WritableAndExecutable, /// The object a capability would refer to no longer exists. NoSuchObject, /// The capability names the wrong kind of object for this operation. diff --git a/crates/kernel/src/memory.rs b/crates/kernel/src/memory.rs index 110baed..5d0e8b1 100644 --- a/crates/kernel/src/memory.rs +++ b/crates/kernel/src/memory.rs @@ -385,6 +385,18 @@ impl Kernel

{ if region.mappings().any(|m| m.process == pid) { return Err(KernelError::AddressInUse); } + // W^X holds across processes too: nobody may write what someone + // else can execute. + let clashes = |other: PagePerms| { + matches!( + (perms, other), + (PagePerms::ReadWrite, PagePerms::ReadExecute) + | (PagePerms::ReadExecute, PagePerms::ReadWrite) + ) + }; + if region.mappings().any(|m| clashes(m.perms)) { + return Err(KernelError::WritableAndExecutable); + } if !Self::in_user_window(va, pages) { return Err(KernelError::InvalidAddress); } diff --git a/crates/kernel/src/syscall/mod.rs b/crates/kernel/src/syscall/mod.rs index 6950ebf..892540b 100644 --- a/crates/kernel/src/syscall/mod.rs +++ b/crates/kernel/src/syscall/mod.rs @@ -38,7 +38,9 @@ pub fn error_code(error: &KernelError) -> ErrorCode { InvalidSlot(_) | RevokedCapability(_) | NoSuchObject | InvalidEndpoint(_) => { ErrorCode::InvalidCap } - PermissionDenied { .. } | ProcessCreateDenied => ErrorCode::PermissionDenied, + PermissionDenied { .. } | ProcessCreateDenied | WritableAndExecutable => { + ErrorCode::PermissionDenied + } WrongObject => ErrorCode::WrongObject, NoFreeSlot => ErrorCode::NoFreeSlot, OutOfMemory => ErrorCode::OutOfMemory, diff --git a/crates/kernel/src/tests/revocation.rs b/crates/kernel/src/tests/revocation.rs index 7d6a2cc..b20b655 100644 --- a/crates/kernel/src/tests/revocation.rs +++ b/crates/kernel/src/tests/revocation.rs @@ -355,6 +355,43 @@ fn a_region_can_be_remapped_after_its_mapping_was_revoked() { assert!(map(&mut t, tb, again, 0x50000, PagePerms::ReadWrite)); } +#[test] +fn a_region_is_never_writable_in_one_process_and_executable_in_another() { + let mut t = Tree::new(); + let (b, ta, tb, a_to_b) = (t.b, t.ta, t.tb, t.a_to_b); + let r = region(&mut t, ta, 1); + let in_b = t.give(ta, r, a_to_b, b); + assert!(map(&mut t, ta, r, 0x30000, PagePerms::ReadWrite)); + assert!(!map(&mut t, tb, in_b, 0x50000, PagePerms::ReadExecute)); + assert!(t.k.platform_mut().mapping(b, 0x50000).is_none()); + // Read-only alongside either is fine. + assert!(map(&mut t, tb, in_b, 0x50000, PagePerms::ReadOnly)); +} + +#[test] +fn code_can_be_written_then_made_executable_once_the_writer_unmaps() { + let mut t = Tree::new(); + let (b, ta, tb, a_to_b) = (t.b, t.ta, t.tb, t.a_to_b); + let r = region(&mut t, ta, 1); + let writer = slot_of(t.run( + ta, + Syscall::Mint { + slot: r, + badge: Badge::NONE, + permissions: all(), + }, + )); + assert!(map(&mut t, ta, writer, 0x30000, PagePerms::ReadWrite)); + let in_b = t.give(ta, r, a_to_b, b); + assert!(!map(&mut t, tb, in_b, 0x50000, PagePerms::ReadExecute)); + assert!(ok(&t.run(ta, Syscall::DropCap { slot: writer }))); + assert!(map(&mut t, tb, in_b, 0x50000, PagePerms::ReadExecute)); + assert!( + !map(&mut t, ta, r, 0x30000, PagePerms::ReadWrite), + "and nobody may write it while it is executable" + ); +} + #[test] fn create_region_is_charged_to_the_caller() { let mut t = Tree::new(); diff --git a/crates/kernel/src/tests/storm.rs b/crates/kernel/src/tests/storm.rs index 77f3087..8bfe1da 100644 --- a/crates/kernel/src/tests/storm.rs +++ b/crates/kernel/src/tests/storm.rs @@ -15,6 +15,7 @@ use platform::Platform; use types::address::DataRef; use types::bounded::CapSlot; use types::id::ThreadId; +use types::mapping::PagePerms; use types::syscall_abi::*; impl Kernel { @@ -96,6 +97,10 @@ impl Kernel { } } for (_, region) in self.regions.iter() { + let has = |want| region.mappings().any(|m| m.perms == want); + if has(PagePerms::ReadWrite) && has(PagePerms::ReadExecute) { + return Err(String::from("region mapped both writable and executable")); + } for m in region.mappings() { if let Some(via) = m.via && (!self.resolves(via) || via.process != m.process) @@ -308,6 +313,19 @@ fn new_round(k: &mut Kernel, init: ProcessId, init_thread: ThreadI } } +/// The slots of every region capability `pid` holds. +fn region_slots(k: &Kernel, pid: Option) -> std::vec::Vec { + let Some(process) = pid.and_then(|p| k.processes.get(p).ok()) else { + return std::vec::Vec::new(); + }; + process + .capabilities() + .iter() + .filter(|(_, c)| matches!(c.resource(), domain::resource::Resource::Memory(_))) + .map(|(s, _)| s) + .collect() +} + #[test] fn the_kernel_survives_a_syscall_storm() { let mut total = [0usize; 36 + 1]; @@ -357,7 +375,20 @@ fn the_kernel_survives_a_syscall_storm() { if is_init && (nr == SYS_EXIT || nr == SYS_EXIT_THREAD) { continue; } - let args: [usize; 6] = core::array::from_fn(|_| rng.arg()); + let mut args: [usize; 6] = core::array::from_fn(|_| rng.arg()); + // Random arguments almost never name a region cap, a free + // address and valid perms at once; half the time, pick one of + // the caller's region caps (often the hub's shared region) so + // regions really are mapped into several processes. + if nr == SYS_MAP_REGION && rng.below(2) == 0 { + let regions = region_slots(&k, k.thread_process(tid)); + if !regions.is_empty() { + args[0] = regions[rng.below(regions.len())].value() as usize; + args[1] = 0x10000 + rng.below(32) * 4096; + // Read-write or read-execute: the pairs W^X forbids. + args[2] = [PERMS_READ_WRITE, PERMS_READ_EXECUTE][rng.below(2)]; + } + } // init keeps the capability it creates processes with. if is_init && nr == SYS_DROP_CAP && args[0] == 0 { continue; diff --git a/docs/ADAPTERS.md b/docs/ADAPTERS.md index efdae0b..f8c14ba 100644 --- a/docs/ADAPTERS.md +++ b/docs/ADAPTERS.md @@ -13,7 +13,12 @@ This generates `crates/adapter-/` with the standard structure. Then: 1. Add `"crates/adapter-"` to workspace `members` in `Cargo.toml` 2. Fill in `board.rs` with device addresses for the target board 3. Add boot assembly (`global_asm!` in `main.rs` or a `.S` file) -4. Update `linker.ld` for the target's entry address and memory layout +4. Update `linker.ld` for the target's entry address and memory layout. + Keep text, read-only data and writable data on separate pages and + export their page-aligned boundaries (`_text_start`, `_text_end`, + `_rodata_end`, `_stack_guard`, `_stack_bottom`): the kernel maps each + range with only the access it needs (W^X) and leaves a guard page under + the stack. 5. Implement the TODOs in `hw/`, `mmu/`, `trap/`, and `platform.rs` ## Structure diff --git a/docs/ipc.md b/docs/ipc.md index 6089154..93be00b 100644 --- a/docs/ipc.md +++ b/docs/ipc.md @@ -37,7 +37,11 @@ from a revocation further up. A region mapping made with `MapRegion(cap, va, perms)` lasts only as long as `cap`: deleting or revoking `cap` unmaps it. `perms` needs `Read`, plus -`Write` for read-write or `Execute` for read-execute. +`Write` for read-write or `Execute` for read-execute. W^X holds across +processes: while a region is mapped read-write anywhere it cannot be mapped +read-execute anywhere, and the other way round (`PermissionDenied`). To +load code, map it writable, fill it, drop that mapping, then map it +executable. | Object | Permission | Allows | |--------------|------------|--------------------------------------------|