From 155d01b11e43a1bebeee7bbc54a15835c684ddfa Mon Sep 17 00:00:00 2001 From: vasilito Date: Thu, 16 Jul 2026 12:11:34 +0900 Subject: [PATCH] kernel(x86_64): make /scheme/sys/msr MSR access #GP-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MSR R/W scheme lets root request an arbitrary wrmsr/rdmsr on any CPU (used by cpufreqd/thermald). A bad MSR number or reserved-bit value raises #GP; on the raw wrmsr/rdmsr that is a kernel-mode fault that panics the whole machine at exit_this_context (unreachable!) — i.e. root userspace can crash the kernel. Observed on KVM: cpufreqd writing a legacy P-state MSR (#GP in the msr IPI handler) halted the boot right before the console/login. Add wrmsr_safe/rdmsr_safe (arch/x86_64): the faulting instruction sits in a __wrmsr_safe_start/end (resp. rdmsr) region, and the #GP handler recognises a fault inside those bounds and returns an error via recover_and_efault — the same fault-recovery mechanism already used for usercopy page faults. The MSR scheme (local path) and the cross-CPU msr IPI handler now use these and surface EIO to the caller (IPI failures propagated via a new MsrMailbox faulted flag) instead of panicking. 32-bit x86 keeps the raw path (untested). --- src/arch/x86_64/mod.rs | 52 ++++++++++++++++++++++ src/arch/x86_shared/interrupt/exception.rs | 19 ++++++++ src/arch/x86_shared/interrupt/ipi.rs | 30 ++++++++++--- src/main.rs | 11 +++++ src/percpu.rs | 13 +++++- src/scheme/sys/msr.rs | 49 ++++++++++++++++---- 6 files changed, 158 insertions(+), 16 deletions(-) diff --git a/src/arch/x86_64/mod.rs b/src/arch/x86_64/mod.rs index 513970014a..68d61a3668 100644 --- a/src/arch/x86_64/mod.rs +++ b/src/arch/x86_64/mod.rs @@ -47,4 +47,56 @@ pub unsafe extern "C" fn arch_copy_to_user(dst: usize, src: usize, len: usize) - } pub use arch_copy_to_user as arch_copy_from_user; +/// Write an MSR, recovering from a #GP fault instead of panicking the kernel. +/// +/// The `/scheme/sys/msr` interface lets root request an arbitrary MSR write on +/// any CPU (used by cpufreqd/thermald). A bad MSR number or reserved-bit value +/// raises #GP; if that fires on the raw `wrmsr` it is a *kernel-mode* fault +/// which is otherwise unrecoverable and panics the whole machine — i.e. root +/// userspace can crash the kernel. Mirror the usercopy fault-recovery scheme: +/// the `wrmsr` instruction is wrapped in the `__wrmsr_safe_start/end` region; +/// the #GP handler (arch/x86_shared/interrupt/exception.rs) recognises a fault +/// inside that region and returns here with rax=1 via `recover_and_efault`. +/// +/// Returns 0 on success, 1 if the write faulted. +#[unsafe(naked)] +pub unsafe extern "C" fn wrmsr_safe(msr: u32, value: u64) -> u8 { + // SysV: msr=edi, value=rsi, ret=al. wrmsr wants ecx=msr, edx:eax=value. + core::arch::naked_asm!( + ".global __wrmsr_safe_start + __wrmsr_safe_start:", + "mov ecx, edi", + "mov eax, esi", + "mov rdx, rsi", + "shr rdx, 32", + "wrmsr", + "xor eax, eax", + "ret", + ".global __wrmsr_safe_end + __wrmsr_safe_end:" + ); +} + +/// Read an MSR, recovering from a #GP fault instead of panicking the kernel. +/// On success writes the value through `out` and returns 0; on #GP returns 1 +/// and leaves `out` untouched. See [`wrmsr_safe`] for the recovery mechanism. +#[unsafe(naked)] +pub unsafe extern "C" fn rdmsr_safe(msr: u32, out: *mut u64) -> u8 { + // SysV: msr=edi, out=rsi, ret=al. rdmsr wants ecx=msr, yields edx:eax. + core::arch::naked_asm!( + ".global __rdmsr_safe_start + __rdmsr_safe_start:", + "mov ecx, edi", + "rdmsr", + "shl rdx, 32", + "mov eax, eax", + "or rax, rdx", + "mov [rsi], rax", + "xor eax, eax", + "ret", + ".global __rdmsr_safe_end + __rdmsr_safe_end:" + ); +} + pub use alternative::kfx_size; diff --git a/src/arch/x86_shared/interrupt/exception.rs b/src/arch/x86_shared/interrupt/exception.rs index 7725a45d0a..41d5a2ff3a 100644 --- a/src/arch/x86_shared/interrupt/exception.rs +++ b/src/arch/x86_shared/interrupt/exception.rs @@ -171,6 +171,25 @@ interrupt_error!(stack_segment, |stack, code| { }); interrupt_error!(protection, |stack, code| { + // A #GP raised inside the safe MSR accessors (wrmsr_safe / rdmsr_safe) + // means root asked /scheme/sys/msr for a bad MSR access (e.g. cpufreqd + // writing a P-state MSR unsupported under KVM). Recover with an error + // return instead of letting a kernel-mode #GP panic the whole machine — + // the same fault-recovery mechanism used for usercopy page faults. + #[cfg(target_arch = "x86_64")] + { + use crate::kernel_executable_offsets::{ + __rdmsr_safe_end, __rdmsr_safe_start, __wrmsr_safe_end, __wrmsr_safe_start, + }; + use crate::memory::ArchIntCtx; + let ip = stack.ip(); + if (__wrmsr_safe_start()..__wrmsr_safe_end()).contains(&ip) + || (__rdmsr_safe_start()..__rdmsr_safe_end()).contains(&ip) + { + stack.recover_and_efault(); + return; + } + } println!("Protection fault code={:#0x}", code); stack.trace(); excp_handler(Exception { diff --git a/src/arch/x86_shared/interrupt/ipi.rs b/src/arch/x86_shared/interrupt/ipi.rs index 9e4c129e97..f06134cfd8 100644 --- a/src/arch/x86_shared/interrupt/ipi.rs +++ b/src/arch/x86_shared/interrupt/ipi.rs @@ -1,6 +1,9 @@ use crate::{ arch::device::local_apic::the_local_apic, context, percpu::PercpuBlock, sync::CleanLockToken, }; +// Only the 32-bit x86 path uses the unguarded intrinsics; x86_64 goes through +// the #GP-recoverable wrmsr_safe/rdmsr_safe so a bad MSR cannot panic the kernel. +#[cfg(not(target_arch = "x86_64"))] use x86::msr::{rdmsr, wrmsr}; interrupt!(wakeup, || { @@ -33,14 +36,31 @@ interrupt!(msr, || { let mailbox = percpu.msr_mailbox(); let op = mailbox.request(); - let result = if op.is_write { - unsafe { wrmsr(op.msr as u32, op.value) }; - op.value + let (result, faulted) = if op.is_write { + #[cfg(target_arch = "x86_64")] + { + let f = unsafe { crate::arch::wrmsr_safe(op.msr as u32, op.value) } != 0; + (op.value, f) + } + #[cfg(not(target_arch = "x86_64"))] + { + unsafe { wrmsr(op.msr as u32, op.value) }; + (op.value, false) + } } else { - unsafe { rdmsr(op.msr as u32) } + #[cfg(target_arch = "x86_64")] + { + let mut v = 0u64; + let f = unsafe { crate::arch::rdmsr_safe(op.msr as u32, &mut v) } != 0; + (v, f) + } + #[cfg(not(target_arch = "x86_64"))] + { + (unsafe { rdmsr(op.msr as u32) }, false) + } }; - mailbox.finish_handle(result); + mailbox.finish_handle(result, faulted); unsafe { the_local_apic().eoi() }; }); diff --git a/src/main.rs b/src/main.rs index 8380e02f94..ac4b969c73 100644 --- a/src/main.rs +++ b/src/main.rs @@ -143,4 +143,15 @@ mod kernel_executable_offsets { #[cfg(target_arch = "x86_64")] linker_offsets!(__altrelocs_start, __altrelocs_end); + + // Fault-recovery regions for the safe MSR accessors (wrmsr_safe/rdmsr_safe). + // A #GP inside these bounds is turned into an error return instead of a + // kernel panic — see arch/x86_shared/interrupt/exception.rs. + #[cfg(target_arch = "x86_64")] + linker_offsets!( + __wrmsr_safe_start, + __wrmsr_safe_end, + __rdmsr_safe_start, + __rdmsr_safe_end + ); } diff --git a/src/percpu.rs b/src/percpu.rs index 1c0874f226..4ffa4dc1aa 100644 --- a/src/percpu.rs +++ b/src/percpu.rs @@ -62,6 +62,10 @@ pub struct MsrMailbox { value: AtomicU64, kind: AtomicU8, done: AtomicBool, + // Set by the remote handler when the wrmsr/rdmsr #GP-faulted (bad MSR). + // Published together with `done`, so a reader that observes done==true via + // Acquire also observes the correct `faulted` value. + faulted: AtomicBool, } impl MsrMailbox { @@ -72,11 +76,13 @@ impl MsrMailbox { value: AtomicU64::new(0), kind: AtomicU8::new(0), done: AtomicBool::new(true), + faulted: AtomicBool::new(false), } } pub fn begin_request(&self, cpu_id: u32, msr: u32, is_write: bool, value: u64) { self.done.store(false, Ordering::Relaxed); + self.faulted.store(false, Ordering::Relaxed); self.cpu_id.store(cpu_id, Ordering::Relaxed); self.msr.store(msr as u64, Ordering::Relaxed); self.value.store(value, Ordering::Relaxed); @@ -95,12 +101,15 @@ impl MsrMailbox { } } - pub fn finish_handle(&self, value: u64) { - self.value.store(value, Ordering::Release); + pub fn finish_handle(&self, value: u64, faulted: bool) { + self.value.store(value, Ordering::Relaxed); + self.faulted.store(faulted, Ordering::Relaxed); self.done.store(true, Ordering::Release); } pub fn read_value(&self) -> u64 { self.value.load(Ordering::Acquire) } + + pub fn faulted(&self) -> bool { self.faulted.load(Ordering::Acquire) } } pub struct MsrRequest { diff --git a/src/scheme/sys/msr.rs b/src/scheme/sys/msr.rs index 94e48316b4..574f3af870 100644 --- a/src/scheme/sys/msr.rs +++ b/src/scheme/sys/msr.rs @@ -11,13 +11,17 @@ //! `scheme:msr` interface for ring-3 access, but this kernel-side //! helper is for the scheme to forward requests to the active CPU). //! +// The 32-bit x86 path still uses the raw intrinsics; x86_64 uses the +// #GP-recoverable wrmsr_safe/rdmsr_safe so a bad MSR write from root cannot +// panic the kernel — it returns EIO instead. +#[cfg(not(target_arch = "x86_64"))] use x86::msr::{rdmsr, wrmsr}; use crate::cpu_count; use crate::ipi::{ipi_single, IpiKind}; use crate::percpu::PercpuBlock; use syscall::{ - error::{Error, Result, EBUSY, EBADF, EINVAL, ENOENT, EPERM, ETIMEDOUT}, + error::{Error, Result, EBUSY, EBADF, EINVAL, EIO, ENOENT, EPERM, ETIMEDOUT}, }; use crate::scheme::CallerCtx; use crate::sync::CleanLockToken; @@ -79,14 +83,7 @@ pub fn write(handle: u64, buf: UserSliceRo, _token: &mut CleanLockToken) -> Resu fn access_msr(cpu: u32, msr: u32, write: Option) -> Result { let current_cpu = PercpuBlock::current().cpu_id.get(); if cpu == current_cpu { - return unsafe { - if let Some(value) = write { - wrmsr(msr, value); - Ok(value) - } else { - Ok(rdmsr(msr)) - } - }; + return access_msr_local(msr, write); } let Some(target) = crate::percpu::get_percpu_block(crate::cpu_set::LogicalCpuId::new(cpu)) @@ -104,6 +101,11 @@ fn access_msr(cpu: u32, msr: u32, write: Option) -> Result { for _ in 0..MSR_TIMEOUT_SPINS { if mailbox.wait_done() { + // The remote CPU may have taken a #GP on a bad MSR; that is now + // recovered (no kernel panic) and reported via the mailbox. + if mailbox.faulted() { + return Err(Error::new(EIO)); + } let value = mailbox.read_value(); return Ok(value); } @@ -112,3 +114,32 @@ fn access_msr(cpu: u32, msr: u32, write: Option) -> Result { Err(Error::new(ETIMEDOUT)) } + +/// Read or write an MSR on the current CPU. On x86_64 a #GP (bad MSR / value) +/// is recovered into an EIO error instead of panicking the kernel. +fn access_msr_local(msr: u32, write: Option) -> Result { + #[cfg(target_arch = "x86_64")] + unsafe { + if let Some(value) = write { + if crate::arch::wrmsr_safe(msr, value) != 0 { + return Err(Error::new(EIO)); + } + Ok(value) + } else { + let mut out = 0u64; + if crate::arch::rdmsr_safe(msr, &mut out) != 0 { + return Err(Error::new(EIO)); + } + Ok(out) + } + } + #[cfg(not(target_arch = "x86_64"))] + unsafe { + if let Some(value) = write { + wrmsr(msr, value); + Ok(value) + } else { + Ok(rdmsr(msr)) + } + } +}