Skip to content

Commit a84fa74

Browse files
committed
Disallow MSR read/write on KVM
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
1 parent 84a0553 commit a84fa74

File tree

10 files changed

+270
-11
lines changed

10 files changed

+270
-11
lines changed

Cargo.lock

Lines changed: 2 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/hyperlight_host/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ windows-version = "0.1"
7575
lazy_static = "1.4.0"
7676

7777
[target.'cfg(unix)'.dependencies]
78-
kvm-bindings = { version = "0.14", features = ["fam-wrappers"], optional = true }
79-
kvm-ioctls = { version = "0.24", optional = true }
78+
kvm-bindings = { git = "https://github.com/rust-vmm/kvm", rev = "3ffc9b62af5978553f73cc0ec79fad13fdd47146", features = ["fam-wrappers"], optional = true }
79+
kvm-ioctls = { git = "https://github.com/rust-vmm/kvm", rev = "3ffc9b62af5978553f73cc0ec79fad13fdd47146", optional = true }
8080
mshv-bindings = { version = "0.6", optional = true }
8181
mshv-ioctls = { version = "0.6", optional = true}
8282

src/hyperlight_host/src/error.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,16 @@ pub enum HyperlightError {
158158
#[error("Memory Access Violation at address {0:#x} of type {1}, but memory is marked as {2}")]
159159
MemoryAccessViolation(u64, MemoryRegionFlags, MemoryRegionFlags),
160160

161+
/// MSR Read Violation. Guest attempted to read from a Model-Specific Register
162+
#[cfg(all(kvm, target_arch = "x86_64"))]
163+
#[error("Guest attempted to read from MSR {0:#x}")]
164+
MsrReadViolation(u32),
165+
166+
/// MSR Write Violation. Guest attempted to write to a Model-Specific Register
167+
#[cfg(all(kvm, target_arch = "x86_64"))]
168+
#[error("Guest attempted to write {1:#x} to MSR {0:#x}")]
169+
MsrWriteViolation(u32, u64),
170+
161171
/// Memory Allocation Failed.
162172
#[error("Memory Allocation Failed with OS Error {0:?}.")]
163173
MemoryAllocationFailed(Option<i32>),
@@ -338,8 +348,13 @@ impl HyperlightError {
338348
| HyperlightError::ExecutionCanceledByHost()
339349
| HyperlightError::PoisonedSandbox
340350
| HyperlightError::ExecutionAccessViolation(_)
341-
| HyperlightError::MemoryAccessViolation(_, _, _)
342-
| HyperlightError::SnapshotSizeMismatch(_, _)
351+
| HyperlightError::MemoryAccessViolation(_, _, _) => true,
352+
353+
#[cfg(all(kvm, target_arch = "x86_64"))]
354+
HyperlightError::MsrReadViolation(_)
355+
| HyperlightError::MsrWriteViolation(_, _) => true,
356+
357+
HyperlightError::SnapshotSizeMismatch(_, _)
343358
| HyperlightError::MemoryRegionSizeMismatch(_, _, _)
344359
// HyperlightVmError::Restore is already handled manually in restore(), but we mark it
345360
// as poisoning here too for defense in depth.

src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,16 @@ impl DispatchGuestCallError {
165165
region_flags,
166166
}) => HyperlightError::MemoryAccessViolation(addr, access_type, region_flags),
167167

168+
#[cfg(all(kvm, target_arch = "x86_64"))]
169+
DispatchGuestCallError::Run(RunVmError::MsrReadViolation(msr_index)) => {
170+
HyperlightError::MsrReadViolation(msr_index)
171+
}
172+
173+
#[cfg(all(kvm, target_arch = "x86_64"))]
174+
DispatchGuestCallError::Run(RunVmError::MsrWriteViolation { msr_index, value }) => {
175+
HyperlightError::MsrWriteViolation(msr_index, value)
176+
}
177+
168178
// Leave others as is
169179
other => HyperlightVmError::DispatchGuestCall(other).into(),
170180
};
@@ -215,6 +225,12 @@ pub enum RunVmError {
215225
MmioReadUnmapped(u64),
216226
#[error("MMIO WRITE access to unmapped address {0:#x}")]
217227
MmioWriteUnmapped(u64),
228+
#[cfg(all(kvm, target_arch = "x86_64"))]
229+
#[error("Guest attempted to read from MSR {0:#x}")]
230+
MsrReadViolation(u32),
231+
#[cfg(all(kvm, target_arch = "x86_64"))]
232+
#[error("Guest attempted to write {value:#x} to MSR {msr_index:#x}")]
233+
MsrWriteViolation { msr_index: u32, value: u64 },
218234
#[error("vCPU run failed: {0}")]
219235
RunVcpu(#[from] RunVcpuError),
220236
#[error("Unexpected VM exit: {0}")]
@@ -658,6 +674,14 @@ impl HyperlightVm {
658674
}
659675
}
660676
}
677+
#[cfg(all(kvm, target_arch = "x86_64"))]
678+
Ok(VmExit::MsrRead(msr_index)) => {
679+
break Err(RunVmError::MsrReadViolation(msr_index));
680+
}
681+
#[cfg(all(kvm, target_arch = "x86_64"))]
682+
Ok(VmExit::MsrWrite { msr_index, value }) => {
683+
break Err(RunVmError::MsrWriteViolation { msr_index, value });
684+
}
661685
Ok(VmExit::Cancelled()) => {
662686
// If cancellation was not requested for this specific guest function call,
663687
// the vcpu was interrupted by a stale cancellation. This can occur when:

src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,14 @@ impl HyperlightVm {
9191

9292
let vm: VmType = match get_available_hypervisor() {
9393
#[cfg(kvm)]
94-
Some(HypervisorType::Kvm) => Box::new(KvmVm::new().map_err(VmError::CreateVm)?),
94+
Some(HypervisorType::Kvm) => {
95+
let kvm_vm = KvmVm::new().map_err(VmError::CreateVm)?;
96+
#[cfg(target_arch = "x86_64")]
97+
if !config.get_allow_msr() {
98+
kvm_vm.enable_msr_filter().map_err(VmError::CreateVm)?;
99+
}
100+
Box::new(kvm_vm)
101+
}
95102
#[cfg(mshv3)]
96103
Some(HypervisorType::Mshv) => Box::new(MshvVm::new().map_err(VmError::CreateVm)?),
97104
#[cfg(target_os = "windows")]

src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,14 @@ use std::sync::LazyLock;
1919
#[cfg(gdb)]
2020
use kvm_bindings::kvm_guest_debug;
2121
use kvm_bindings::{
22-
kvm_debugregs, kvm_fpu, kvm_regs, kvm_sregs, kvm_userspace_memory_region, kvm_xsave,
22+
kvm_debugregs, kvm_enable_cap, kvm_fpu, kvm_regs, kvm_sregs, kvm_userspace_memory_region,
23+
kvm_xsave,
2324
};
2425
use kvm_ioctls::Cap::UserMemory;
25-
use kvm_ioctls::{Kvm, VcpuExit, VcpuFd, VmFd};
26+
use kvm_ioctls::{
27+
Cap, Kvm, MsrExitReason, MsrFilterDefaultAction, MsrFilterRange, MsrFilterRangeFlags, VcpuExit,
28+
VcpuFd, VmFd,
29+
};
2630
use tracing::{Span, instrument};
2731
#[cfg(feature = "trace_guest")]
2832
use tracing_opentelemetry::OpenTelemetrySpanExt;
@@ -136,6 +140,44 @@ impl KvmVm {
136140
debug_regs: kvm_guest_debug::default(),
137141
})
138142
}
143+
144+
/// Enable MSR filtering: tell KVM to exit to userspace on filtered MSR
145+
/// access, then install a deny-all filter so every RDMSR/WRMSR traps.
146+
///
147+
/// Requires KVM_CAP_X86_USER_SPACE_MSR and KVM_CAP_X86_MSR_FILTER
148+
pub(crate) fn enable_msr_filter(&self) -> std::result::Result<(), CreateVmError> {
149+
let hv = KVM.as_ref().map_err(|e| e.clone())?;
150+
if !hv.check_extension(Cap::X86UserSpaceMsr) || !hv.check_extension(Cap::X86MsrFilter) {
151+
tracing::error!(
152+
"KVM does not support KVM_CAP_X86_USER_SPACE_MSR or KVM_CAP_X86_MSR_FILTER."
153+
);
154+
return Err(CreateVmError::MsrFilterNotSupported);
155+
}
156+
157+
let cap = kvm_enable_cap {
158+
cap: Cap::X86UserSpaceMsr as u32,
159+
args: [MsrExitReason::Filter.bits() as u64, 0, 0, 0],
160+
..Default::default()
161+
};
162+
self.vm_fd
163+
.enable_cap(&cap)
164+
.map_err(|e| CreateVmError::InitializeVm(e.into()))?;
165+
166+
// At least one range is required when using KVM_MSR_FILTER_DEFAULT_DENY.
167+
let bitmap = [0u8; 1]; // 1 byte covers 8 MSRs, all bits 0 (deny)
168+
self.vm_fd
169+
.set_msr_filter(
170+
MsrFilterDefaultAction::DENY,
171+
&[MsrFilterRange {
172+
flags: MsrFilterRangeFlags::READ | MsrFilterRangeFlags::WRITE,
173+
base: 0,
174+
msr_count: 1,
175+
bitmap: &bitmap,
176+
}],
177+
)
178+
.map_err(|e| CreateVmError::InitializeVm(e.into()))?;
179+
Ok(())
180+
}
139181
}
140182

141183
impl VirtualMachine for KvmVm {
@@ -176,6 +218,40 @@ impl VirtualMachine for KvmVm {
176218
Ok(VcpuExit::IoOut(port, data)) => Ok(VmExit::IoOut(port, data.to_vec())),
177219
Ok(VcpuExit::MmioRead(addr, _)) => Ok(VmExit::MmioRead(addr)),
178220
Ok(VcpuExit::MmioWrite(addr, _)) => Ok(VmExit::MmioWrite(addr)),
221+
// KVM_EXIT_X86_RDMSR / KVM_EXIT_X86_WRMSR (KVM API §5, kvm_run structure):
222+
//
223+
// The "index" field tells userspace which MSR the guest wants to
224+
// read/write. If the request was unsuccessful, userspace indicates
225+
// that with a "1" in the "error" field. "This will inject a #GP
226+
// into the guest when the VCPU is executed again."
227+
//
228+
// "for KVM_EXIT_IO, KVM_EXIT_MMIO, [...] KVM_EXIT_X86_RDMSR and
229+
// KVM_EXIT_X86_WRMSR the corresponding operations are complete
230+
// (and guest state is consistent) only after userspace has
231+
// re-entered the kernel with KVM_RUN."
232+
//
233+
// We set error=1 and then re-run with `immediate_exit` to let KVM
234+
// inject the #GP without executing further guest code. From the
235+
// kvm_run docs: "[immediate_exit] is polled once when KVM_RUN
236+
// starts; if non-zero, KVM_RUN exits immediately, returning
237+
// -EINTR."
238+
Ok(VcpuExit::X86Rdmsr(msr_exit)) => {
239+
let msr_index = msr_exit.index;
240+
*msr_exit.error = 1;
241+
self.vcpu_fd.set_kvm_immediate_exit(1);
242+
let _ = self.vcpu_fd.run();
243+
self.vcpu_fd.set_kvm_immediate_exit(0);
244+
Ok(VmExit::MsrRead(msr_index))
245+
}
246+
Ok(VcpuExit::X86Wrmsr(msr_exit)) => {
247+
let msr_index = msr_exit.index;
248+
let value = msr_exit.data;
249+
*msr_exit.error = 1;
250+
self.vcpu_fd.set_kvm_immediate_exit(1);
251+
let _ = self.vcpu_fd.run();
252+
self.vcpu_fd.set_kvm_immediate_exit(0);
253+
Ok(VmExit::MsrWrite { msr_index, value })
254+
}
179255
#[cfg(gdb)]
180256
Ok(VcpuExit::Debug(debug_exit)) => Ok(VmExit::Debug {
181257
dr6: debug_exit.dr6,

src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,12 @@ pub(crate) enum VmExit {
135135
MmioRead(u64),
136136
/// The vCPU tried to write to the given (unmapped) addr
137137
MmioWrite(u64),
138+
/// The vCPU tried to read from the given MSR
139+
#[cfg(all(kvm, target_arch = "x86_64"))]
140+
MsrRead(u32),
141+
/// The vCPU tried to write to the given MSR with the given value
142+
#[cfg(all(kvm, target_arch = "x86_64"))]
143+
MsrWrite { msr_index: u32, value: u64 },
138144
/// The vCPU execution has been cancelled
139145
Cancelled(),
140146
/// The vCPU has exited for a reason that is not handled by Hyperlight
@@ -179,6 +185,11 @@ pub enum CreateVmError {
179185
HypervisorNotAvailable(HypervisorError),
180186
#[error("Initialize VM failed: {0}")]
181187
InitializeVm(HypervisorError),
188+
#[cfg(all(kvm, target_arch = "x86_64"))]
189+
#[error(
190+
"KVM MSR filtering not supported (requires KVM_CAP_X86_USER_SPACE_MSR and KVM_CAP_X86_MSR_FILTER)"
191+
)]
192+
MsrFilterNotSupported,
182193
#[error("Set Partition Property failed: {0}")]
183194
SetPartitionProperty(HypervisorError),
184195
#[cfg(target_os = "windows")]

src/hyperlight_host/src/sandbox/config.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ pub struct SandboxConfiguration {
7474
interrupt_vcpu_sigrtmin_offset: u8,
7575
/// How much writable memory to offer the guest
7676
scratch_size: usize,
77+
/// Allow MSR (Model Specific Register) access. This is disabled by default for security reasons.
78+
#[cfg(all(kvm, target_arch = "x86_64"))]
79+
allow_msr: bool,
7780
}
7881

7982
impl SandboxConfiguration {
@@ -118,6 +121,8 @@ impl SandboxConfiguration {
118121
guest_debug_info,
119122
#[cfg(crashdump)]
120123
guest_core_dump,
124+
#[cfg(all(kvm, target_arch = "x86_64"))]
125+
allow_msr: false,
121126
}
122127
}
123128

@@ -159,6 +164,27 @@ impl SandboxConfiguration {
159164
self.interrupt_vcpu_sigrtmin_offset
160165
}
161166

167+
/// Set whether MSR access is allowed. By default, MSR access is disabled
168+
/// for security reasons. This setting only applies when using KVM. It is a no-op on MSHV and WHP.
169+
///
170+
/// # Safety
171+
///
172+
/// If enabled, MSR intercepts are not installed, which means guest-modified
173+
/// MSRs are not reset across snapshot restores. This can leak state
174+
/// between guest executions within the same sandbox.
175+
#[cfg(all(kvm, target_arch = "x86_64"))]
176+
#[instrument(skip_all, parent = Span::current(), level= "Trace")]
177+
pub unsafe fn set_allow_msr(&mut self, allow_msr: bool) {
178+
self.allow_msr = allow_msr;
179+
}
180+
181+
/// Get whether MSR access is allowed
182+
#[cfg(all(kvm, target_arch = "x86_64"))]
183+
#[instrument(skip_all, parent = Span::current(), level= "Trace")]
184+
pub(crate) fn get_allow_msr(&self) -> bool {
185+
self.allow_msr
186+
}
187+
162188
/// Sets the offset from `SIGRTMIN` to determine the real-time signal used for
163189
/// interrupting the VCPU thread.
164190
///

src/hyperlight_host/src/sandbox/initialized_multi_use.rs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2011,4 +2011,76 @@ mod tests {
20112011

20122012
let _ = std::fs::remove_file(&path);
20132013
}
2014+
2015+
#[test]
2016+
#[cfg(all(kvm, target_arch = "x86_64"))]
2017+
fn test_msr_read_write_denied() {
2018+
use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor};
2019+
2020+
match get_available_hypervisor() {
2021+
Some(HypervisorType::Kvm) => {}
2022+
_ => {
2023+
return;
2024+
}
2025+
}
2026+
2027+
let mut sbox = UninitializedSandbox::new(
2028+
GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")),
2029+
None,
2030+
)
2031+
.unwrap()
2032+
.evolve()
2033+
.unwrap();
2034+
2035+
let snapshot = sbox.snapshot().unwrap();
2036+
let msr_index: u32 = 0xC000_0102; // IA32_KERNEL_GS_BASE
2037+
2038+
// RDMSR should be intercepted
2039+
let result = sbox.call::<u64>("ReadMSR", msr_index);
2040+
assert!(
2041+
matches!(
2042+
&result,
2043+
Err(HyperlightError::MsrReadViolation(idx)) if *idx == msr_index
2044+
),
2045+
"RDMSR 0x{:X}: expected MsrReadViolation, got: {:?}",
2046+
msr_index,
2047+
result
2048+
);
2049+
assert!(sbox.poisoned());
2050+
2051+
// Restore before next call
2052+
sbox.restore(snapshot.clone()).unwrap();
2053+
2054+
// WRMSR should be intercepted
2055+
let result = sbox.call::<()>("WriteMSR", (msr_index, 0x5u64));
2056+
assert!(
2057+
matches!(
2058+
&result,
2059+
Err(HyperlightError::MsrWriteViolation(idx, _)) if *idx == msr_index
2060+
),
2061+
"WRMSR 0x{:X}: expected MsrWriteViolation, got: {:?}",
2062+
msr_index,
2063+
result
2064+
);
2065+
assert!(sbox.poisoned());
2066+
2067+
// Also verify that MSR access works when explicitly allowed
2068+
let mut cfg = SandboxConfiguration::default();
2069+
unsafe { cfg.set_allow_msr(true) };
2070+
2071+
let mut sbox = UninitializedSandbox::new(
2072+
GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")),
2073+
Some(cfg),
2074+
)
2075+
.unwrap()
2076+
.evolve()
2077+
.unwrap();
2078+
2079+
let msr_index: u32 = 0xC000_0102; // IA32_KERNEL_GS_BASE
2080+
let value: u64 = 0x5;
2081+
2082+
sbox.call::<()>("WriteMSR", (msr_index, value)).unwrap();
2083+
let read_value: u64 = sbox.call("ReadMSR", msr_index).unwrap();
2084+
assert_eq!(read_value, value);
2085+
}
20142086
}

0 commit comments

Comments
 (0)