Skip to content

Commit

Permalink
cpu/idt: prevent system call from being invoked as an external interrupt
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
msft-jlange committed Oct 23, 2024
1 parent 0f8cacb commit 215cf4b
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 6 deletions.
18 changes: 18 additions & 0 deletions kernel/src/cpu/idt/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
}
1 change: 1 addition & 0 deletions kernel/src/cpu/idt/entry.S
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 15 additions & 6 deletions kernel/src/cpu/idt/svsm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!");
}
Expand Down Expand Up @@ -275,4 +284,4 @@ global_asm!(
include_str!("entry.S"),
IF = const RFlags::IF.bits(),
options(att_syntax)
);
);
4 changes: 4 additions & 0 deletions kernel/src/platform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
}
Expand Down
7 changes: 7 additions & 0 deletions kernel/src/platform/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!();
}
Expand Down
9 changes: 9 additions & 0 deletions kernel/src/platform/snp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand Down
7 changes: 7 additions & 0 deletions kernel/src/platform/tdp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!();
}
Expand Down

0 comments on commit 215cf4b

Please sign in to comment.