Skip to content

Commit ca728b5

Browse files
committed
Disallow guest MSR reads and writes
Guest reads and writes of model-specific registers are denied by default. SandboxConfiguration::allow_msr and allow_msrs permit specific MSRs. KVM enforces the boundary with a deny-by-default MSR filter. MSHV and WHP have no per-MSR filter, so a guest access to an unallowed MSR faults directly. On those backends isolation comes from capturing the retained MSR state at VM creation and resetting it on restore. Captured MSR state persists through OCI snapshots and reloads on restore. A denied access poisons the sandbox. KVM reports the MSR index. MSHV and WHP surface a guest general protection fault. See docs/msr.md. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
1 parent 817a2d3 commit ca728b5

22 files changed

Lines changed: 3613 additions & 27 deletions

File tree

.github/workflows/ValidatePullRequest.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ jobs:
149149
# pull goldens from GHCR in the called workflow
150150
packages: read
151151
strategy:
152-
fail-fast: true
152+
fail-fast: false
153153
matrix:
154154
# Temporarily disabled while Arm runners are unavailable.
155155
hypervisor: ['hyperv-ws2025', mshv3, kvm]

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
77
### Added
88

99
### Changed
10+
* **Breaking:** Guest reads and writes of model-specific registers are now denied
11+
by default and restored on snapshot restore. A guest permits specific MSRs with
12+
`SandboxConfiguration::allow_msrs` by @ludfjig in https://github.com/hyperlight-dev/hyperlight/pull/991
1013

1114
### Removed
1215

Justfile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,10 @@ test-isolated target=default-target features="" :
243243
{{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F " + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --test integration_test -- log_message --exact --ignored
244244
@# CPU vendor check, gated to known CI runner hardware
245245
{{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F " + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- sandbox::snapshot::file::config::tests::cpu_vendor_current_is_recognized --exact --ignored
246+
@# Slow host-dependent MSR audit. Run once per x86_64 CI profile.
247+
{{ if features == "" { if hyperlight-target-arch == "x86_64" { cargo-cmd + " test --profile=" + (if target == "debug" { "dev" } else { target }) + " " + target-triple-flag + " -p hyperlight-host --lib -- sandbox::initialized_multi_use::tests::msr_tests::test_no_msr_leaks_across_restore_full_window_sweep --exact --ignored --nocapture" } else { "" } } else { "" } }}
248+
@# LAPIC-enabled MSHV can expose additional MSRs. Audit it once in debug.
249+
{{ if features == "mshv3,hw-interrupts" { if target == "debug" { if hyperlight-target-arch == "x86_64" { cargo-cmd + " test --no-default-features -F mshv3,hw-interrupts --profile=dev " + target-triple-flag + " -p hyperlight-host --lib -- sandbox::initialized_multi_use::tests::msr_tests::test_no_msr_leaks_across_restore_full_window_sweep --exact --ignored --nocapture" } else { "" } } else { "" } } else { "" } }}
246250
@# metrics tests
247251
{{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F function_call_metrics," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- metrics::tests::test_metrics_are_emitted --exact
248252

docs/msr.md

Lines changed: 318 additions & 0 deletions
Large diffs are not rendered by default.

src/hyperlight_host/src/error.rs

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

157+
/// A denied guest MSR read.
158+
#[cfg(all(target_arch = "x86_64", kvm))]
159+
#[error("Guest read from denied MSR {0:#x}")]
160+
MsrReadViolation(u32),
161+
162+
/// A denied guest MSR write.
163+
#[cfg(all(target_arch = "x86_64", kvm))]
164+
#[error("Guest write of {1:#x} to denied MSR {0:#x}")]
165+
MsrWriteViolation(u32, u64),
166+
157167
/// Memory Allocation Failed.
158168
#[error("Memory Allocation Failed with OS Error {0:?}.")]
159169
MemoryAllocationFailed(Option<i32>),
@@ -352,6 +362,10 @@ impl HyperlightError {
352362
// as poisoning here too for defense in depth.
353363
| HyperlightError::HyperlightVmError(HyperlightVmError::Restore(_)) => true,
354364

365+
#[cfg(all(target_arch = "x86_64", kvm))]
366+
HyperlightError::MsrReadViolation(_)
367+
| HyperlightError::MsrWriteViolation(_, _) => true,
368+
355369
// These errors poison the sandbox because they can leave
356370
// it in an inconsistent state due to snapshot restore
357371
// failing partway through

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

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

166+
#[cfg(all(target_arch = "x86_64", kvm))]
167+
DispatchGuestCallError::Run(RunVmError::MsrReadViolation(msr_index)) => {
168+
HyperlightError::MsrReadViolation(msr_index)
169+
}
170+
171+
#[cfg(all(target_arch = "x86_64", kvm))]
172+
DispatchGuestCallError::Run(RunVmError::MsrWriteViolation { msr_index, value }) => {
173+
HyperlightError::MsrWriteViolation(msr_index, value)
174+
}
175+
166176
// Leave others as is
167177
other => HyperlightVmError::DispatchGuestCall(other).into(),
168178
};
@@ -213,6 +223,12 @@ pub enum RunVmError {
213223
MmioReadUnmapped(u64),
214224
#[error("MMIO WRITE access to unmapped address {0:#x}")]
215225
MmioWriteUnmapped(u64),
226+
#[cfg(all(target_arch = "x86_64", kvm))]
227+
#[error("Guest read from denied MSR {0:#x}")]
228+
MsrReadViolation(u32),
229+
#[cfg(all(target_arch = "x86_64", kvm))]
230+
#[error("Guest write of {value:#x} to denied MSR {msr_index:#x}")]
231+
MsrWriteViolation { msr_index: u32, value: u64 },
216232
#[error("vCPU run failed: {0}")]
217233
RunVcpu(#[from] RunVcpuError),
218234
#[error("Unexpected VM exit: {0}")]
@@ -409,6 +425,9 @@ pub(crate) struct HyperlightVm {
409425
pub(super) trace_info: MemTraceInfo,
410426
#[cfg(crashdump)]
411427
pub(super) rt_cfg: SandboxRuntimeConfig,
428+
/// MSRs restored on snapshot restore.
429+
#[cfg(target_arch = "x86_64")]
430+
pub(super) msr_reset: crate::hypervisor::regs::MsrResetState,
412431
}
413432

414433
impl HyperlightVm {
@@ -732,6 +751,14 @@ impl HyperlightVm {
732751
}
733752
}
734753
}
754+
#[cfg(all(target_arch = "x86_64", kvm))]
755+
Ok(VmExit::MsrRead(msr_index)) => {
756+
break Err(RunVmError::MsrReadViolation(msr_index));
757+
}
758+
#[cfg(all(target_arch = "x86_64", kvm))]
759+
Ok(VmExit::MsrWrite { msr_index, value }) => {
760+
break Err(RunVmError::MsrWriteViolation { msr_index, value });
761+
}
735762
Ok(VmExit::Cancelled()) => {
736763
// If cancellation was not requested for this specific guest function call,
737764
// the vcpu was interrupted by a stale cancellation. This can occur when:

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

Lines changed: 141 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,12 @@ use crate::hypervisor::gdb::{
4141
#[cfg(gdb)]
4242
use crate::hypervisor::gdb::{DebugError, DebugMemoryAccessError};
4343
use crate::hypervisor::regs::{
44-
CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters,
44+
CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, MsrEntry, MsrResetState,
45+
core_reset_indices, is_mtrr_reset_index,
4546
};
46-
#[cfg(not(gdb))]
47+
#[cfg(kvm)]
48+
use crate::hypervisor::regs::{MSR_KERNEL_GS_BASE, MSR_TSC};
49+
#[cfg(any(not(gdb), mshv3, target_os = "windows"))]
4750
use crate::hypervisor::virtual_machine::VirtualMachine;
4851
#[cfg(kvm)]
4952
use crate::hypervisor::virtual_machine::kvm::KvmVm;
@@ -69,6 +72,26 @@ use crate::sandbox::trace::MemTraceInfo;
6972
#[cfg(crashdump)]
7073
use crate::sandbox::uninitialized::SandboxRuntimeConfig;
7174

75+
#[cfg(gdb)]
76+
type BoxedVm = Box<dyn DebuggableVm>;
77+
#[cfg(not(gdb))]
78+
type BoxedVm = Box<dyn VirtualMachine>;
79+
80+
/// Determines the MTRR reset indices for an MSHV or WHP backend and validates
81+
/// its allow list. Both hosts lack an MSR filter, so the allow list adds reset
82+
/// state.
83+
#[cfg(any(mshv3, target_os = "windows"))]
84+
fn determine_reset_msrs(
85+
vm: &dyn VirtualMachine,
86+
allowed: &[u32],
87+
) -> std::result::Result<Vec<u32>, VmError> {
88+
use crate::hypervisor::virtual_machine::{mtrr_reset_indices, validate_allowed_msrs};
89+
90+
let required_mtrrs = mtrr_reset_indices(vm).map_err(VmError::CreateVm)?;
91+
validate_allowed_msrs(vm, allowed).map_err(VmError::CreateVm)?;
92+
Ok(required_mtrrs)
93+
}
94+
7295
impl HyperlightVm {
7396
/// Create a new HyperlightVm instance (will not run vm until calling `initialise`)
7497
#[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")]
@@ -85,18 +108,27 @@ impl HyperlightVm {
85108
#[cfg(crashdump)] rt_cfg: SandboxRuntimeConfig,
86109
#[cfg(feature = "mem_profile")] trace_info: MemTraceInfo,
87110
) -> std::result::Result<Self, CreateHyperlightVmError> {
88-
#[cfg(gdb)]
89-
type VmType = Box<dyn DebuggableVm>;
90-
#[cfg(not(gdb))]
91-
type VmType = Box<dyn VirtualMachine>;
92-
93-
let vm: VmType = match get_available_hypervisor() {
111+
let (vm, required_mtrrs): (BoxedVm, Vec<u32>) = match get_available_hypervisor() {
94112
#[cfg(kvm)]
95-
Some(HypervisorType::Kvm) => Box::new(KvmVm::new().map_err(VmError::CreateVm)?),
113+
Some(HypervisorType::Kvm) => {
114+
let kvm_vm = KvmVm::new().map_err(VmError::CreateVm)?;
115+
kvm_vm
116+
.configure_msr_access(config.get_allowed_msrs())
117+
.map_err(VmError::CreateVm)?;
118+
(Box::new(kvm_vm), Vec::new())
119+
}
96120
#[cfg(mshv3)]
97-
Some(HypervisorType::Mshv) => Box::new(MshvVm::new().map_err(VmError::CreateVm)?),
121+
Some(HypervisorType::Mshv) => {
122+
let vm: BoxedVm = Box::new(MshvVm::new().map_err(VmError::CreateVm)?);
123+
let required_mtrrs = determine_reset_msrs(vm.as_ref(), config.get_allowed_msrs())?;
124+
(vm, required_mtrrs)
125+
}
98126
#[cfg(target_os = "windows")]
99-
Some(HypervisorType::Whp) => Box::new(WhpVm::new().map_err(VmError::CreateVm)?),
127+
Some(HypervisorType::Whp) => {
128+
let vm: BoxedVm = Box::new(WhpVm::new().map_err(VmError::CreateVm)?);
129+
let required_mtrrs = determine_reset_msrs(vm.as_ref(), config.get_allowed_msrs())?;
130+
(vm, required_mtrrs)
131+
}
100132
None => return Err(CreateHyperlightVmError::NoHypervisorFound),
101133
};
102134

@@ -136,6 +168,10 @@ impl HyperlightVm {
136168
}),
137169
});
138170

171+
// The MSRs reset on restore and captured in snapshots.
172+
let msr_reset =
173+
Self::capture_msr_reset_state(&vm, required_mtrrs, config.get_allowed_msrs())?;
174+
139175
let snapshot_slot = 0u32;
140176
let scratch_slot = 1u32;
141177
#[cfg_attr(not(gdb), allow(unused_mut))]
@@ -168,6 +204,7 @@ impl HyperlightVm {
168204
trace_info,
169205
#[cfg(crashdump)]
170206
rt_cfg,
207+
msr_reset,
171208
};
172209

173210
ret.update_snapshot_mapping(snapshot_mem)?;
@@ -199,6 +236,50 @@ impl HyperlightVm {
199236
Ok(ret)
200237
}
201238

239+
/// Determines this VM's MSR reset set: the MSRs reset on restore and
240+
/// captured in snapshots. `required_mtrrs` is the VCNT-driven MTRR set
241+
/// gathered at creation. `allowed` is the validated allow list.
242+
fn capture_msr_reset_state(
243+
vm: &BoxedVm,
244+
required_mtrrs: Vec<u32>,
245+
allowed: &[u32],
246+
) -> std::result::Result<MsrResetState, VmError> {
247+
let core: Vec<u32> = match get_available_hypervisor() {
248+
#[cfg(kvm)]
249+
Some(HypervisorType::Kvm) => vec![MSR_KERNEL_GS_BASE, MSR_TSC], // GS_BASE is needed for correctness. TSC is to match mshv/whp behavior
250+
#[cfg(mshv3)]
251+
Some(HypervisorType::Mshv) => Self::probe_core_reset_indices(vm),
252+
#[cfg(target_os = "windows")]
253+
Some(HypervisorType::Whp) => Self::probe_core_reset_indices(vm),
254+
// The VM already exists, so a hypervisor was found. The `None`
255+
// arm in `new` returned before this point.
256+
None => unreachable!(),
257+
};
258+
let mut indices: Vec<u32> = core
259+
.into_iter()
260+
.chain(required_mtrrs)
261+
.chain(allowed.iter().copied())
262+
.collect();
263+
indices.sort_unstable();
264+
indices.dedup();
265+
let baseline = vm.msrs(&indices).map_err(VmError::Register)?;
266+
let mut allowed = allowed.to_vec();
267+
allowed.sort_unstable();
268+
allowed.dedup();
269+
Ok(MsrResetState::new(baseline, allowed))
270+
}
271+
272+
/// Core stateful MSRs a filterless host can read.
273+
///
274+
/// MTRRs come from the VCNT-driven required set, so they are excluded.
275+
#[cfg(any(mshv3, target_os = "windows"))]
276+
fn probe_core_reset_indices(vm: &BoxedVm) -> Vec<u32> {
277+
core_reset_indices()
278+
.filter(|i| !is_mtrr_reset_index(*i))
279+
.filter(|i| vm.msrs(&[*i]).is_ok())
280+
.collect()
281+
}
282+
202283
/// Initialise the internally stored vCPU with the given PEB address and
203284
/// random number seed, then run it until a HLT instruction.
204285
#[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")]
@@ -270,6 +351,55 @@ impl HyperlightVm {
270351
Ok(self.vm.sregs()?)
271352
}
272353

354+
/// Returns the current values of the MSR reset set.
355+
pub(crate) fn get_msr_reset_state(&self) -> Result<Vec<MsrEntry>, AccessPageTableError> {
356+
Ok(self.vm.msrs(&self.msr_reset.indices())?)
357+
}
358+
359+
/// Returns this VM's requested allow list, sorted and deduplicated.
360+
pub(crate) fn get_msr_allow_list(&self) -> Vec<u32> {
361+
self.msr_reset.allowed().to_vec()
362+
}
363+
364+
/// Restores snapshot MSRs or the initialization baseline.
365+
pub(crate) fn restore_msrs(
366+
&mut self,
367+
snap_msrs: Option<&Vec<MsrEntry>>,
368+
snap_allowed: Option<&[u32]>,
369+
) -> std::result::Result<(), ResetVcpuError> {
370+
match snap_msrs {
371+
// A snapshot with no captured set predates MSR capture.
372+
// Reset to this VM's creation-time baseline.
373+
None => self.vm.set_msrs(self.msr_reset.baseline())?,
374+
// Reset the MSRs to the snapshot's captured values. The snapshot's
375+
// allow list must be a subset of this VM's, and every captured
376+
// index must be in this reset set, so validation rejects a
377+
// mismatch first.
378+
Some(msrs) => {
379+
let entries = self
380+
.msr_reset
381+
.validate_snapshot(msrs, snap_allowed.unwrap_or(&[]))?;
382+
self.vm.set_msrs(&entries)?;
383+
}
384+
}
385+
Ok(())
386+
}
387+
388+
/// Reads arbitrary backend MSRs for tests.
389+
#[cfg(all(test, mshv3, target_arch = "x86_64"))]
390+
pub(crate) fn capture_msrs_for_test(
391+
&self,
392+
indices: &[u32],
393+
) -> std::result::Result<Vec<MsrEntry>, RegisterError> {
394+
self.vm.msrs(indices)
395+
}
396+
397+
/// Attempts one backend MSR write for tests.
398+
#[cfg(all(test, mshv3, target_arch = "x86_64"))]
399+
pub(crate) fn try_set_msr_for_test(&self, index: u32, value: u64) -> bool {
400+
self.vm.set_msrs(&[MsrEntry { index, value }]).is_ok()
401+
}
402+
273403
/// Dispatch a call from the host to the guest using the given pointer
274404
/// to the dispatch function _in the guest's address space_.
275405
///

src/hyperlight_host/src/hypervisor/regs/x86_64/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@ limitations under the License.
1616

1717
mod debug_regs;
1818
mod fpu;
19+
mod msrs;
1920
mod special_regs;
2021
mod standard_regs;
2122

2223
pub(crate) use debug_regs::*;
2324
pub(crate) use fpu::*;
25+
pub(crate) use msrs::*;
2426
pub(crate) use special_regs::*;
2527
pub(crate) use standard_regs::*;
2628

0 commit comments

Comments
 (0)