From 215cf4b79081425816a33dc6739185e90679ec8b Mon Sep 17 00:00:00 2001 From: Jon Lange Date: Sat, 12 Oct 2024 09:26:21 -0700 Subject: [PATCH] cpu/idt: prevent system call from being invoked as an external interrupt Some platforms permit the injection of arbitrary interrupt vectors. System calls from user mode are implemented as software interrupts, and handling an injected interrupt as a system call would be a security issue. The system call handler must determine whether it was invoked in an unsafe way. Signed-off-by: Jon Lange --- kernel/src/cpu/idt/common.rs | 18 ++++++++++++++++++ kernel/src/cpu/idt/entry.S | 1 + kernel/src/cpu/idt/svsm.rs | 21 +++++++++++++++------ kernel/src/platform/mod.rs | 4 ++++ kernel/src/platform/native.rs | 7 +++++++ kernel/src/platform/snp.rs | 9 +++++++++ kernel/src/platform/tdp.rs | 7 +++++++ 7 files changed, 61 insertions(+), 6 deletions(-) diff --git a/kernel/src/cpu/idt/common.rs b/kernel/src/cpu/idt/common.rs index a6cc1d655..3a1754060 100644 --- a/kernel/src/cpu/idt/common.rs +++ b/kernel/src/cpu/idt/common.rs @@ -401,3 +401,21 @@ global_asm!( "#, options(att_syntax) ); + +#[repr(u32)] +#[derive(Debug, Clone, Copy)] +pub enum IdtEventType { + Unknown = 0, + External, + Software, +} + +impl IdtEventType { + pub fn is_external_interrupt(&self, vector: usize) -> bool { + match self { + Self::External => true, + Self::Software => false, + Self::Unknown => SVSM_PLATFORM.as_dyn_ref().is_external_interrupt(vector), + } + } +} diff --git a/kernel/src/cpu/idt/entry.S b/kernel/src/cpu/idt/entry.S index 5888618e9..cfd04c37f 100644 --- a/kernel/src/cpu/idt/entry.S +++ b/kernel/src/cpu/idt/entry.S @@ -42,6 +42,7 @@ asm_entry_\name: push_regs movl $\vector, %esi movq %rsp, %rdi + xorl %edx, %edx call ex_handler_\handler jmp default_return .endm diff --git a/kernel/src/cpu/idt/svsm.rs b/kernel/src/cpu/idt/svsm.rs index 2d23eeace..bcce012bb 100644 --- a/kernel/src/cpu/idt/svsm.rs +++ b/kernel/src/cpu/idt/svsm.rs @@ -10,10 +10,10 @@ use super::super::percpu::{current_task, this_cpu}; use super::super::tss::IST_DF; use super::super::vc::handle_vc_exception; use super::common::{ - idt_mut, user_mode, IdtEntry, PageFaultError, AC_VECTOR, BP_VECTOR, BR_VECTOR, CP_VECTOR, - DB_VECTOR, DE_VECTOR, DF_VECTOR, GP_VECTOR, HV_VECTOR, INT_INJ_VECTOR, MCE_VECTOR, MF_VECTOR, - NMI_VECTOR, NM_VECTOR, NP_VECTOR, OF_VECTOR, PF_VECTOR, SS_VECTOR, SX_VECTOR, TS_VECTOR, - UD_VECTOR, VC_VECTOR, XF_VECTOR, + idt_mut, user_mode, IdtEntry, IdtEventType, PageFaultError, AC_VECTOR, BP_VECTOR, BR_VECTOR, + CP_VECTOR, DB_VECTOR, DE_VECTOR, DF_VECTOR, GP_VECTOR, HV_VECTOR, INT_INJ_VECTOR, MCE_VECTOR, + MF_VECTOR, NMI_VECTOR, NM_VECTOR, NP_VECTOR, OF_VECTOR, PF_VECTOR, SS_VECTOR, SX_VECTOR, + TS_VECTOR, UD_VECTOR, VC_VECTOR, XF_VECTOR, }; use crate::address::VirtAddr; use crate::cpu::registers::RFlags; @@ -227,7 +227,16 @@ extern "C" fn ex_handler_vmm_communication(ctxt: &mut X86ExceptionContext, vecto // System Call SoftIRQ handler #[no_mangle] -extern "C" fn ex_handler_system_call(ctxt: &mut X86ExceptionContext) { +extern "C" fn ex_handler_system_call( + ctxt: &mut X86ExceptionContext, + vector: usize, + event_type: IdtEventType, +) { + // Ensure that this vector was not invoked as a hardware interrupt vector. + if event_type.is_external_interrupt(vector) { + panic!("Syscall handler invoked as external interrupt!"); + } + if !user_mode(ctxt) { panic!("Syscall handler called from kernel mode!"); } @@ -275,4 +284,4 @@ global_asm!( include_str!("entry.S"), IF = const RFlags::IF.bits(), options(att_syntax) -); \ No newline at end of file +); diff --git a/kernel/src/platform/mod.rs b/kernel/src/platform/mod.rs index 00e569bc2..0d7eaad81 100644 --- a/kernel/src/platform/mod.rs +++ b/kernel/src/platform/mod.rs @@ -124,6 +124,10 @@ pub trait SvsmPlatform { /// Perform an EOI of the current interrupt. fn eoi(&self); + /// Determines whether a given interrupt vector was invoked as an external + /// interrupt. + fn is_external_interrupt(&self, vector: usize) -> bool; + /// Start an additional processor. fn start_cpu(&self, cpu: &PerCpu, start_rip: u64) -> Result<(), SvsmError>; } diff --git a/kernel/src/platform/native.rs b/kernel/src/platform/native.rs index ebcad9b2b..17f3d4421 100644 --- a/kernel/src/platform/native.rs +++ b/kernel/src/platform/native.rs @@ -141,6 +141,13 @@ impl SvsmPlatform for NativePlatform { todo!(); } + fn is_external_interrupt(&self, _vector: usize) -> bool { + // For a native platform, the hypervisor is fully trusted with all + // event delivery, so all events are assumed not to be external + // interrupts. + false + } + fn start_cpu(&self, _cpu: &PerCpu, _start_rip: u64) -> Result<(), SvsmError> { todo!(); } diff --git a/kernel/src/platform/snp.rs b/kernel/src/platform/snp.rs index 728fbbcf6..ad0377649 100644 --- a/kernel/src/platform/snp.rs +++ b/kernel/src/platform/snp.rs @@ -286,6 +286,15 @@ impl SvsmPlatform for SnpPlatform { } } + fn is_external_interrupt(&self, _vector: usize) -> bool { + // When restricted injection is active, the event disposition is + // already known to the caller and thus need not be examined. When + // restricted injection is not active, the hypervisor must be trusted + // with all event delivery, so all events are assumed not to be + // external interrupts. + false + } + fn start_cpu(&self, cpu: &PerCpu, start_rip: u64) -> Result<(), SvsmError> { let (vmsa_pa, sev_features) = cpu.alloc_svsm_vmsa(*VTOM as u64, start_rip)?; diff --git a/kernel/src/platform/tdp.rs b/kernel/src/platform/tdp.rs index 8a6676ab1..d0e9c0873 100644 --- a/kernel/src/platform/tdp.rs +++ b/kernel/src/platform/tdp.rs @@ -154,6 +154,13 @@ impl SvsmPlatform for TdpPlatform { fn eoi(&self) {} + fn is_external_interrupt(&self, _vector: usize) -> bool { + // Examine the APIC ISR to determine whether this interrupt vector is + // active. If so, it is assumed to be an external interrupt. + // TODO - add code to read the APIC ISR. + todo!(); + } + fn start_cpu(&self, _cpu: &PerCpu, _start_rip: u64) -> Result<(), SvsmError> { todo!(); }