From d237fe907cec0de17e621ca02cda0698fce96e9a Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 11 Nov 2026 19:41:28 +0200 Subject: [PATCH 1/6] x86-64: FSGSBASE is taken, and the switch carries each thread's FS and GS bases Intel has FSGSBASE used only with a complete OS implementation: a switch and process-trace calls that manage the FS and GS bases apart from their selectors, and ring transitions insensitive to the GS base (Guidance for Enabling FSGSBASE, ID 767615). The previous commits made every transition insensitive to it, and ToyOS has no call that reads or writes another thread's registers, so what was left is the switch: - `CR4.FSGSBASE` joins `CR4_REQUIRED`: every CPU of the kernel's range (Intel since 2012, AMD since Zen) has it, so one without it is refused by name like one without SMEP, and `CR4_FORBIDDEN` goes. - The switch saves the outgoing thread's user GS base from `IA32_KERNEL_GS_BASE`, where the kernel keeps it while it runs, and writes the incoming one's (`arch::hw::UserBases`, which AArch64 defines empty: its one base register is the thread pointer). A thread's FS base moves as before, now by `RDFSBASE` and `WRFSBASE`. - NMI, `#MC` and `#DF` read and install GS's base with `RDGSBASE` and `WRGSBASE` instead of the MSR, clobbering only `rax`. No branch precedes the write, so no load through GS after it is speculated with another base. - The zero-base premise goes with them: a thread now writes any canonical GS base, a kernel-half one included, so `percpu::clear_user_gs_base` and the compile-time check of the user descriptors' bases are deleted. The switch writes every incoming thread's base before it can reach Ring 3. `fsgs_bases_per_thread` pins two threads to one CPU; each writes bases of its own with `WRFSBASE` and `WRGSBASE` (one GS base kernel-half), yields and reads them back. It is a shared-boot member on each bench; `a_threads_fs_and_gs_bases_stay_its_own` runs it under QEMU. Stage C of `issues/the-guest-suite-runs-only-what-no-cheaper-tier-reaches.md` no longer brings back `gsbase_locked` or its `user-writable-gsbase` kernel: a user-written GS base is now the design, and the readback is its test. Co-Authored-By: Claude Opus 5.5 --- ...-threads-segment-selectors-are-its-cpus.md | 13 +++-- issues/fsgsbase-is-taken.md | 12 +++-- ...-runs-only-what-no-cheaper-tier-reaches.md | 6 +-- kernel/src/arch/aarch64/hw.rs | 9 ++++ kernel/src/arch/x86_64/control_regs.rs | 24 ++++----- kernel/src/arch/x86_64/cpu.rs | 29 ++++++++--- kernel/src/arch/x86_64/entry.rs | 34 ++++++------- kernel/src/arch/x86_64/hw.rs | 32 +++++++++--- kernel/src/arch/x86_64/idt/debug.rs | 21 +++----- kernel/src/arch/x86_64/idt/mod.rs | 1 - kernel/src/arch/x86_64/idt/nmi.rs | 29 +++++------ kernel/src/arch/x86_64/idt/nmi_user_gs.rs | 8 +-- kernel/src/arch/x86_64/percpu.rs | 35 +++++++------ kernel/src/sched/driver.rs | 2 + kernel/src/sched/payload.rs | 2 + tests/guest/rust-tests/src/arch/fsgsbase.rs | 38 ++++++++++++++ .../src/bin/fsgs_bases_per_thread.rs | 49 +++++++++++++++++++ 17 files changed, 228 insertions(+), 116 deletions(-) create mode 100644 tests/guest/rust-tests/src/arch/fsgsbase.rs create mode 100644 tests/guest/rust-tests/src/bin/fsgs_bases_per_thread.rs diff --git a/issues/a-threads-segment-selectors-are-its-cpus.md b/issues/a-threads-segment-selectors-are-its-cpus.md index 2adb2bf6fad..a8ddb799e1f 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/fsgsbase-is-taken.md b/issues/fsgsbase-is-taken.md index f5241baadb6..444b0c7b3c0 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: 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 ae53045b6e0..86300b9b3c0 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 d3db4ff8c8e..f19121aa107 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 ce988731adf..793350d7e03 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 cb3f2c439b4..3bfb6e93fa7 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 e225af1a3dc..db312a2362c 100644 --- a/kernel/src/arch/x86_64/entry.rs +++ b/kernel/src/arch/x86_64/entry.rs @@ -13,12 +13,12 @@ //! other gate swaps when its frame's `CS` is Ring 3's. That decision is sound //! because the kernel's CPL-0 spans with a user's base live — before //! `SYSCALL`'s `swapgs`, and from an exit's `swapgs` to its `SYSRET` or -//! `IRETQ` — run with `IF` clear and no instruction that can fault. What can -//! still land there takes the kernel's base from `GDTR` whatever GS holds -//! ([`install_kernel_gs`]): NMI, `#MC`, `#DF`, and `#DB`, which a `MOV SS` -//! holds over the next instruction into the first one of whatever entry that -//! instruction reaches — for a data breakpoint on Intel (SDM Vol. 3A §7.8.3), -//! for single-step too on AMD (APM Vol. 2 §8.1.4). `#DB` relies on neither. +//! `IRETQ` — run with `IF` clear and no instruction that can fault, and no +//! `#DB` lands in them: nothing arms a debug register, `IA32_FMASK` clears +//! `TF`, and a single-step trap after `MOV SS` is suppressed, not deferred +//! (SDM Vol. 3A §7.8.3). NMI, `#MC` and `#DF` can land there, so their gates +//! take the kernel's base from `GDTR` whatever GS holds +//! ([`install_kernel_gs`]). /// `cld`, which every Ring 0 entry runs before it calls anything: no gate /// clears `DF`, and this kernel's forward-only `memmove`/`memcpy` write @@ -42,9 +42,8 @@ macro_rules! cld { /// `naked_asm!` for an interrupt or trap gate a Ring 3 thread can arrive /// through, `cs = N;` bytes below its frame's `CS` on entry: prepends `cld`, /// `clac`, and a `swapgs` when that `CS` is Ring 3's. The body must end with a -/// trailing comma, and swap back before every `iretq` that can return to -/// Ring 3: [`gate_iretq`] where the frame decides, a bare `swapgs` on a path -/// only a Ring 3 frame takes. +/// trailing comma, and every `iretq` in it that can return to Ring 3 is +/// [`gate_iretq`]'s. /// /// Such a gate clears neither `DF` nor `AC` (Intel SDM Vol. 3A §6.12.1.3 names /// `TF`, `VM`, `RF`, `NT`, and `IF` for an interrupt gate), and Ring 3 sets @@ -96,23 +95,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 9f5f8fc1e88..7b26e913c24 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 434b46e59d9..c4064cb5773 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 5bc05eff25d..a27395d6595 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 9fb53054b78..2686a40086a 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 56a3fd666fe..7dcfa6ae61a 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 b9d8423144c..64e2b7247c2 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; `u64::MAX`, which no base is, 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,7 @@ 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; 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 +414,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: u64::MAX, fpu_holder: 0, irq_counts: [const { AtomicU64::new(0) }; crate::irq_census::Source::COUNT], }, @@ -586,7 +586,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 +610,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); @@ -636,7 +627,6 @@ pub fn init_ap(percpu_ptr: *mut PerCpu) { let percpu = unsafe { &mut *percpu_ptr }; // 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); @@ -725,6 +715,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 7cab127cb87..36ff4acf0e1 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 99c531a987d..de5509e9d2d 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 00000000000..40229ef95b8 --- /dev/null +++ b/tests/guest/rust-tests/src/arch/fsgsbase.rs @@ -0,0 +1,38 @@ +//! 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 { + /// 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 00000000000..6fe1c066c2a --- /dev/null +++ b/tests/guest/rust-tests/src/bin/fsgs_bases_per_thread.rs @@ -0,0 +1,49 @@ +//! 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::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]; + +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"); + 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 +} From 55ed042ee3cf9b65927b1f7d492a56c2b7c4c074 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 11 Nov 2026 19:47:45 +0200 Subject: [PATCH 2/6] issues: an AP takes IST vectors before its TSS Found while moving the IST entries to `RDGSBASE`/`WRGSBASE`: an AP loads the IDT in its trampoline, and its TSS and `CR4` only in `init_ap`, so an IST vector in between has neither its stack nor, now, its instructions. Unchanged by this branch beyond the `#UD`, and recorded with its exit. Co-Authored-By: Claude Opus 5.5 --- .../an-ap-takes-ist-vectors-before-its-tss.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 issues/an-ap-takes-ist-vectors-before-its-tss.md 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 00000000000..bdc870c74e5 --- /dev/null +++ b/issues/an-ap-takes-ist-vectors-before-its-tss.md @@ -0,0 +1,19 @@ +--- +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`) and its declared `CR4` only later, in `percpu::init_ap`. Until +`ltr`, TR is still what INIT left, so an NMI, `#DF` or `#MC` in that span +takes its IST stack pointer from no TSS of the kernel's. Until `CR4` is +applied, those three entries' `rdgsbase` and `wrgsbase` (`arch::entry`) +would also be `#UD`. Either way such an event ends the machine with no +report. Found by reading while making those entries independent of GS. + +**Exit**: an AP holds its own GDT, TSS and declared `CR4` before it loads +the IDT. From 14bf8a2f7ed5a1abfafc0f99144eb55f6c521308 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 11 Nov 2026 20:46:12 +0200 Subject: [PATCH 3/6] x86-64: the switch reads a thread's bases only for a user thread, and an AP takes its CPU declaration before its TSS Review round 1 of #1264, on #1262's round. The rebase onto #1262 also gave its new `#DB` entry `RDGSBASE` and `WRGSBASE`, as the NMI's has. - The switch reads the outgoing FS and GS bases only for a thread of a process; the idle context's were never read back, and the read sat on the wake path. - An AP applies its `control_regs` declaration before `load_gdt`, so from the `ltr` that gives the IST entries their stacks they also have `CR4.FSGSBASE` for their instructions; the record of the earlier span, before `ltr`, says only what is left in it. - `fsgs_bases_per_thread` starts its rounds once both threads are pinned, so each thread running across the other's is the test's premise and not a race. Co-Authored-By: Claude Opus 5.5 --- issues/an-ap-takes-ist-vectors-before-its-tss.md | 13 +++++-------- kernel/src/arch/x86_64/percpu.rs | 4 +++- .../rust-tests/src/bin/fsgs_bases_per_thread.rs | 6 ++++++ 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/issues/an-ap-takes-ist-vectors-before-its-tss.md b/issues/an-ap-takes-ist-vectors-before-its-tss.md index bdc870c74e5..aaeca10ece9 100644 --- a/issues/an-ap-takes-ist-vectors-before-its-tss.md +++ b/issues/an-ap-takes-ist-vectors-before-its-tss.md @@ -8,12 +8,9 @@ opened: 2026-11-11 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`) and its declared `CR4` only later, in `percpu::init_ap`. Until -`ltr`, TR is still what INIT left, so an NMI, `#DF` or `#MC` in that span -takes its IST stack pointer from no TSS of the kernel's. Until `CR4` is -applied, those three entries' `rdgsbase` and `wrgsbase` (`arch::entry`) -would also be `#UD`. Either way such an event ends the machine with no -report. Found by reading while making those entries independent of GS. +(`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, TSS and declared `CR4` before it loads -the IDT. +**Exit**: an AP holds its own GDT and TSS before it loads the IDT. diff --git a/kernel/src/arch/x86_64/percpu.rs b/kernel/src/arch/x86_64/percpu.rs index 64e2b7247c2..db29acc4323 100644 --- a/kernel/src/arch/x86_64/percpu.rs +++ b/kernel/src/arch/x86_64/percpu.rs @@ -625,9 +625,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(); } - 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); 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 index 6fe1c066c2a..f961421a13b 100644 --- a/tests/guest/rust-tests/src/bin/fsgs_bases_per_thread.rs +++ b/tests/guest/rust-tests/src/bin/fsgs_bases_per_thread.rs @@ -5,6 +5,7 @@ //! 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; @@ -19,6 +20,10 @@ const ROUNDS: u64 = 500; /// 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); @@ -33,6 +38,7 @@ fn main() { /// 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(); let mut crossed = 0; for round in 0..ROUNDS { let fs = 0x0000_7000_0000_0000 | (me as u64) << 40 | round << 4; From 5e6d8fe43bdf58f9680769413cbff06539829092 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 11 Nov 2026 22:10:13 +0200 Subject: [PATCH 4/6] x86-64: entry.rs keeps #1262's account of #DB and of the gate contract, and the metal #1264 owes is recorded Review round 2 of #1264. The earlier rebase resolved `arch::entry`'s header and the gate macro's contract with this branch's text from before #1262's round 2, which said no `#DB` lands in the swapped spans and that every `iretq` to Ring 3 is `gate_iretq`'s. Both are #1262's again; this branch keeps only its `install_kernel_gs` in `WRGSBASE` form. The rebase onto restore. The metal rows #1264 owes, with the benches down, are recorded in `issues/fsgsbase-is-taken.md`. Co-Authored-By: Claude Opus 5.5 --- issues/fsgsbase-is-taken.md | 8 ++++++++ kernel/src/arch/x86_64/entry.rs | 17 +++++++++-------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/issues/fsgsbase-is-taken.md b/issues/fsgsbase-is-taken.md index 444b0c7b3c0..b356e60d069 100644 --- a/issues/fsgsbase-is-taken.md +++ b/issues/fsgsbase-is-taken.md @@ -53,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/kernel/src/arch/x86_64/entry.rs b/kernel/src/arch/x86_64/entry.rs index db312a2362c..4ccea977456 100644 --- a/kernel/src/arch/x86_64/entry.rs +++ b/kernel/src/arch/x86_64/entry.rs @@ -13,12 +13,12 @@ //! other gate swaps when its frame's `CS` is Ring 3's. That decision is sound //! because the kernel's CPL-0 spans with a user's base live — before //! `SYSCALL`'s `swapgs`, and from an exit's `swapgs` to its `SYSRET` or -//! `IRETQ` — run with `IF` clear and no instruction that can fault, and no -//! `#DB` lands in them: nothing arms a debug register, `IA32_FMASK` clears -//! `TF`, and a single-step trap after `MOV SS` is suppressed, not deferred -//! (SDM Vol. 3A §7.8.3). NMI, `#MC` and `#DF` can land there, so their gates -//! take the kernel's base from `GDTR` whatever GS holds -//! ([`install_kernel_gs`]). +//! `IRETQ` — run with `IF` clear and no instruction that can fault. What can +//! still land there takes the kernel's base from `GDTR` whatever GS holds +//! ([`install_kernel_gs`]): NMI, `#MC`, `#DF`, and `#DB`, which a `MOV SS` +//! holds over the next instruction into the first one of whatever entry that +//! instruction reaches — for a data breakpoint on Intel (SDM Vol. 3A §7.8.3), +//! for single-step too on AMD (APM Vol. 2 §8.1.4). `#DB` relies on neither. /// `cld`, which every Ring 0 entry runs before it calls anything: no gate /// clears `DF`, and this kernel's forward-only `memmove`/`memcpy` write @@ -42,8 +42,9 @@ macro_rules! cld { /// `naked_asm!` for an interrupt or trap gate a Ring 3 thread can arrive /// through, `cs = N;` bytes below its frame's `CS` on entry: prepends `cld`, /// `clac`, and a `swapgs` when that `CS` is Ring 3's. The body must end with a -/// trailing comma, and every `iretq` in it that can return to Ring 3 is -/// [`gate_iretq`]'s. +/// trailing comma, and swap back before every `iretq` that can return to +/// Ring 3: [`gate_iretq`] where the frame decides, a bare `swapgs` on a path +/// only a Ring 3 frame takes. /// /// Such a gate clears neither `DF` nor `AC` (Intel SDM Vol. 3A §6.12.1.3 names /// `TF`, `VM`, `RF`, `NT`, and `IF` for an interrupt gate), and Ring 3 sets From a671bdb5a504af767a6e49375ffc9eef0accd879 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 12 Nov 2026 04:16:33 +0200 Subject: [PATCH 5/6] issues: the GS-base defect's record says what #1262 built and what it owes on metal #1251 brought the record to main, and #1262 then fixed the defect without touching it, so main's record still read as an open, unfixed hole. It now says what #1262 built, how the exit's guest test and its negative control came out under QEMU, and what the benches still owe before it closes. Co-Authored-By: Claude Opus 5.5 --- ...s-the-kernels-gs-base-with-a-segment-load.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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 aaff2353fd0..eec79304c2d 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,6 +31,20 @@ 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 From a72a3d82b3c297b04b98f8303ec71f30369befea Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 12 Nov 2026 04:36:07 +0200 Subject: [PATCH 6/6] x86-64: the per-CPU GS record starts at a base no thread can hold, and the readback test checks a thread's first base Review of #1264's rebase. `percpu::user_gs_base` started at `u64::MAX`, which is canonical, so a thread that had written that base and was first switched in on a fresh CPU kept whatever `IA32_KERNEL_GS_BASE` held there: nothing sets that register at boot since `clear_user_gs_base` went. The record now starts at `1 << 63`, which is not canonical; `WRGSBASE` and `WRMSR` refuse it, so no thread holds it, and a CPU's first switch into any thread writes that thread's base. `fsgs_bases_per_thread` reads each thread's GS base once after both are pinned and before either writes one, and asserts 0. A switch-out that failed to refresh the record could otherwise leave a thread that had not yet written its base on another thread's, and the rounds after would not see it. The GS-base defect's record drops a sentence about the fix being the owner's to choose, which its new lead had already answered. Co-Authored-By: Claude Opus 5.5 --- ...laces-the-kernels-gs-base-with-a-segment-load.md | 6 ++---- kernel/src/arch/x86_64/percpu.rs | 13 +++++++++---- tests/guest/rust-tests/src/arch/fsgsbase.rs | 8 ++++++++ .../rust-tests/src/bin/fsgs_bases_per_thread.rs | 4 ++++ 4 files changed, 23 insertions(+), 8 deletions(-) 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 eec79304c2d..51fed904426 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 @@ -48,7 +48,5 @@ 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/kernel/src/arch/x86_64/percpu.rs b/kernel/src/arch/x86_64/percpu.rs index db29acc4323..6ebade5d0e9 100644 --- a/kernel/src/arch/x86_64/percpu.rs +++ b/kernel/src/arch/x86_64/percpu.rs @@ -132,9 +132,9 @@ pub struct PerCpu { /// 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; `u64::MAX`, which no base is, 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`. + /// 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. @@ -243,6 +243,11 @@ 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; @@ -414,7 +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: u64::MAX, + user_gs_base: NO_GS_BASE, fpu_holder: 0, irq_counts: [const { AtomicU64::new(0) }; crate::irq_census::Source::COUNT], }, diff --git a/tests/guest/rust-tests/src/arch/fsgsbase.rs b/tests/guest/rust-tests/src/arch/fsgsbase.rs index 40229ef95b8..6fc88109e3a 100644 --- a/tests/guest/rust-tests/src/arch/fsgsbase.rs +++ b/tests/guest/rust-tests/src/arch/fsgsbase.rs @@ -5,6 +5,14 @@ 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. 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 index f961421a13b..9480717bc3a 100644 --- a/tests/guest/rust-tests/src/bin/fsgs_bases_per_thread.rs +++ b/tests/guest/rust-tests/src/bin/fsgs_bases_per_thread.rs @@ -39,6 +39,10 @@ fn main() { 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;