diff --git a/issues/a-threads-segment-selectors-are-its-cpus.md b/issues/a-threads-segment-selectors-are-its-cpus.md index 2adb2bf6fa..a8ddb799e1 100644 --- a/issues/a-threads-segment-selectors-are-its-cpus.md +++ b/issues/a-threads-segment-selectors-are-its-cpus.md @@ -8,13 +8,12 @@ opened: 2026-11-11 Ring 3 loads DS, ES, FS and GS with any selector CPL 3 may load (SDM Vol. 2B, MOV): null or the user data or code descriptor, at any RPL. The x86-64 switch -(`kernel/src/arch/x86_64/hw.rs`) carries a thread's FS base and nothing of -those four selectors, so a thread switched in reads, with `mov ax, gs`, the -selector the thread before it on that CPU loaded, and loses its own. The -bases are not involved: every descriptor Ring 3 may load has base 0, the FS -base moves with the thread, and the kernel's GS base is swapped in at every -entry (`kernel/src/arch/x86_64/entry.rs`). Found by reading while building -that entry. +(`kernel/src/arch/x86_64/hw.rs`) carries a thread's FS and GS bases and +nothing of those four selectors, so a thread switched in reads, with `mov ax, +gs`, the selector the thread before it on that CPU loaded, and loses its own. +The bases are not involved: they move with the thread, and the kernel's GS +base is swapped in at every entry (`kernel/src/arch/x86_64/entry.rs`). Found +by reading while building that entry. **Exit**: a thread reads back the DS, ES, FS and GS selectors it loaded after its CPU has run a thread that loaded others, by the switch saving and diff --git a/issues/an-ap-takes-ist-vectors-before-its-tss.md b/issues/an-ap-takes-ist-vectors-before-its-tss.md new file mode 100644 index 0000000000..aaeca10ece --- /dev/null +++ b/issues/an-ap-takes-ist-vectors-before-its-tss.md @@ -0,0 +1,16 @@ +--- +status: open +kind: finding +opened: 2026-11-11 +--- + +# An AP takes IST vectors before its TSS + +An AP's trampoline (`kernel/src/arch/x86_64/smp.rs`) loads the kernel's IDT +and enters Rust under the BSP's GDT. The AP loads its own GDT and TSS +(`ltr`) only later, in `percpu::init_ap`, just after its declared `CR4`. +Until `ltr`, TR is still what INIT left, so an NMI, `#DB`, `#DF` or `#MC` +in that span takes its IST stack pointer from no TSS of the kernel's. Found +by reading while making those entries independent of GS. + +**Exit**: an AP holds its own GDT and TSS before it loads the IDT. diff --git a/issues/fsgsbase-is-taken.md b/issues/fsgsbase-is-taken.md index f5241baadb..b356e60d06 100644 --- a/issues/fsgsbase-is-taken.md +++ b/issues/fsgsbase-is-taken.md @@ -6,9 +6,6 @@ opened: 2026-09-29 # FSGSBASE is taken -`CR4.FSGSBASE` is forbidden (`CR4_FORBIDDEN`, `kernel/src/arch/x86_64/control_regs.rs`): -the switch moves no thread's GS base, since every GS load Ring 3 can make -leaves it zero, so a `WRGSBASE` would reach the next thread on the CPU. Intel names what it gives, user programs' FS and GS bases without a system call and a more efficient kernel context switch, and has it used "never in production environments unless accompanied by a complete OS implementation": @@ -30,9 +27,14 @@ guidance asks; NMI, `#DB`, `#MC` and `#DF` install the kernel's base from found. The `nmi-user-gs` actuator enters vector 2 at CPL 0 under a decoy kernel-half base, which a sign test of `IA32_GS_BASE` would take for the kernel's. +`CR4.FSGSBASE` is required of every CPU (`kernel/src/arch/x86_64/control_regs.rs`), +and the switch carries each thread's FS and GS bases +(`kernel/src/arch/x86_64/hw.rs`); `fsgs_bases_per_thread` reads them back +across yields on one CPU. -**Left**: `CR4.FSGSBASE` on every CPU, and the switch saving and restoring -each thread's GS base beside its FS base. +**Left**: the exit's runs: the vector-2 arm and the readback on each proving +machine and under TCG, and the T14's pipe round trips with and without +FSGSBASE. **Owed on metal** (#1262; the benches were down when it was reviewed). On the T14 and the AMD bench, main must show: @@ -51,6 +53,14 @@ the T14 and the AMD bench, main must show: - `nmi_finds_the_kernels_gs`, on the `shared-debug` boot: `nmi-user-gs: PASS`. - `syscall_cost`, on the `testcases` boot: within its bound. +**Owed on metal** (#1264, the same way). On the T14 and the AMD bench, main +must show: +- `fsgs_bases_per_thread`, on the `testcases` boot: both threads read back + their own bases, and both crossing counts are above 0. +- The rows above, through the entries' `RDGSBASE` and `WRGSBASE`. +- An A/B of main before and after #1264 on the T14's `benchcase` boot: + `pingpong.pipe.*`, reported. It decides nothing. + **Unobserved**: the `#DB` entry's resume into a gate it interrupted, where a single-step held over a `MOV SS` lands on the first instruction of `SYSCALL`'s entry or of the `#BP` gate (`tf-mov-ss-syscall`, diff --git a/issues/ring-3-replaces-the-kernels-gs-base-with-a-segment-load.md b/issues/ring-3-replaces-the-kernels-gs-base-with-a-segment-load.md index aaff2353fd..51fed90442 100644 --- a/issues/ring-3-replaces-the-kernels-gs-base-with-a-segment-load.md +++ b/issues/ring-3-replaces-the-kernels-gs-base-with-a-segment-load.md @@ -7,6 +7,9 @@ severity: [security, kernel-from-userland] # Ring 3 replaces the kernel's GS base with a segment load +Fixed in code by #1262; what follows is the defect as it stood, and below it +what the exit still owes. + The kernel runs no `swapgs`: GS holds this CPU's per-CPU block from boot on, and the `SYSCALL` entry's first instructions address it through `gs:` while still on the user stack (`kernel/src/arch/x86_64/syscall.rs`). `CR4.FSGSBASE` @@ -28,10 +31,22 @@ first. Every handler after it addresses `gs:` with the same zero base. Found by reading, while deciding whether the switch may keep a thread's FS base instead of reading it back (it may not, for the same reason); not reproduced. +**Built** (#1262): every entry from Ring 3 swaps in the kernel's GS base, and +the IST entries install it from `GDTR` whatever GS holds +(`kernel/src/arch/x86_64/entry.rs`). `gs_segment_load` is the exit's test. Its +child loads GS with each selector CPL 3 may load (null, user data and user +code, at RPL 0 to 3) and enters the kernel by a syscall, a page fault and a +park. The call answers, and GS's base reads back as its own. It was green +under QEMU. Its negative control, the fix reverted, stalled the machine at the +first entry after a GS load. + +**Owed on metal** (the benches were down when #1262 landed): on the T14 and +the AMD bench, main's `testcases` boot must show `gs_segment_load` with all 12 +selectors reported. `issues/fsgsbase-is-taken.md` lists it with the rest #1262 +owes. The record closes when it has. + **Exit**: a guest test, run on the T14 and the AMD bench, whose child loads GS with each selector Ring 3 can name and then makes a system call, and the kernel survives and ends the child; its negative control is the kernel without the -fix. The fix is an entry that does not trust the GS base it finds, the owner's -to choose; `issues/fsgsbase-is-taken.md` weighs the `swapgs` one. A GDT without -a loadable Ring 3 data descriptor is not one: `IRETQ` to Ring 3 needs SS to name -a writable DPL-3 data segment. +fix. A GDT without a loadable Ring 3 data descriptor is no fix: `IRETQ` to +Ring 3 needs SS to name a writable DPL-3 data segment. diff --git a/issues/the-guest-suite-runs-only-what-no-cheaper-tier-reaches.md b/issues/the-guest-suite-runs-only-what-no-cheaper-tier-reaches.md index ae53045b6e..86300b9b3c 100644 --- a/issues/the-guest-suite-runs-only-what-no-cheaper-tier-reaches.md +++ b/issues/the-guest-suite-runs-only-what-no-cheaper-tier-reaches.md @@ -83,9 +83,9 @@ Metal: test kernels armed on the T14. Host: `kernel::sched` and `kernel-sim`, `k `toyos-wallclock`. Exit: as stage A; an unrepresentable name is a type the kernel builds against and a compile-fail case. -- metal `fpu_isolation`, `gsbase_locked`, first: x86-64 FPU isolation runs nowhere until this - lands. One process's FPU state and GS base never reach another; brings back the - `fpu-save-nothing` and `user-writable-gsbase` features, their negative controls. +- metal `fpu_isolation`, first: x86-64 FPU isolation runs nowhere until this lands. One + process's FPU state never reaches another; brings back the `fpu-save-nothing` feature, its + negative control. - metal `control_regs_negative`: an AP left without the declared control registers is caught, under `no-ap-control-regs`. - metal `va_exhaustion`, `heap_ceiling_bounds`: an exhausted address space and heap refuse by name. diff --git a/kernel/src/arch/aarch64/hw.rs b/kernel/src/arch/aarch64/hw.rs index d3db4ff8c8..f19121aa10 100644 --- a/kernel/src/arch/aarch64/hw.rs +++ b/kernel/src/arch/aarch64/hw.rs @@ -61,6 +61,15 @@ fn check_switch_frame(ctx: &KernelCtx) -> u64 { sp } +/// Nothing: an AArch64 thread's one base register is its thread pointer, +/// `TPIDR_EL0`. +#[derive(Clone, Copy)] +pub struct UserBases; + +impl UserBases { + pub const INITIAL: Self = Self; +} + impl Hw for KernelHw { type Payload = KernelPayload; diff --git a/kernel/src/arch/x86_64/control_regs.rs b/kernel/src/arch/x86_64/control_regs.rs index ce988731ad..793350d7e0 100644 --- a/kernel/src/arch/x86_64/control_regs.rs +++ b/kernel/src/arch/x86_64/control_regs.rs @@ -166,8 +166,11 @@ pub const CR0: u64 = cr0::PE | cr0::MP | cr0::ET | cr0::NE | cr0::WP | cr0::PG; /// from or touching a user page (SDM Vol. 3A §4.6), which every CPU of this /// kernel's range (2020 on) offers, so one without them is refused rather than /// run with the user half open to a kernel bug; and every Ring 0 entry's -/// `clac` (`arch::entry`) is `#UD` without `SMAP`. `FSGSBASE` is -/// [`CR4_FORBIDDEN`], not here. +/// `clac` (`arch::entry`) is `#UD` without `SMAP`. `FSGSBASE`, in the same +/// range, gives Ring 3 its own FS and GS bases and the kernel both without an +/// MSR, under Intel's condition (Guidance for Enabling FSGSBASE, ID 767615): +/// the switch carries both bases per thread (`arch::hw`), and no entry trusts +/// the GS base it finds (`arch::entry`). const CR4_REQUIRED: u64 = cr4::DE | cr4::PAE | cr4::MCE @@ -176,10 +179,11 @@ const CR4_REQUIRED: u64 = cr4::DE | cr4::OSXMMEXCPT | cr4::OSXSAVE | cr4::SMEP - | cr4::SMAP; + | cr4::SMAP + | cr4::FSGSBASE; /// [`CR4_REQUIRED`]'s bits by name, for a refusal to say which a CPU lacks. -const CR4_REQUIRED_NAMES: [(u64, &str); 9] = [ +const CR4_REQUIRED_NAMES: [(u64, &str); 10] = [ (cr4::DE, "DE"), (cr4::PAE, "PAE"), (cr4::MCE, "MCE"), @@ -189,6 +193,7 @@ const CR4_REQUIRED_NAMES: [(u64, &str); 9] = [ (cr4::OSXSAVE, "OSXSAVE"), (cr4::SMEP, "SMEP"), (cr4::SMAP, "SMAP"), + (cr4::FSGSBASE, "FSGSBASE"), ]; const _: () = { @@ -217,17 +222,6 @@ impl core::fmt::Display for Cr4Names { /// `CR4` bits this kernel takes when the CPU offers them (checked against CPUID first: an undefined bit is `#GP`). const CR4_OPTIONAL: u64 = cr4::PCIDE | cr4::UMIP; -/// The `CR4` bit no shipping CPU may hold: `FSGSBASE` gives Ring 3 `WRGSBASE`, -/// and the switch moves no thread's GS base, which every Ring 3 GS load -/// leaves zero (`percpu::clear_user_gs_base`) — one thread's would reach the -/// next. The kernel's FS base uses `IA32_FS_BASE` (`cpu::write_fs_base`) -/// instead. -const CR4_FORBIDDEN: u64 = cr4::FSGSBASE; - -// Forbidden in the declaration, and never optional. -const _: () = assert!(CR4_REQUIRED & CR4_FORBIDDEN == 0); -const _: () = assert!(CR4_OPTIONAL & CR4_FORBIDDEN == 0); - /// `IA32_EFER` on every CPU: `SCE`, `LME`, `LMA`, `NXE`. `SCE` is declared only /// here, never by `arch::syscall::init`, so one register keeps one owner. pub const EFER: u64 = efer::SCE | efer::LME | efer::LMA | efer::NXE; diff --git a/kernel/src/arch/x86_64/cpu.rs b/kernel/src/arch/x86_64/cpu.rs index cb3f2c439b..3bfb6e93fa 100644 --- a/kernel/src/arch/x86_64/cpu.rs +++ b/kernel/src/arch/x86_64/cpu.rs @@ -436,20 +436,37 @@ pub fn disable_interrupts() { } } -/// The kernel's own FS base, not `rd/wrfsbase` (`control_regs::CR4_FORBIDDEN`). -const IA32_FS_BASE: u32 = 0xC000_0100; - +/// This CPU's FS base, which Ring 3 may have written itself (`CR4.FSGSBASE`). #[inline] pub fn read_fs_base() -> u64 { - rdmsr(IA32_FS_BASE) + let base: u64; + // SAFETY: reads FS's base alone; `CR4.FSGSBASE` is every CPU's declaration. + unsafe { asm!("rdfsbase {}", out(reg) base, options(nomem, nostack, preserves_flags)) }; + base } /// # Safety /// `val` becomes this CPU's FS base; #GP if non-canonical, and the caller owns which thread's thread-locals it addresses. #[inline] pub unsafe fn write_fs_base(val: u64) { - // SAFETY: `IA32_FS_BASE` takes any canonical value; the caller owns both. - unsafe { wrmsr(IA32_FS_BASE, val) }; + // SAFETY: the caller's; `CR4.FSGSBASE` is every CPU's declaration. + unsafe { asm!("wrfsbase {}", in(reg) val, options(nomem, nostack, preserves_flags)) }; +} + +/// The GS base Ring 3 returns to: `IA32_KERNEL_GS_BASE` while the kernel runs +/// (`arch::entry`). +#[inline] +pub fn read_user_gs_base() -> u64 { + rdmsr(super::percpu::MSR_KERNEL_GS_BASE) +} + +/// # Safety +/// `val` is canonical, and the GS base of the thread this CPU next returns to +/// Ring 3. +#[inline] +pub unsafe fn write_user_gs_base(val: u64) { + // SAFETY: the caller's. + unsafe { wrmsr(super::percpu::MSR_KERNEL_GS_BASE, val) }; } pub fn halt() -> ! { diff --git a/kernel/src/arch/x86_64/entry.rs b/kernel/src/arch/x86_64/entry.rs index e225af1a3d..4ccea97745 100644 --- a/kernel/src/arch/x86_64/entry.rs +++ b/kernel/src/arch/x86_64/entry.rs @@ -96,23 +96,20 @@ macro_rules! syscall_naked_asm { }; } -/// Puts this CPU's `PerCpu` in `IA32_GS_BASE` whatever GS held, for a gate -/// that can arrive in the kernel's CPL-0 stretches with a user's base live. +/// Puts this CPU's `PerCpu` in GS's base whatever it held, for a gate that +/// can arrive in the kernel's CPL-0 stretches with a user's base live. /// `GDTR`'s base is the block's own `gdt` (`percpu::load_gdt`), so it names -/// the CPU without reading GS. Clobbers `rax`, `rcx` and `rdx`; the operands -/// are `gs_base_msr` and `gdt_in_percpu`. No branch precedes the `wrmsr`, so -/// no load through GS after it is speculated with another base. +/// the CPU without reading GS. Clobbers `rax`; the operand is +/// `gdt_in_percpu`. No branch precedes the `wrgsbase`, so no load through GS +/// after it is speculated with another base. macro_rules! install_kernel_gs { () => { - "mov ecx, {gs_base_msr}\n\ - sub rsp, 16\n\ + "sub rsp, 16\n\ sgdt [rsp + 6]\n\ mov rax, [rsp + 8]\n\ add rsp, 16\n\ sub rax, {gdt_in_percpu}\n\ - mov rdx, rax\n\ - shr rdx, 32\n\ - wrmsr" + wrgsbase rax" }; } diff --git a/kernel/src/arch/x86_64/hw.rs b/kernel/src/arch/x86_64/hw.rs index 9f5f8fc1e8..7b26e913c2 100644 --- a/kernel/src/arch/x86_64/hw.rs +++ b/kernel/src/arch/x86_64/hw.rs @@ -250,6 +250,18 @@ pub fn declare_vendor(vendor: kernel::cpu::Vendor) { SYSRET_KEEPS_SS.store(vendor != kernel::cpu::Vendor::Intel, Relaxed); } +/// A thread's GS base, which Ring 3 writes with `WRGSBASE` and the kernel +/// keeps in `IA32_KERNEL_GS_BASE` while it runs (`arch::entry`). +#[derive(Clone, Copy)] +pub struct UserBases { + gs: u64, +} + +impl UserBases { + /// A thread's before it first runs. + pub const INITIAL: Self = Self { gs: 0 }; +} + fn read_ss() -> u16 { let ss: u16; // SAFETY: reads a segment selector into a scratch register; touches nothing else. @@ -309,18 +321,22 @@ impl Hw for KernelHw { // moves `IF`; the resumed context's own guard puts back what it had. #[cfg(feature = "mask-windows")] let _masked = crate::arch::IrqGuard::close(); - // SAFETY: `save`/`restore` are live Box-backed contexts from `SchedPass::finish`, freed only by a later pass; `incoming.thread_pointer` is this kernel's own canonical value for the thread being installed; `area` is the outgoing task's own, saved as it leaves this CPU and restored when it returns into this frame. + // SAFETY: `save`/`restore` are live Box-backed contexts from `SchedPass::finish`, freed only by a later pass; `incoming.thread_pointer` and `incoming.user_bases` are canonical values the CPU itself handed the switch out of that thread, or zero; `area` is the outgoing task's own, saved as it leaves this CPU and restored when it returns into this frame. unsafe { // A thread's user state area lies above its stack's top (`fpu`); // the idle context has none, and switches with neither a save nor a // restore. The thread resumes in this frame, so the restore below - // reads the area its own save wrote. Its FS base is read back, not - // kept: Ring 3 moves it with a segment load (SDM Vol. 2B, MOV). + // reads the area its own save wrote. Its bases are read back, not + // kept: Ring 3 moves them with a segment load (SDM Vol. 2B, MOV), + // `WRFSBASE` and `WRGSBASE`. let area = match (*save).runs { Runs::User(_) => { - let base = cpu::read_fs_base(); - (*save).thread_pointer = base; - percpu::set_fs_base(base); + let fs = cpu::read_fs_base(); + (*save).thread_pointer = fs; + percpu::set_fs_base(fs); + let gs = cpu::read_user_gs_base(); + (*save).user_bases.gs = gs; + percpu::set_user_gs_base(gs); Some((*save).kernel_stack_top) } Runs::Idle => None, @@ -353,6 +369,10 @@ impl Hw for KernelHw { cpu::write_fs_base(incoming.thread_pointer); percpu::set_fs_base(incoming.thread_pointer); } + if percpu::user_gs_base() != incoming.user_bases.gs { + cpu::write_user_gs_base(incoming.user_bases.gs); + percpu::set_user_gs_base(incoming.user_bases.gs); + } } // idle's stack top is per-CPU, unknowable at boot-time init, so // it is read here instead. It keeps whatever root is loaded: diff --git a/kernel/src/arch/x86_64/idt/debug.rs b/kernel/src/arch/x86_64/idt/debug.rs index 434b46e59d..c4064cb577 100644 --- a/kernel/src/arch/x86_64/idt/debug.rs +++ b/kernel/src/arch/x86_64/idt/debug.rs @@ -19,7 +19,7 @@ use core::arch::naked_asm; use crate::arch::entry::install_kernel_gs; -use crate::arch::percpu::{MSR_GS_BASE, OFF_GDT, OFF_KERNEL_RSP, USER_CS, USER_DS}; +use crate::arch::percpu::{OFF_GDT, OFF_KERNEL_RSP, USER_CS, USER_DS}; /// Eleven pushes place the CPU frame's `rip` at 88. const RIP_OFFSET: usize = 88; @@ -34,14 +34,11 @@ pub(super) extern "sysv64" fn debug_entry() { "test byte ptr [rsp + 8], 3", "jnz 2f", "push rax", - "push rcx", - "push rdx", - "mov ecx, {gs_base_msr}", - "rdmsr", - "shl rdx, 32", - "or rax, rdx", + "rdgsbase rax", "push rax", install_kernel_gs!(), + "push rcx", + "push rdx", "push rsi", "push rdi", "push r8", @@ -61,14 +58,11 @@ pub(super) extern "sysv64" fn debug_entry() { "pop r8", "pop rdi", "pop rsi", - // The base this `#DB` found, back: nothing after it reads GS. - "pop rax", - "mov rdx, rax", - "shr rdx, 32", - "mov ecx, {gs_base_msr}", - "wrmsr", "pop rdx", "pop rcx", + // The base this `#DB` found, back: nothing after it reads GS. + "pop rax", + "wrgsbase rax", "pop rax", // `deferred` makes the frame a Ring 3 one where its thread is to end. "test byte ptr [rsp + 8], 3", @@ -98,7 +92,6 @@ pub(super) extern "sysv64" fn debug_entry() { "push 0", "push {vector}", "jmp {common}", - gs_base_msr = const MSR_GS_BASE, gdt_in_percpu = const OFF_GDT, kernel_rsp = const OFF_KERNEL_RSP, rip_offset = const RIP_OFFSET, diff --git a/kernel/src/arch/x86_64/idt/mod.rs b/kernel/src/arch/x86_64/idt/mod.rs index 5bc05eff25..a27395d659 100644 --- a/kernel/src/arch/x86_64/idt/mod.rs +++ b/kernel/src/arch/x86_64/idt/mod.rs @@ -365,7 +365,6 @@ extern "sysv64" fn fatal_entry() { "call {dispatch}", "ud2", dispatch = sym trap_dispatch, - gs_base_msr = const percpu::MSR_GS_BASE, gdt_in_percpu = const percpu::OFF_GDT, ); } diff --git a/kernel/src/arch/x86_64/idt/nmi.rs b/kernel/src/arch/x86_64/idt/nmi.rs index 9fb53054b7..2686a40086 100644 --- a/kernel/src/arch/x86_64/idt/nmi.rs +++ b/kernel/src/arch/x86_64/idt/nmi.rs @@ -13,11 +13,11 @@ use core::arch::naked_asm; use crate::arch::entry::install_kernel_gs; -use crate::arch::percpu::{MSR_GS_BASE, OFF_GDT, OFF_NMI_ACTIVE}; +use crate::arch::percpu::{OFF_GDT, OFF_NMI_ACTIVE}; -/// The words the entry pushes before it can tell a nested NMI: three -/// registers and the GS base it found. -const FIRST_PUSHES: usize = 4; +/// The words the entry pushes before it can tell a nested NMI: `rax` and the +/// GS base it found. +const FIRST_PUSHES: usize = 2; /// The interrupt frame's `rip` and `rsp`, at the nested check. const NESTED_RIP_OFFSET: usize = 8 * FIRST_PUSHES; @@ -26,7 +26,7 @@ const NESTED_RSP_OFFSET: usize = NESTED_RIP_OFFSET + 24; /// Eleven pushes in all place the frame's `rip` at 88, and its `rflags` two /// words above. `IF` is what separates a cpu that has stopped taking /// interrupts from one that is merely slow. -const RFLAGS_OFFSET: usize = 8 * (FIRST_PUSHES + 7) + 16; +const RFLAGS_OFFSET: usize = 8 * (FIRST_PUSHES + 9) + 16; #[unsafe(naked)] pub(super) extern "sysv64" fn nmi_entry() { @@ -37,17 +37,14 @@ pub(super) extern "sysv64" fn nmi_entry() { "clac", // Flags are free throughout: iretq restores RFLAGS from the frame. "push rax", - "push rcx", - "push rdx", - "mov ecx, {gs_base_msr}", - "rdmsr", - "shl rdx, 32", - "or rax, rdx", + "rdgsbase rax", "push rax", install_kernel_gs!(), "cmp dword ptr gs:[{active}], 0", "jne 2f", "mov dword ptr gs:[{active}], 1", + "push rcx", + "push rdx", "push rsi", "push rdi", "push r8", @@ -67,16 +64,13 @@ pub(super) extern "sysv64" fn nmi_entry() { "pop r8", "pop rdi", "pop rsi", + "pop rdx", + "pop rcx", // Cleared before iretq, never after: delivery is blocked until iretq retires, so no second NMI can find it clear while this one is still on the stack. "mov dword ptr gs:[{active}], 0", // The base this NMI found, back: nothing after it reads GS. "pop rax", - "mov rdx, rax", - "shr rdx, 32", - "mov ecx, {gs_base_msr}", - "wrmsr", - "pop rdx", - "pop rcx", + "wrgsbase rax", "pop rax", // No EOI: an NMI isn't delivered through the IRR, and acknowledging one would clear an unrelated interrupt. "iretq", @@ -88,7 +82,6 @@ pub(super) extern "sysv64" fn nmi_entry() { "call {nested}", "ud2", active = const OFF_NMI_ACTIVE, - gs_base_msr = const MSR_GS_BASE, gdt_in_percpu = const OFF_GDT, rflags_offset = const RFLAGS_OFFSET, nested_rip = const NESTED_RIP_OFFSET, diff --git a/kernel/src/arch/x86_64/idt/nmi_user_gs.rs b/kernel/src/arch/x86_64/idt/nmi_user_gs.rs index 56a3fd666f..7dcfa6ae61 100644 --- a/kernel/src/arch/x86_64/idt/nmi_user_gs.rs +++ b/kernel/src/arch/x86_64/idt/nmi_user_gs.rs @@ -23,16 +23,16 @@ pub fn selftest() { // Closed across the swapped span: a maskable gate there would read a // Ring 0 frame and keep the decoy. let _masked = crate::arch::IrqGuard::close(); - let kept = cpu::rdmsr(percpu::MSR_KERNEL_GS_BASE); + let kept = cpu::read_user_gs_base(); // SAFETY: the swapped span is `int 2` alone, which touches no // GS-relative memory of this code's; the second `swapgs` leaves in // `IA32_KERNEL_GS_BASE` the base the handler returned to, and this // thread's user GS base goes back there before interrupts open. unsafe { - cpu::wrmsr(percpu::MSR_KERNEL_GS_BASE, staged); + cpu::write_user_gs_base(staged); core::arch::asm!("swapgs", "int 2", "swapgs"); - returned_to = cpu::rdmsr(percpu::MSR_KERNEL_GS_BASE); - cpu::wrmsr(percpu::MSR_KERNEL_GS_BASE, kept); + returned_to = cpu::read_user_gs_base(); + cpu::write_user_gs_base(kept); } } let after = irq_census::deliveries(cpu, Source::Nmi).unwrap_or(before); diff --git a/kernel/src/arch/x86_64/percpu.rs b/kernel/src/arch/x86_64/percpu.rs index b9d8423144..6ebade5d0e 100644 --- a/kernel/src/arch/x86_64/percpu.rs +++ b/kernel/src/arch/x86_64/percpu.rs @@ -129,8 +129,13 @@ pub struct PerCpu { /// `IA32_FS_BASE` as `load_gdt` left it or this CPU's last switch read or /// wrote it. Exact at every switch-in, since every switch away from a user /// thread reads the register back; stale while a thread runs, which moves - /// it by a segment load or `SYS_SET_THREAD_POINTER`. + /// it by a segment load, `WRFSBASE` or `SYS_SET_THREAD_POINTER`. fs_base: u64, + /// `IA32_KERNEL_GS_BASE`, the user GS base while the kernel runs, as this + /// CPU's last switch read or wrote it; [`NO_GS_BASE`] before the first. + /// Exact at every switch-in as `fs_base` is; stale while a thread runs, + /// which moves it by a segment load or `WRGSBASE`. + user_gs_base: u64, /// The context whose switch-out last left its user state in this CPU's /// registers, 0 before the first: `arch::fpu`'s record. fpu_holder: u64, @@ -148,13 +153,6 @@ const GDT_ENTRIES: [u64; 7] = [ 0, // TSS high (runtime) ]; -/// A descriptor's base, bits 16–39 and 56–63 (SDM Vol. 3A §3.4.5). -const DESCRIPTOR_BASE: u64 = 0xFF00_00FF_FFFF_0000; - -// The two descriptors Ring 3 may load into GS, its data and its code: their -// base is the one a user GS load makes (`clear_user_gs_base`). -const _: () = assert!(GDT_ENTRIES[3] & DESCRIPTOR_BASE == 0 && GDT_ENTRIES[4] & DESCRIPTOR_BASE == 0); - #[repr(C, packed)] struct GdtPointer { limit: u16, @@ -244,6 +242,12 @@ pub(crate) const OFF_NMI_ACTIVE: u32 = offset_of!(PerCpu, nmi_active) as u32; const OFF_AP_TOKEN: u32 = offset_of!(PerCpu, ap_token) as u32; const OFF_ROOT: u32 = offset_of!(PerCpu, root) as u32; const OFF_FS_BASE: u32 = offset_of!(PerCpu, fs_base) as u32; +const OFF_USER_GS_BASE: u32 = offset_of!(PerCpu, user_gs_base) as u32; + +/// `user_gs_base`'s start, which no thread's base equals: it is not canonical, +/// and `WRGSBASE` and `WRMSR` refuse a base that is not (SDM Vol. 2D, WRGSBASE), +/// so a CPU's first switch into a thread always writes the thread's. +const NO_GS_BASE: u64 = 1 << 63; const OFF_FPU_HOLDER: u32 = offset_of!(PerCpu, fpu_holder) as u32; /// Where this CPU's interrupt counters start; `irq_census::slot_offset` derives every handler's offset from it. pub const OFF_IRQ_COUNTS: u32 = offset_of!(PerCpu, irq_counts) as u32; @@ -415,6 +419,7 @@ fn alloc_percpu(cpu_id: u32) -> *mut PerCpu { // `load_gdt` loads FS with `KERNEL_DS`, whose base is 0, and a // segment load clears the base's upper half (SDM Vol. 3A §3.4.4). fs_base: 0, + user_gs_base: NO_GS_BASE, fpu_holder: 0, irq_counts: [const { AtomicU64::new(0) }; crate::irq_census::Source::COUNT], }, @@ -586,7 +591,6 @@ pub fn init_bsp(lapic_id: u32) { // SAFETY: the write that makes `gs:` valid on the BSP; `ptr`'s `&mut` ended at `load_gdt` above, so this hands the CPU its only reference. unsafe { cpu::wrmsr(MSR_GS_BASE, ptr as u64) }; - clear_user_gs_base(); super::control_regs::init(0); // SAFETY: this CPU's own block, with no IDT of this kernel's loaded yet // and the `&mut` above ended. @@ -611,14 +615,6 @@ pub fn init_bsp(lapic_id: u32) { log!("percpu: BSP cpu_id=0 lapic_id={lapic_id}"); } -/// Gives this CPU the user GS base every thread has: zero. Every descriptor -/// Ring 3 may load into GS has base 0 and `CR4.FSGSBASE` is off, so no thread -/// makes another and the switch moves none. Before this CPU's first `swapgs`. -fn clear_user_gs_base() { - // SAFETY: zero is canonical, and nothing on this CPU has swapped GS yet. - unsafe { cpu::wrmsr(MSR_KERNEL_GS_BASE, 0) }; -} - /// Allocate percpu for an AP; the trampoline writes the pointer into IA32_GS_BASE. pub fn alloc_ap(cpu_id: u32, token: u32) -> *mut PerCpu { let ptr = alloc_percpu(cpu_id); @@ -634,10 +630,11 @@ pub fn alloc_ap(cpu_id: u32, token: u32) -> *mut PerCpu { pub fn init_ap(percpu_ptr: *mut PerCpu) { // SAFETY: this CPU's own `PerCpu`, read from `gs:[0]`; the BSP dropped its `&mut` before the SIPI. let percpu = unsafe { &mut *percpu_ptr }; + // Before `load_gdt`'s `ltr`: from there the IST entries have their stacks, + // and they need `CR4.FSGSBASE` for `rdgsbase` and `wrgsbase`. + super::control_regs::init(percpu.cpu_id); // SAFETY: `load_gdt`'s once-per-CPU contract; this is this AP's call. unsafe { percpu.load_gdt(); } - clear_user_gs_base(); - super::control_regs::init(percpu.cpu_id); super::control_regs::init_performance(percpu.cpu_id); super::fpu::init(percpu.cpu_id); super::fpu::log_state(percpu.cpu_id); @@ -725,6 +722,15 @@ pub fn set_fs_base(base: u64) { gs::write_u64::(base); } +/// `IA32_KERNEL_GS_BASE` as the switch last read or wrote it on this CPU. +pub fn user_gs_base() -> u64 { + gs::read_u64::() +} + +pub fn set_user_gs_base(base: u64) { + gs::write_u64::(base); +} + /// The context whose switch-out last left its user state in this CPU's /// registers; 0 before the first. pub fn fpu_holder() -> u64 { diff --git a/kernel/src/sched/driver.rs b/kernel/src/sched/driver.rs index 7cab127cb8..36ff4acf0e 100644 --- a/kernel/src/sched/driver.rs +++ b/kernel/src/sched/driver.rs @@ -375,6 +375,7 @@ fn idle_ctx() -> KernelCtx { // a switch away from idle always writes the real depth first. preempt: 0, fpu: crate::arch::fpu::Left::NOWHERE, + user_bases: crate::arch::hw::UserBases::INITIAL, } } @@ -420,6 +421,7 @@ pub fn spawn(new: NewTask) -> (ThreadSched, CpuId) { // The one level `trampoline_entry` discharges before the first `iretq`. preempt: 1, fpu: crate::arch::fpu::Left::NOWHERE, + user_bases: crate::arch::hw::UserBases::INITIAL, }; // One clock read for the placement and the build: `HW.now` is a divide. let now = HW.now(); diff --git a/kernel/src/sched/payload.rs b/kernel/src/sched/payload.rs index 99c531a987..de5509e9d2 100644 --- a/kernel/src/sched/payload.rs +++ b/kernel/src/sched/payload.rs @@ -51,6 +51,8 @@ pub struct KernelCtx { pub preempt: u32, /// Where the switch last left this context's user machine state. pub fpu: crate::arch::fpu::Left, + /// The user registers besides the thread pointer that the switch carries. + pub user_bases: crate::arch::hw::UserBases, } /// What a context runs. diff --git a/tests/guest/rust-tests/src/arch/fsgsbase.rs b/tests/guest/rust-tests/src/arch/fsgsbase.rs new file mode 100644 index 0000000000..6fc88109e3 --- /dev/null +++ b/tests/guest/rust-tests/src/arch/fsgsbase.rs @@ -0,0 +1,46 @@ +//! FS and GS bases as Ring 3 writes them under `CR4.FSGSBASE`. + +#[cfg(target_arch = "x86_64")] +pub use x86_64::*; + +#[cfg(target_arch = "x86_64")] +mod x86_64 { + /// This thread's GS base. + pub fn gs_base() -> u64 { + let base: u64; + // SAFETY: reads GS's base alone. + unsafe { core::arch::asm!("rdgsbase {}", out(reg) base, options(nomem, nostack, preserves_flags)) }; + base + } + + /// Writes `fs` and `gs` as this thread's bases, makes syscall `yield_number`, + /// and reads both back; the FS base it found is back before it returns, so + /// nothing here runs on another thread-local block. The bases it read. + pub fn bases_across_a_yield(fs: u64, gs: u64, yield_number: u64) -> (u64, u64) { + let (fs_read, gs_read): (u64, u64); + // SAFETY: both bases canonical, as `WRFSBASE` and `WRGSBASE` require; + // nothing between the writes and the restore addresses FS or GS, and + // the `syscall` clobbers rax, rcx and r11. + unsafe { + core::arch::asm!( + "rdfsbase {kept}", + "wrfsbase {fs}", + "wrgsbase {gs}", + "syscall", + "rdfsbase {fs_read}", + "rdgsbase {gs_read}", + "wrfsbase {kept}", + kept = out(reg) _, + fs = in(reg) fs, + gs = in(reg) gs, + fs_read = out(reg) fs_read, + gs_read = out(reg) gs_read, + in("rdi") yield_number, + lateout("rax") _, + out("rcx") _, + out("r11") _, + ); + } + (fs_read, gs_read) + } +} diff --git a/tests/guest/rust-tests/src/bin/fsgs_bases_per_thread.rs b/tests/guest/rust-tests/src/bin/fsgs_bases_per_thread.rs new file mode 100644 index 0000000000..9480717bc3 --- /dev/null +++ b/tests/guest/rust-tests/src/bin/fsgs_bases_per_thread.rs @@ -0,0 +1,59 @@ +//! Under `CR4.FSGSBASE` a Ring 3 thread writes its own FS and GS bases, and +//! the switch keeps each thread's: two threads pinned to one CPU each write +//! bases of their own, yield, and read them back while the other writes +//! others. One thread's GS base is a kernel-half address, which `WRGSBASE` +//! allows and no kernel entry may take for its own. + +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::Barrier; +use std::thread; + +use core_affinity::CoreId; +use toyos_abi::syscall::SYS_YIELD; + +#[path = "../arch/fsgsbase.rs"] +mod fsgsbase; + +const ROUNDS: u64 = 500; + +/// Each thread's finished rounds: a change in the other's across one of a +/// thread's rounds is the other having run on their one CPU meanwhile. +static ROUNDS_DONE: [AtomicU64; 2] = [const { AtomicU64::new(0) }; 2]; + +/// Both threads pinned before either starts its rounds, so each runs across +/// the other's. +static PINNED: Barrier = Barrier::new(2); + +fn main() { + let sibling = thread::spawn(|| rounds(1)); + let mine = rounds(0); + let theirs = sibling.join().expect("the sibling's rounds"); + assert!(mine > 0 && theirs > 0, "the two threads never ran across each other's rounds: {mine} and {theirs}"); + println!( + "fsgs_bases_per_thread: two threads on cpu0 read back their own FS and GS bases over {ROUNDS} yields each, \ + the other running across {mine} and {theirs} of them" + ); +} + +/// Thread `me`'s rounds, and how many of them the other thread ran across. +fn rounds(me: usize) -> u64 { + assert!(core_affinity::set_for_current(CoreId { id: 0 }), "pin thread {me} to cpu0"); + PINNED.wait(); + // Before either thread writes one: the base a thread starts with, which a + // switch that kept another thread's base or record would not give it. + let first = fsgsbase::gs_base(); + assert_eq!(first, 0, "thread {me} started with GS base {first:#x}"); + let mut crossed = 0; + for round in 0..ROUNDS { + let fs = 0x0000_7000_0000_0000 | (me as u64) << 40 | round << 4; + let gs = [0x0000_6000_0000_0000, 0xffff_9000_0000_0000][me] | round << 4; + let other = ROUNDS_DONE[1 - me].load(Ordering::Acquire); + let read = fsgsbase::bases_across_a_yield(fs, gs, SYS_YIELD); + assert_eq!(read, (fs, gs), "thread {me} round {round} wrote FS {fs:#x} and GS {gs:#x} and read {read:#x?}"); + ROUNDS_DONE[me].fetch_add(1, Ordering::Release); + if ROUNDS_DONE[1 - me].load(Ordering::Acquire) != other { + crossed += 1; + } + } + crossed +}