From 9b718276bbe847eed163deba38032cc549fd4f66 Mon Sep 17 00:00:00 2001 From: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> Date: Wed, 15 Jul 2026 16:37:56 -0700 Subject: [PATCH 1/4] Prevent MSR state leaking across restore KVM denies guest MSR access by default. SandboxConfiguration::allow_msrs permits selected MSRs. MSHV and WHP have no per-MSR filter. Hyperlight captures exposed retained MSR state at VM creation and resets it on restore. Captured MSR state persists in OCI snapshots. KVM denials report the MSR index. Unsupported accesses on MSHV and WHP raise a guest general protection fault. Both failures poison the sandbox. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> --- CHANGELOG.md | 4 + Justfile | 4 + docs/msr.md | 304 ++++ docs/snapshot-versioning.md | 7 + src/hyperlight_host/src/error.rs | 14 + .../src/hypervisor/hyperlight_vm/mod.rs | 27 + .../src/hypervisor/hyperlight_vm/x86_64.rs | 151 +- .../src/hypervisor/regs/x86_64/mod.rs | 2 + .../src/hypervisor/regs/x86_64/msrs.rs | 483 +++++++ .../hypervisor/regs/x86_64/special_regs.rs | 10 + .../hypervisor/virtual_machine/kvm/x86_64.rs | 336 ++++- .../src/hypervisor/virtual_machine/mod.rs | 118 ++ .../hypervisor/virtual_machine/mshv/x86_64.rs | 140 +- .../src/hypervisor/virtual_machine/whp.rs | 203 ++- src/hyperlight_host/src/mem/mgr.rs | 8 + src/hyperlight_host/src/sandbox/config.rs | 128 +- .../src/sandbox/initialized_multi_use.rs | 1254 ++++++++++++++++- .../src/sandbox/snapshot/file/config.rs | 85 ++ .../src/sandbox/snapshot/file/mod.rs | 17 + .../src/sandbox/snapshot/file_tests.rs | 194 +++ .../src/sandbox/snapshot/mod.rs | 56 + src/tests/rust_guests/simpleguest/src/main.rs | 81 ++ 22 files changed, 3600 insertions(+), 26 deletions(-) create mode 100644 docs/msr.md create mode 100644 src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 571a2ec98..09824af04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added ### Changed +* **Breaking:** Retained guest MSR state is restored on snapshot restore. KVM + denies guest MSR reads and writes by default. MSHV and WHP reset all exposed + retained MSR state. `SandboxConfiguration::allow_msrs` permits specific MSRs + by @ludfjig in https://github.com/hyperlight-dev/hyperlight/pull/991 ### Removed diff --git a/Justfile b/Justfile index 5112f2f88..f69d88c41 100644 --- a/Justfile +++ b/Justfile @@ -244,6 +244,10 @@ test-isolated target=default-target features="" : {{ 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 @# CPU vendor check, gated to known CI runner hardware {{ 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 + @# Slow host-dependent MSR audit. Run once per x86_64 CI profile. + {{ 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 { "" } }} + @# LAPIC-enabled MSHV can expose additional MSRs. Audit it once in debug. + {{ 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 { "" } }} @# metrics tests {{ 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 diff --git a/docs/msr.md b/docs/msr.md new file mode 100644 index 000000000..316f097c5 --- /dev/null +++ b/docs/msr.md @@ -0,0 +1,304 @@ +# MSR state across restore + +## Requirement + +A snapshot restore must remove all model-specific register (MSR) state written +after the snapshot. The guest must observe the MSR values saved with the +restored state. + +## Reset set + +The reset set contains every MSR whose guest-written value can persist. Each +running snapshot stores values for this set. A snapshot created from a guest +binary has no saved MSR values, so restore uses the baseline captured when the +VM was created. + +`MSR_TABLE` lists the MSRs that hold retained state restore must write. A +write-only command MSR holds no state, so it is absent from the table. + +The resolved reset set contains the backend core set, required MTRRs, and the +validated allow list. Hyperlight sorts and deduplicates the indices before +capturing the initialization baseline. + +The required invariant is: + +```text +guest-writable retained state => host-readable and host-writable state +``` + +Host-readable state need not be guest-writable. Extra reset entries are safe. +`EFER`, `APIC_BASE`, `FS_BASE`, and `GS_BASE` belong to the special-register +state. + +The two halves are established differently. Resolution checks the +host-readable half at run time: a candidate index enters the set only when the +host read succeeds, so an unreadable MSR is dropped. Nothing checks the +host-writable half at run time. It holds by construction. Every reset MSR is +stored in VP register state that the Hyper-V host interface both reads and +writes, except `KERNEL_GS_BASE`, a real register the host reads and writes +directly. Round-trip tests plant a guest value, restore, and assert that it does +not survive. Every future entry must remain host-readable and host-writable. + +## Reset set justification + +Each entry is grounded in how the Hyper-V hypervisor handles a guest access to +that register, confirmed against the Hyper-V source. + +A register is reset when the guest can write it and Hyper-V keeps the written +value. Hyper-V keeps it in one of two ways: it stores the value in the VP's +saved register state, or it lets the guest write the real register directly. +Only `FS_BASE`, `GS_BASE`, and `KERNEL_GS_BASE` are written directly. Every +other register below is intercepted and stored. Hyper-V stores the value on both +Intel and AMD hosts. + +Interception alone is not the test. Hyper-V also intercepts registers the guest +can only read, or that return a host-derived value. Those keep no +guest-controlled state and are not reset. Each row below names the guest state +that persists. + +| MSR (index) | Retained guest state | +| --- | --- | +| SYSENTER CS, ESP, EIP (`0x174`-`0x176`) | Guest write retained. | +| STAR, LSTAR, CSTAR, SFMASK (`0xC000_0081`-`0xC000_0084`) | Guest write retained (syscall targets). | +| KERNEL_GS_BASE (`0xC000_0102`) | Guest write retained. Written to the real register, and reachable through `SWAPGS` without a `WRMSR`. | +| PAT (`0x277`) | Guest write retained. | +| DEBUGCTL (`0x1D9`) | Guest write retained. | +| SPEC_CTRL (`0x48`) | Guest write retained. | +| CET U_CET, S_CET, PL0-3_SSP, INTERRUPT_SSP_TABLE_ADDR (`0x6A0`, `0x6A2`, `0x6A4`-`0x6A8`) | Guest write retained. | +| XSS (`0xDA0`) | Guest write retained. | +| TSC (`0x10`) | Guest write retained. Hyper-V forbids intercepting its implemented TSC, so restore rewrites the captured value. | +| TSC_ADJUST (`0x3B`) | Guest write retained, independent of TSC. | +| TSC_AUX (`0xC000_0103`) | Guest write retained. | +| MTRRs (`0x2FF`, `0x200`-`0x21F`, `0x250`, `0x258`-`0x259`, `0x268`-`0x26F`) | Guest write retained (memory-type state). | +| TSX_CTRL (`0x122`) | Guest write retained. | +| XFD, XFD_ERR (`0x1C4`, `0x1C5`) | Guest write retained. | +| UMWAIT_CONTROL (`0xE1`) | Guest write retained. Intel only. | +| TSC_DEADLINE (`0x6E0`) | Guest write retained. | +| BNDCFGS (`0xD90`) | Guest write retained when the host supports MPX. A guest access faults otherwise. | +| MPERF, APERF (`0xE7`, `0xE8`) | Guest write retained in per-VP counters. | + +Write-only command MSRs hold no state. A guest write performs an action and +leaves nothing to restore, so they are absent from the reset table and cannot +be allowed. + +| MSR (index) | Behavior | +| --- | --- | +| PRED_CMD (`0x49`) | Guest write issues a prediction barrier. | +| FLUSH_CMD (`0x10B`) | Guest write flushes caches. | + +Some registers are deliberately absent because guest access cannot leave state +outside another reset mechanism. + +| MSR (index) | Why excluded | +| --- | --- | +| MISC_ENABLE (`0x1A0`) | Intercepted, but Hyper-V discards a guest write and returns a fixed value. No retained state. On AMD the access faults. See below. | +| FRED (`0x1CC`-`0x1D4`) | Retained only when the host exposes FRED, which Hyperlight does not. A guest access faults otherwise. | +| PASID (`0xD93`) | MSHV exposes ENQCMD on capable Intel hosts. PASID is a supervisor XSAVE component, so XSAVE reset clears it. WHP does not expose ENQCMD. | +| PMU: PMC0, PERFEVTSEL0, FIXED_CTR_CTRL, PERF_GLOBAL_CTRL (`0xC1`, `0x186`, `0x38D`, `0x38F`) | Heads of the performance-monitoring class. Hyper-V leaves these unimplemented for the guest and installs guest-accessible descriptors, sized to the CPU counter count, only when perfmon is enabled. Hyperlight enables no perfmon, so a guest access faults and retains nothing. | +| LBR: LBR_SELECT, LASTBRANCH_TOS, LBR_CTL, LBR_DEPTH (`0x1C8`, `0x1C9`, `0x14CE`, `0x14CF`) | Last-branch registers, gated with perfmon. A guest access faults and retains nothing while perfmon stays off. | + +Hyper-V virtualizes `BNDCFGS`, `FRED`, and `PASID` only when the matching CPU +feature is exposed. `BNDCFGS` is reset because MPX is exposed by default on +capable hosts. `FRED` stays excluded because Hyperlight does not expose it. +MSHV can expose ENQCMD, but its XSAVE state mask then includes PASID. Hyperlight +clears PASID during XSAVE reset before restoring MSRs. The performance-monitoring +and last-branch registers remain inaccessible while perfmon is off. + +## Snapshot validation + +Snapshot MSR entries are untrusted. A snapshot records the reset values and the +capturing sandbox's allow list. `validate_snapshot` enforces two rules against +the destination VM's reset set: + +* The snapshot's allow list must be a subset of the destination's. A + destination that allows at least as much accepts the snapshot. +* Every supplied index must belong to the destination reset set. + +Indices the destination resets but the snapshot omits take the destination's +creation-time baseline. A rejected restore poisons the sandbox before the guest +can run. Equivalent allow lists produce the same sorted reset set, regardless of +insertion order. + +## Restore across allow lists + +A restore or `from_snapshot` succeeds when the destination allow list is a +superset of the snapshot's, on every backend. The snapshot's allowed MSRs keep +their captured values. An MSR the destination allows but the snapshot did not +resets to the destination baseline. A non-superset allow list is rejected +uniformly. + +The rule is backend independent even though each backend sizes its reset set +differently. KVM derives its reset set from the allow list. MSHV and WHP reset +the full host table. The allow-list subset check gates the restore before either +reset set is applied, so a flow that succeeds on one backend succeeds on all. + +The superset check is the common rule across backends. MSHV and WHP accept any +allow list on their own. The shared check gives every backend KVM's constraint. + +## Allow list + +`SandboxConfiguration::allow_msrs` adds indices to the requested allow list. It +enforces capacity only. VM creation verifies that each index is resettable and +supported by the selected backend. + +KVM requires the index in `KVM_GET_MSR_INDEX_LIST` and a successful host read +and write. MSHV and WHP require a named-register mapping and a successful host +read. + +At most 16 distinct MSRs may be requested. KVM also limits the resulting +contiguous filter groups to 16. + +## KVM + +KVM installs a deny filter over the full MSR space. Allowed indices form the +only guest `RDMSR` and `WRMSR` paths through that filter. A denied access exits +to Hyperlight, injects `#GP`, and poisons the sandbox. The denied write stores +no state. + +The KVM reset set contains the allow list plus `KERNEL_GS_BASE` and `TSC`. +`KERNEL_GS_BASE` is required because `WRGSBASE` followed by `SWAPGS` changes it +without `WRMSR`. `TSC` gives restore the same clock semantics on every backend. + +KVM does not filter x2APIC indices `0x800..=0x8FF`. Hyperlight keeps the APIC in +xAPIC mode, where MSR access to that range raises `#GP`. `APIC_BASE` is not an +allowable MSR, so a guest cannot enable x2APIC. Snapshots created by Hyperlight +therefore retain `APIC_BASE.EXTD = 0`. File snapshots serialize `APIC_BASE` +without semantic validation, so the caller must trust the snapshot source as +required by the snapshot format. + +## MTRRs + +MSHV and WHP read `IA32_MTRRCAP` when the VM is created. The required set +contains `MTRR_DEF_TYPE`, each variable pair reported by `VCNT`, and all fixed +MTRRs. + +Hyper-V accepts fixed-MTRR writes even when `MTRRCAP.FIX` is clear. All fixed +MTRRs are therefore required. Hyper-V supports at most 16 variable pairs. VM +creation fails when the count is larger or a required MTRR cannot be read. + +## MSHV + +MSHV has no per-MSR filter. Hyper-V permits an MSR intercept only for an +unimplemented index, which already faults for the guest, and cannot intercept +the implemented MSRs that hold retained state. Isolation therefore comes from +reset, not a deny filter. + +The MSHV reset set contains every table entry that has a Hyper-V +register mapping and can be read, plus the allow list. + +`msr_to_hv_reg_name` determines which indices the get and set path can reach. +The enumerated host index list does not identify retained state, so it does not +define the reset set. + +MSHV maps `IA32_XSS` through `MSR_IA32_REGISTER_U_XSS`. It maps `IA32_MPERF` +and `IA32_APERF` to the per-VP `MCount` and `ACount` registers. TSX control, +XFD, MPX (`BNDCFGS`), WAITPKG (`UMWAIT_CONTROL`), and the TSC deadline timer +enter the reset set when their host-register probes succeed. + +MSHV enables every host-supported processor feature unless the caller supplies +an explicit disabled-feature mask. Hyperlight supplies no mask. On capable +Intel hosts this can expose ENQCMD and its PASID MSR. MSHV reports PASID in the +partition XSAVE state mask, and Hyperlight's XSAVE reset clears it. + +## WHP + +WHP has no per-MSR filter. Its reset set contains every table entry +that has a WHP register name and can be read, plus the allow list. + +WHP uses Germanium compatibility. Speculation control is off in its default +feature banks, and perfmon (the PMU and architectural LBR) is a separate +property WHP leaves off. Experimental `DEBUGCTL` bits stay disabled. The WHP +API defines no FRED feature and its supported feature mask omits ENQCMD, so WHP +cannot expose FRED or PASID. + +Each guest MSR write is either captured for restore or unsupported by the +partition. Unsupported writes store no state. + +## TSC + +MSHV and WHP expose `TSC` as a host-writable register. Hyper-V stores `TSC` and +`TSC_ADJUST` independently, so restoring `TSC_ADJUST` cannot undo a guest +`WRMSR(TSC)`. + +While time is running, Hyper-V preserves `TSC - TSC_ADJUST`: writing `TSC` +adds the same delta to `TSC_ADJUST`, and writing `TSC_ADJUST` adds its delta to +the internal TSC offset. Restoring `TSC` followed by `TSC_ADJUST` therefore +cancels any guest-controlled delta. Freezing partition time is not required for +isolation. + +Hyper-V does not permit an intercept for its implemented `TSC` MSR. Restore +must therefore write the captured `TSC` value. KVM also restores `TSC` so all +backends rewind guest time with the rest of the snapshot state. + +## Feature exposure + +On MSHV and WHP a guest reaches an MSR only when the hypervisor exposes that +CPU feature to the partition. This gives three cases: + +* Not exposed. Features the partition does not enable, such as the + performance-monitoring unit, last-branch records, and FRED. Hyper-V may still + model the register, but a guest access faults and stores no state until the + feature is exposed. +* Exposed by default. Features the host CPU supports, such as TSC deadline, + UMWAIT, TSX control, CET, `MPERF`/`APERF`, XFD, AMX, and MPX. Their MSRs + must be in the reset set. +* Reset through another state class. MSHV can expose ENQCMD and PASID on + capable Intel hosts. PASID is cleared by XSAVE reset, so it is not duplicated + in the MSR reset set. + +MSHV and WHP enable partition features differently. MSHV creates the partition +without an explicit feature mask, so it enables every processor feature the host +supports. WHP starts from the host-supported set with speculation control off. +MSHV exposes the broader surface and determines which registers the reset set +must cover. + +Perfmon is not part of either default. The performance-monitoring unit and the +last-branch registers are a separate opt-in partition property, off by default +on both backends. Hyperlight never enables it, so those registers stay +unreachable regardless of the enable-everything processor-feature default. + +Only reachable, retained MSRs need coverage, and retained state is always held +in a host-readable and writable register. The mapped registers therefore bound +the reset set: coverage is complete when every mapped register is in the reset +table and reset. + +## Host-addressable but not guest-writable + +A host register mapping does not imply the guest can write the MSR. +`IA32_MISC_ENABLE` (`0x1A0`) is the notable case. Hyper-V emulates it, discards +a guest write, and returns a fixed value to the guest regardless of what was +written. A guest cannot change it to any value, so it retains no guest state +and needs no reset. On AMD the guest access faults. + +## Failed access reporting + +A KVM-denied or Hyper-V-unsupported MSR access does not persist and poisons the +sandbox. The error type and its detail differ by backend. + +* KVM traps the access at the deny filter. Hyperlight reports + `MsrReadViolation` or `MsrWriteViolation`, naming the MSR index and, for a + write, the value. The report is host-verified. +* MSHV and WHP have no host MSR trap. An unsupported access faults inside the + guest as a general protection fault from Hyper-V, so Hyperlight reports + `GuestAborted`. The message records the fault and the faulting instruction but + does not identify the MSR. An exposed MSR can succeed even when absent from + the allow list. Its retained state must be in the reset table. + +Future work: the guest exception handler could decode a faulting `RDMSR` or +`WRMSR` and report the index, promoting the abort to a typed MSR violation on +MSHV and WHP. That index would be guest-reported and therefore advisory. It is +not implemented. + +## Limitations + +KVM's security boundary is structural because its deny filter bounds guest +writes. MSHV and WHP depend on the reset table and exposed processor +features. + +The filterless backend tests run on one CPU model per runner. Model-specific +state absent on that CPU is not exercised. A backend that exposes a new +retained MSR feature needs a matching table entry before Hyperlight can use it +safely. + +The ignored full-window audit probes fixed index ranges with a small set of +values. It cannot prove that every vendor MSR or accepted value is covered. diff --git a/docs/snapshot-versioning.md b/docs/snapshot-versioning.md index 6ce5785fc..a5ff5e305 100644 --- a/docs/snapshot-versioning.md +++ b/docs/snapshot-versioning.md @@ -56,6 +56,13 @@ made before it was added remain loadable. At the next hard break, make the field required, remove `serde(default)`, and reject zero as an invalid entry point rather than treating it as unknown. +### Optional MSR fields + +The persisted `msrs` and `allowed_msrs` fields are optional so snapshots made +before MSR capture remain loadable. At the next hard break, make them required +vectors and remove `serde(default)` and the missing-field fallback. Keep the +in-memory fields optional while `Snapshot` represents pre-init state. + ## Enforcement The format is large and easy to change by accident. Two mechanisms diff --git a/src/hyperlight_host/src/error.rs b/src/hyperlight_host/src/error.rs index c6738374d..0b982eed6 100644 --- a/src/hyperlight_host/src/error.rs +++ b/src/hyperlight_host/src/error.rs @@ -154,6 +154,16 @@ pub enum HyperlightError { #[error("Memory Access Violation at address {0:#x} of type {1}, but memory is marked as {2}")] MemoryAccessViolation(u64, MemoryRegionFlags, MemoryRegionFlags), + /// A denied guest MSR read. + #[cfg(all(target_arch = "x86_64", kvm))] + #[error("Guest read from denied MSR {0:#x}")] + MsrReadViolation(u32), + + /// A denied guest MSR write. + #[cfg(all(target_arch = "x86_64", kvm))] + #[error("Guest write of {1:#x} to denied MSR {0:#x}")] + MsrWriteViolation(u32, u64), + /// Memory Allocation Failed. #[error("Memory Allocation Failed with OS Error {0:?}.")] MemoryAllocationFailed(Option), @@ -352,6 +362,10 @@ impl HyperlightError { // as poisoning here too for defense in depth. | HyperlightError::HyperlightVmError(HyperlightVmError::Restore(_)) => true, + #[cfg(all(target_arch = "x86_64", kvm))] + HyperlightError::MsrReadViolation(_) + | HyperlightError::MsrWriteViolation(_, _) => true, + // These errors poison the sandbox because they can leave // it in an inconsistent state due to snapshot restore // failing partway through diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs index b046f070f..7f8ed28aa 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs @@ -163,6 +163,16 @@ impl DispatchGuestCallError { region_flags, }) => HyperlightError::MemoryAccessViolation(addr, access_type, region_flags), + #[cfg(all(target_arch = "x86_64", kvm))] + DispatchGuestCallError::Run(RunVmError::MsrReadViolation(msr_index)) => { + HyperlightError::MsrReadViolation(msr_index) + } + + #[cfg(all(target_arch = "x86_64", kvm))] + DispatchGuestCallError::Run(RunVmError::MsrWriteViolation { msr_index, value }) => { + HyperlightError::MsrWriteViolation(msr_index, value) + } + // Leave others as is other => HyperlightVmError::DispatchGuestCall(other).into(), }; @@ -213,6 +223,12 @@ pub enum RunVmError { MmioReadUnmapped(u64), #[error("MMIO WRITE access to unmapped address {0:#x}")] MmioWriteUnmapped(u64), + #[cfg(all(target_arch = "x86_64", kvm))] + #[error("Guest read from denied MSR {0:#x}")] + MsrReadViolation(u32), + #[cfg(all(target_arch = "x86_64", kvm))] + #[error("Guest write of {value:#x} to denied MSR {msr_index:#x}")] + MsrWriteViolation { msr_index: u32, value: u64 }, #[error("vCPU run failed: {0}")] RunVcpu(#[from] RunVcpuError), #[error("Unexpected VM exit: {0}")] @@ -409,6 +425,9 @@ pub(crate) struct HyperlightVm { pub(super) trace_info: MemTraceInfo, #[cfg(crashdump)] pub(super) rt_cfg: SandboxRuntimeConfig, + /// MSRs restored on snapshot restore. + #[cfg(target_arch = "x86_64")] + pub(super) msr_reset: crate::hypervisor::regs::MsrResetState, } impl HyperlightVm { @@ -740,6 +759,14 @@ impl HyperlightVm { } } } + #[cfg(all(target_arch = "x86_64", kvm))] + Ok(VmExit::MsrRead(msr_index)) => { + break Err(RunVmError::MsrReadViolation(msr_index)); + } + #[cfg(all(target_arch = "x86_64", kvm))] + Ok(VmExit::MsrWrite { msr_index, value }) => { + break Err(RunVmError::MsrWriteViolation { msr_index, value }); + } Ok(VmExit::Cancelled()) => { // If cancellation was not requested for this specific guest function call, // the vcpu was interrupted by a stale cancellation. This can occur when: diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs index ae17e100b..47b92125c 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -41,9 +41,12 @@ use crate::hypervisor::gdb::{ #[cfg(gdb)] use crate::hypervisor::gdb::{DebugError, DebugMemoryAccessError}; use crate::hypervisor::regs::{ - CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, + CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, MsrEntry, MsrResetState, + core_reset_indices, is_mtrr_reset_index, }; -#[cfg(not(gdb))] +#[cfg(kvm)] +use crate::hypervisor::regs::{MSR_KERNEL_GS_BASE, MSR_TSC}; +#[cfg(any(not(gdb), mshv3, target_os = "windows"))] use crate::hypervisor::virtual_machine::VirtualMachine; #[cfg(kvm)] use crate::hypervisor::virtual_machine::kvm::KvmVm; @@ -69,6 +72,26 @@ use crate::sandbox::trace::MemTraceInfo; #[cfg(crashdump)] use crate::sandbox::uninitialized::SandboxRuntimeConfig; +#[cfg(gdb)] +type BoxedVm = Box; +#[cfg(not(gdb))] +type BoxedVm = Box; + +/// Determines the MTRR reset indices for an MSHV or WHP backend and validates +/// its allow list. Both hosts lack an MSR filter, so the allow list adds reset +/// state. +#[cfg(any(mshv3, target_os = "windows"))] +fn determine_reset_msrs( + vm: &dyn VirtualMachine, + allowed: &[u32], +) -> std::result::Result, VmError> { + use crate::hypervisor::virtual_machine::{mtrr_reset_indices, validate_allowed_msrs}; + + let required_mtrrs = mtrr_reset_indices(vm).map_err(VmError::CreateVm)?; + validate_allowed_msrs(vm, allowed).map_err(VmError::CreateVm)?; + Ok(required_mtrrs) +} + impl HyperlightVm { /// Create a new HyperlightVm instance (will not run vm until calling `initialise`) #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] @@ -85,18 +108,27 @@ impl HyperlightVm { #[cfg(crashdump)] rt_cfg: SandboxRuntimeConfig, #[cfg(feature = "mem_profile")] trace_info: MemTraceInfo, ) -> std::result::Result { - #[cfg(gdb)] - type VmType = Box; - #[cfg(not(gdb))] - type VmType = Box; - - let vm: VmType = match get_available_hypervisor() { + let (vm, required_mtrrs): (BoxedVm, Vec) = match get_available_hypervisor() { #[cfg(kvm)] - Some(HypervisorType::Kvm) => Box::new(KvmVm::new().map_err(VmError::CreateVm)?), + Some(HypervisorType::Kvm) => { + let kvm_vm = KvmVm::new().map_err(VmError::CreateVm)?; + kvm_vm + .configure_msr_access(config.get_allowed_msrs()) + .map_err(VmError::CreateVm)?; + (Box::new(kvm_vm), Vec::new()) + } #[cfg(mshv3)] - Some(HypervisorType::Mshv) => Box::new(MshvVm::new().map_err(VmError::CreateVm)?), + Some(HypervisorType::Mshv) => { + let vm: BoxedVm = Box::new(MshvVm::new().map_err(VmError::CreateVm)?); + let required_mtrrs = determine_reset_msrs(vm.as_ref(), config.get_allowed_msrs())?; + (vm, required_mtrrs) + } #[cfg(target_os = "windows")] - Some(HypervisorType::Whp) => Box::new(WhpVm::new().map_err(VmError::CreateVm)?), + Some(HypervisorType::Whp) => { + let vm: BoxedVm = Box::new(WhpVm::new().map_err(VmError::CreateVm)?); + let required_mtrrs = determine_reset_msrs(vm.as_ref(), config.get_allowed_msrs())?; + (vm, required_mtrrs) + } None => return Err(CreateHyperlightVmError::NoHypervisorFound), }; @@ -136,6 +168,10 @@ impl HyperlightVm { }), }); + // The MSRs reset on restore and captured in snapshots. + let msr_reset = + Self::capture_msr_reset_state(&vm, required_mtrrs, config.get_allowed_msrs())?; + let snapshot_slot = 0u32; let scratch_slot = 1u32; #[cfg_attr(not(gdb), allow(unused_mut))] @@ -168,6 +204,7 @@ impl HyperlightVm { trace_info, #[cfg(crashdump)] rt_cfg, + msr_reset, }; ret.update_snapshot_mapping(snapshot_mem)?; @@ -199,6 +236,50 @@ impl HyperlightVm { Ok(ret) } + /// Determines this VM's MSR reset set: the MSRs reset on restore and + /// captured in snapshots. `required_mtrrs` is the VCNT-driven MTRR set + /// gathered at creation. `allowed` is the validated allow list. + fn capture_msr_reset_state( + vm: &BoxedVm, + required_mtrrs: Vec, + allowed: &[u32], + ) -> std::result::Result { + let core: Vec = match get_available_hypervisor() { + #[cfg(kvm)] + Some(HypervisorType::Kvm) => vec![MSR_KERNEL_GS_BASE, MSR_TSC], // GS_BASE is needed for correctness. TSC is to match mshv/whp behavior + #[cfg(mshv3)] + Some(HypervisorType::Mshv) => Self::probe_core_reset_indices(vm), + #[cfg(target_os = "windows")] + Some(HypervisorType::Whp) => Self::probe_core_reset_indices(vm), + // The VM already exists, so a hypervisor was found. The `None` + // arm in `new` returned before this point. + None => unreachable!(), + }; + let mut indices: Vec = core + .into_iter() + .chain(required_mtrrs) + .chain(allowed.iter().copied()) + .collect(); + indices.sort_unstable(); + indices.dedup(); + let baseline = vm.msrs(&indices).map_err(VmError::Register)?; + let mut allowed = allowed.to_vec(); + allowed.sort_unstable(); + allowed.dedup(); + Ok(MsrResetState::new(baseline, allowed)) + } + + /// Core stateful MSRs a filterless host can read. + /// + /// MTRRs come from the VCNT-driven required set, so they are excluded. + #[cfg(any(mshv3, target_os = "windows"))] + fn probe_core_reset_indices(vm: &BoxedVm) -> Vec { + core_reset_indices() + .filter(|i| !is_mtrr_reset_index(*i)) + .filter(|i| vm.msrs(&[*i]).is_ok()) + .collect() + } + /// Initialise the internally stored vCPU with the given PEB address and /// random number seed, then run it until a HLT instruction. #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] @@ -270,6 +351,54 @@ impl HyperlightVm { Ok(self.vm.sregs()?) } + /// Returns the current values of the MSR reset set. + pub(crate) fn get_msr_reset_state(&self) -> Result, AccessPageTableError> { + Ok(self.vm.msrs(&self.msr_reset.indices())?) + } + + /// Returns this VM's requested allow list, sorted and deduplicated. + pub(crate) fn get_msr_allow_list(&self) -> Vec { + self.msr_reset.allowed().to_vec() + } + + /// Restores snapshot MSRs or the initialization baseline. + pub(crate) fn restore_msrs( + &mut self, + snap_msrs: Option<&Vec>, + snap_allowed: Option<&[u32]>, + ) -> std::result::Result<(), ResetVcpuError> { + match snap_msrs { + // No captured MSRs. Use this VM's baseline. + None => self.vm.set_msrs(self.msr_reset.baseline())?, + // Reset the MSRs to the snapshot's captured values. The snapshot's + // allow list must be a subset of this VM's, and every captured + // index must be in this reset set, so validation rejects a + // mismatch first. + Some(msrs) => { + let entries = self + .msr_reset + .validate_snapshot(msrs, snap_allowed.unwrap_or(&[]))?; + self.vm.set_msrs(&entries)?; + } + } + Ok(()) + } + + /// Reads arbitrary backend MSRs for tests. + #[cfg(all(test, mshv3, target_arch = "x86_64"))] + pub(crate) fn capture_msrs_for_test( + &self, + indices: &[u32], + ) -> std::result::Result, RegisterError> { + self.vm.msrs(indices) + } + + /// Attempts one backend MSR write for tests. + #[cfg(all(test, mshv3, target_arch = "x86_64"))] + pub(crate) fn try_set_msr_for_test(&self, index: u32, value: u64) -> bool { + self.vm.set_msrs(&[MsrEntry { index, value }]).is_ok() + } + /// Dispatch a call from the host to the guest using the given pointer /// to the dispatch function _in the guest's address space_. /// diff --git a/src/hyperlight_host/src/hypervisor/regs/x86_64/mod.rs b/src/hyperlight_host/src/hypervisor/regs/x86_64/mod.rs index 88724d95a..12d65f056 100644 --- a/src/hyperlight_host/src/hypervisor/regs/x86_64/mod.rs +++ b/src/hyperlight_host/src/hypervisor/regs/x86_64/mod.rs @@ -16,11 +16,13 @@ limitations under the License. mod debug_regs; mod fpu; +mod msrs; mod special_regs; mod standard_regs; pub(crate) use debug_regs::*; pub(crate) use fpu::*; +pub(crate) use msrs::*; pub(crate) use special_regs::*; pub(crate) use standard_regs::*; diff --git a/src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs b/src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs new file mode 100644 index 000000000..82782a180 --- /dev/null +++ b/src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs @@ -0,0 +1,483 @@ +/* +Copyright 2025 The Hyperlight Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +//! Model-specific register (MSR) state restored with a snapshot. +//! +//! The reset set contains every MSR whose guest-written state can persist. +//! Allowing guest access to an MSR is a separate concern. Allowed MSRs still +//! reset. + +use serde::{Deserialize, Serialize}; + +/// A single MSR captured for reset. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub(crate) struct MsrEntry { + /// The index passed to `RDMSR` and `WRMSR`. + pub index: u32, + /// The captured value. + pub value: u64, +} + +/// The MSR reset set and initialization baseline for one VM. +#[derive(Debug, Clone)] +pub(crate) struct MsrResetState { + /// Creation-time value for every MSR this VM resets on restore and + /// captures in a snapshot: core indices, required MTRRs, and the allow + /// list. Sorted by index and deduplicated. + baseline: Vec, + /// The user-supplied allow list (`SandboxConfiguration::allow_msrs`), + /// sorted and deduplicated. A subset of the `baseline` indices, kept as a + /// separate copy so restore can compare it, as a superset, against a + /// snapshot's allow list. + allowed: Vec, +} + +impl MsrResetState { + /// Builds the reset state from the creation-time baseline entries and the + /// requested allow list. + pub fn new(baseline: Vec, allowed: Vec) -> Self { + Self { baseline, allowed } + } + + /// The creation-time baseline entries. + pub fn baseline(&self) -> &[MsrEntry] { + &self.baseline + } + + /// This VM's requested allow list. + pub fn allowed(&self) -> &[u32] { + &self.allowed + } + + /// The MSR indices this VM resets, for host reads. + pub fn indices(&self) -> Vec { + self.baseline.iter().map(|entry| entry.index).collect() + } + + /// Resolves an untrusted snapshot's MSRs against this VM's reset set. + /// + /// The snapshot's allow list must be a subset of this VM's, so a + /// destination that allows at least as much accepts the snapshot on every + /// backend. Each supplied index must belong to this reset set. Returns the + /// entries to write: the snapshot's value for each reset index, or the + /// baseline where the snapshot omits it. + pub fn validate_snapshot( + &self, + snapshot: &[MsrEntry], + snap_allowed: &[u32], + ) -> Result, crate::hypervisor::virtual_machine::RegisterError> { + use crate::hypervisor::virtual_machine::RegisterError; + + let missing: Vec = snap_allowed + .iter() + .copied() + .filter(|index| !self.allowed.contains(index)) + .collect(); + if !missing.is_empty() { + return Err(RegisterError::SnapshotMsrNotAllowed { missing }); + } + + for entry in snapshot { + if !self.baseline.iter().any(|base| base.index == entry.index) { + return Err(RegisterError::InvalidSnapshotMsrIndex { index: entry.index }); + } + } + + Ok(self + .baseline + .iter() + .map(|base| MsrEntry { + index: base.index, + value: snapshot + .iter() + .find(|entry| entry.index == base.index) + .map_or(base.value, |entry| entry.value), + }) + .collect()) + } +} + +pub(crate) const MSR_TSC: u32 = 0x10; +pub(crate) const MSR_TSC_ADJUST: u32 = 0x3B; +pub(crate) const MSR_SPEC_CTRL: u32 = 0x48; +pub(crate) const MSR_UMWAIT_CONTROL: u32 = 0xE1; +pub(crate) const MSR_MPERF: u32 = 0xE7; +pub(crate) const MSR_APERF: u32 = 0xE8; +pub(crate) const MSR_MTRR_CAP: u32 = 0xFE; +pub(crate) const MSR_TSX_CTRL: u32 = 0x122; +pub(crate) const MSR_SYSENTER_CS: u32 = 0x174; +pub(crate) const MSR_SYSENTER_ESP: u32 = 0x175; +pub(crate) const MSR_SYSENTER_EIP: u32 = 0x176; +pub(crate) const MSR_XFD: u32 = 0x1C4; +pub(crate) const MSR_XFD_ERR: u32 = 0x1C5; +pub(crate) const MSR_DEBUGCTL: u32 = 0x1D9; +pub(crate) const MSR_MTRR_FIX64K_00000: u32 = 0x250; +pub(crate) const MSR_PAT: u32 = 0x277; +pub(crate) const MSR_MTRR_DEF_TYPE: u32 = 0x2FF; +pub(crate) const MSR_U_CET: u32 = 0x6A0; +pub(crate) const MSR_S_CET: u32 = 0x6A2; +pub(crate) const MSR_PL0_SSP: u32 = 0x6A4; +pub(crate) const MSR_PL1_SSP: u32 = 0x6A5; +pub(crate) const MSR_PL2_SSP: u32 = 0x6A6; +pub(crate) const MSR_PL3_SSP: u32 = 0x6A7; +pub(crate) const MSR_INTERRUPT_SSP_TABLE_ADDR: u32 = 0x6A8; +pub(crate) const MSR_TSC_DEADLINE: u32 = 0x6E0; +pub(crate) const MSR_BNDCFGS: u32 = 0xD90; +pub(crate) const MSR_XSS: u32 = 0xDA0; +pub(crate) const MSR_STAR: u32 = 0xC000_0081; +pub(crate) const MSR_LSTAR: u32 = 0xC000_0082; +pub(crate) const MSR_CSTAR: u32 = 0xC000_0083; +pub(crate) const MSR_SFMASK: u32 = 0xC000_0084; +pub(crate) const MSR_KERNEL_GS_BASE: u32 = 0xC000_0102; +pub(crate) const MSR_TSC_AUX: u32 = 0xC000_0103; +pub(crate) const MSR_VIRT_SPEC_CTRL: u32 = 0xC001_011F; + +const HYPERV_VARIABLE_MTRR_COUNT: u8 = 16; + +// Every guest-writable retained value must be host-readable and host-writable. +// EFER, APIC_BASE, FS_BASE, and GS_BASE are part of the special-register state. +// PRED_CMD (0x49) and FLUSH_CMD (0x10B) are intentionally absent: they are +// write-only commands with no retained state, so they cannot be reset and +// therefore cannot be allowed. +const MSR_TABLE: &[u32] = &[ + // Guest and host access use the matching SYSENTER state. + MSR_SYSENTER_CS, + MSR_SYSENTER_ESP, + MSR_SYSENTER_EIP, + // WHP exposes no writable DEBUGCTL bits under its default feature banks. + MSR_DEBUGCTL, + // Guest and host access use the same PAT state. + MSR_PAT, + // Guest and host access use the matching syscall state. + MSR_STAR, + MSR_LSTAR, + MSR_CSTAR, + MSR_SFMASK, + // SWAPGS and host access use the same KERNEL_GS_BASE state. + MSR_KERNEL_GS_BASE, + // Guest and host access use the same SPEC_CTRL state. + MSR_SPEC_CTRL, + // AMD virtualized SSBD control. Guest-writable only where the host exposes + // the legacy VIRT_SPEC_CTRL SSBD mechanism. Host probing omits it otherwise. + MSR_VIRT_SPEC_CTRL, + // Host probing omits CET state when CET is unavailable. + MSR_U_CET, + MSR_S_CET, + // Host probing omits shadow-stack state when it is unavailable. + MSR_PL0_SSP, + MSR_PL1_SSP, + MSR_PL2_SSP, + MSR_PL3_SSP, + MSR_INTERRUPT_SSP_TABLE_ADDR, + // MSHV maps XSS through its U_XSS alias. + MSR_XSS, + // Guest and host access use the same virtual counter. + MSR_TSC, + // Hyper-V stores TSC_ADJUST independently from TSC. + MSR_TSC_ADJUST, + // Host probing omits TSC_AUX when RDTSCP is unavailable. + MSR_TSC_AUX, + // Hyper-V exposes MPERF and APERF as per-VP counters. + MSR_MPERF, + MSR_APERF, + // Host probing omits TSX_CTRL when TSX control is unavailable. + MSR_TSX_CTRL, + // Host probing omits XFD state when XFD is unavailable. + MSR_XFD, + MSR_XFD_ERR, + // Feature MSRs enabled by default on capable hosts. Host probing omits + // each when its feature is unavailable. + MSR_UMWAIT_CONTROL, + MSR_TSC_DEADLINE, + MSR_BNDCFGS, + // Hyper-V accepts fixed-MTRR writes even when MTRRCAP.FIX is clear. + MSR_MTRR_DEF_TYPE, + 0x200, // PHYSBASE0 + 0x201, // PHYSMASK0 + 0x202, // PHYSBASE1 + 0x203, // PHYSMASK1 + 0x204, // PHYSBASE2 + 0x205, // PHYSMASK2 + 0x206, // PHYSBASE3 + 0x207, // PHYSMASK3 + 0x208, // PHYSBASE4 + 0x209, // PHYSMASK4 + 0x20A, // PHYSBASE5 + 0x20B, // PHYSMASK5 + 0x20C, // PHYSBASE6 + 0x20D, // PHYSMASK6 + 0x20E, // PHYSBASE7 + 0x20F, // PHYSMASK7 + 0x210, // PHYSBASE8 + 0x211, // PHYSMASK8 + 0x212, // PHYSBASE9 + 0x213, // PHYSMASK9 + 0x214, // PHYSBASEA + 0x215, // PHYSMASKA + 0x216, // PHYSBASEB + 0x217, // PHYSMASKB + 0x218, // PHYSBASEC + 0x219, // PHYSMASKC + 0x21A, // PHYSBASED + 0x21B, // PHYSMASKD + 0x21C, // PHYSBASEE + 0x21D, // PHYSMASKE + 0x21E, // PHYSBASEF + 0x21F, // PHYSMASKF + MSR_MTRR_FIX64K_00000, // FIX64K_00000 + 0x258, // FIX16K_80000 + 0x259, // FIX16K_A0000 + 0x268, // FIX4K_C0000 + 0x269, // FIX4K_C8000 + 0x26A, // FIX4K_D0000 + 0x26B, // FIX4K_D8000 + 0x26C, // FIX4K_E0000 + 0x26D, // FIX4K_E8000 + 0x26E, // FIX4K_F0000 + 0x26F, // FIX4K_F8000 +]; + +/// Whether an MSR carries retained state eligible for the reset set. +pub(crate) fn is_resettable_msr(index: u32) -> bool { + MSR_TABLE.contains(&index) +} + +/// Returns core stateful indices for host filtering. +pub(crate) fn core_reset_indices() -> impl Iterator { + MSR_TABLE.iter().copied() +} + +pub(crate) fn hyperv_mtrr_reset_indices( + mtrr_cap: u64, +) -> Result, crate::hypervisor::virtual_machine::CreateVmError> { + use crate::hypervisor::virtual_machine::CreateVmError; + + let advertised = (mtrr_cap & 0xff) as u8; + if advertised > HYPERV_VARIABLE_MTRR_COUNT { + return Err(CreateVmError::UnexpectedVariableMtrrCount { + advertised, + maximum: HYPERV_VARIABLE_MTRR_COUNT, + }); + } + + let mut indices = Vec::with_capacity(1 + usize::from(advertised) * 2 + 11); + indices.push(MSR_MTRR_DEF_TYPE); + indices.extend((0..u32::from(advertised) * 2).map(|offset| 0x200 + offset)); + indices.extend([ + MSR_MTRR_FIX64K_00000, + 0x258, + 0x259, + 0x268, + 0x269, + 0x26A, + 0x26B, + 0x26C, + 0x26D, + 0x26E, + 0x26F, + ]); + Ok(indices) +} + +pub(crate) fn is_mtrr_reset_index(index: u32) -> bool { + index == MSR_MTRR_DEF_TYPE + || (0x200..=0x21F).contains(&index) + || matches!( + index, + MSR_MTRR_FIX64K_00000 + | 0x258 + | 0x259 + | 0x268 + | 0x269 + | 0x26A + | 0x26B + | 0x26C + | 0x26D + | 0x26E + | 0x26F + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::hypervisor::virtual_machine::{CreateVmError, RegisterError}; + + fn state() -> MsrResetState { + MsrResetState { + baseline: vec![ + MsrEntry { + index: MSR_SYSENTER_CS, + value: 0x1C, + }, + MsrEntry { + index: MSR_SYSENTER_ESP, + value: 0x2C, + }, + MsrEntry { + index: MSR_KERNEL_GS_BASE, + value: 0x3C, + }, + ], + allowed: vec![MSR_SYSENTER_CS, MSR_SYSENTER_ESP], + } + } + + #[test] + fn hyperv_mtrr_indices_follow_guest_capability() { + assert_eq!( + hyperv_mtrr_reset_indices(2).unwrap(), + [ + MSR_MTRR_DEF_TYPE, + 0x200, + 0x201, + 0x202, + 0x203, + MSR_MTRR_FIX64K_00000, + 0x258, + 0x259, + 0x268, + 0x269, + 0x26A, + 0x26B, + 0x26C, + 0x26D, + 0x26E, + 0x26F, + ] + ); + let fixed_bit_does_not_change_reset_set = hyperv_mtrr_reset_indices(2 | (1 << 8)).unwrap(); + assert_eq!( + fixed_bit_does_not_change_reset_set, + hyperv_mtrr_reset_indices(2).unwrap() + ); + } + + #[test] + fn hyperv_mtrr_count_rejects_more_than_sixteen_pairs() { + let indices = hyperv_mtrr_reset_indices(16).unwrap(); + assert!(indices.contains(&0x21F)); + assert_eq!(indices.last(), Some(&0x26F)); + assert!(indices.iter().all(|&index| is_resettable_msr(index))); + assert!(matches!( + hyperv_mtrr_reset_indices(17), + Err(CreateVmError::UnexpectedVariableMtrrCount { + advertised: 17, + maximum: 16 + }) + )); + } + + #[test] + fn snapshot_msr_validation_accepts_exact_canonical_set() { + let supplied = vec![ + MsrEntry { + index: MSR_SYSENTER_CS, + value: 1, + }, + MsrEntry { + index: MSR_SYSENTER_ESP, + value: 2, + }, + MsrEntry { + index: MSR_KERNEL_GS_BASE, + value: 3, + }, + ]; + + assert_eq!( + state() + .validate_snapshot(&supplied, &[MSR_SYSENTER_CS]) + .unwrap(), + supplied + ); + } + + #[test] + fn snapshot_msr_validation_baselines_omitted_indices() { + // A snapshot covering a subset of this VM's reset set is accepted. + // The omitted index takes the creation-time baseline value. + let supplied = vec![ + MsrEntry { + index: MSR_SYSENTER_CS, + value: 1, + }, + MsrEntry { + index: MSR_KERNEL_GS_BASE, + value: 3, + }, + ]; + + assert_eq!( + state().validate_snapshot(&supplied, &[]).unwrap(), + vec![ + MsrEntry { + index: MSR_SYSENTER_CS, + value: 1 + }, + MsrEntry { + index: MSR_SYSENTER_ESP, + value: 0x2C + }, + MsrEntry { + index: MSR_KERNEL_GS_BASE, + value: 3 + }, + ] + ); + } + + #[test] + fn snapshot_msr_validation_rejects_non_superset_allow_list() { + // The snapshot allows an MSR the destination does not. + assert!(matches!( + state().validate_snapshot(&[], &[MSR_KERNEL_GS_BASE]), + Err(RegisterError::SnapshotMsrNotAllowed { missing }) if missing == vec![MSR_KERNEL_GS_BASE] + )); + } + + #[test] + fn snapshot_msr_validation_rejects_index_outside_reset_set() { + let supplied = vec![ + MsrEntry { + index: MSR_SYSENTER_CS, + value: 1, + }, + MsrEntry { + index: MSR_SYSENTER_ESP, + value: 2, + }, + MsrEntry { + index: MSR_PAT, + value: 3, + }, + ]; + + assert!(matches!( + state().validate_snapshot(&supplied, &[]), + Err(RegisterError::InvalidSnapshotMsrIndex { index }) if index == MSR_PAT + )); + } + + #[test] + fn snapshot_msr_validation_accepts_empty_canonical_set() { + let state = MsrResetState { + baseline: Vec::new(), + allowed: Vec::new(), + }; + assert!(state.validate_snapshot(&[], &[]).unwrap().is_empty()); + } +} diff --git a/src/hyperlight_host/src/hypervisor/regs/x86_64/special_regs.rs b/src/hyperlight_host/src/hypervisor/regs/x86_64/special_regs.rs index e67ab3b49..adecfb24a 100644 --- a/src/hyperlight_host/src/hypervisor/regs/x86_64/special_regs.rs +++ b/src/hyperlight_host/src/hypervisor/regs/x86_64/special_regs.rs @@ -677,6 +677,16 @@ mod tests { assert_eq!(original, roundtrip); } + /// The guest boots in xAPIC mode. `APIC_BASE` bit 10 (`EXTD`) enables + /// x2APIC, which would make the `0x800`-`0x8FF` MSR range legal. Keeping it + /// clear is why a guest write to an x2APIC MSR faults. + #[test] + fn standard_defaults_leave_x2apic_disabled() { + const X2APIC_ENABLE: u64 = 1 << 10; + let sregs = CommonSpecialRegisters::standard_64bit_defaults(0x1000); + assert_eq!(sregs.apic_base & X2APIC_ENABLE, 0); + } + #[cfg(mshv3)] #[test] fn round_trip_mshv_sregs() { diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs index 3dc8ec87a..87f9f111e 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs @@ -14,16 +14,21 @@ See the License for the specific language governing permissions and limitations under the License. */ +use std::collections::HashSet; use std::sync::LazyLock; use hyperlight_common::outb::VmAction; #[cfg(gdb)] use kvm_bindings::kvm_guest_debug; use kvm_bindings::{ - kvm_debugregs, kvm_fpu, kvm_regs, kvm_sregs, kvm_userspace_memory_region, kvm_xsave, + Msrs, kvm_debugregs, kvm_enable_cap, kvm_fpu, kvm_msr_entry, kvm_regs, kvm_sregs, + kvm_userspace_memory_region, kvm_xsave, }; use kvm_ioctls::Cap::UserMemory; -use kvm_ioctls::{Kvm, VcpuExit, VcpuFd, VmFd}; +use kvm_ioctls::{ + Cap, Kvm, MsrExitReason, MsrFilterDefaultAction, MsrFilterRange, MsrFilterRangeFlags, VcpuExit, + VcpuFd, VmFd, +}; use tracing::{Span, instrument}; #[cfg(feature = "trace_guest")] use tracing_opentelemetry::OpenTelemetrySpanExt; @@ -34,7 +39,7 @@ use vmm_sys_util::eventfd::EventFd; use crate::hypervisor::gdb::{DebugError, DebuggableVm}; use crate::hypervisor::regs::{ CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, FP_CONTROL_WORD_DEFAULT, - MXCSR_DEFAULT, + MXCSR_DEFAULT, MsrEntry, is_resettable_msr, }; #[cfg(test)] use crate::hypervisor::virtual_machine::XSAVE_BUFFER_SIZE; @@ -110,6 +115,42 @@ pub(crate) struct KvmVm { static KVM: LazyLock> = LazyLock::new(|| Kvm::new().map_err(|e| CreateVmError::HypervisorNotAvailable(e.into()))); +/// Cached host indices reported by `KVM_GET_MSR_INDEX_LIST`. +/// An empty set makes support checks fail closed. +static HOST_MSR_INDICES: LazyLock> = LazyLock::new(|| match KVM.as_ref() { + Ok(kvm) => match kvm.get_msr_index_list() { + Ok(list) => list.as_slice().iter().copied().collect(), + Err(e) => { + tracing::warn!("KVM_GET_MSR_INDEX_LIST failed: {e}"); + HashSet::new() + } + }, + Err(_) => HashSet::new(), +}); + +/// Returns the set of MSR indices the host KVM supports for get/set. +pub(crate) fn host_msr_indices() -> &'static HashSet { + &HOST_MSR_INDICES +} + +/// KVM allows at most this many MSR filter ranges. +const KVM_MSR_FILTER_MAX_RANGES: usize = 16; + +/// Returns the smallest contiguous ranges covering the supplied indices. +fn coalesce_msr_ranges(indices: &[u32]) -> Vec<(u32, usize)> { + let mut sorted: Vec = indices.to_vec(); + sorted.sort_unstable(); + sorted.dedup(); + let mut groups: Vec<(u32, usize)> = Vec::new(); + for idx in sorted { + match groups.last_mut() { + Some((base, count)) if *base + *count as u32 == idx => *count += 1, + _ => groups.push((idx, 1)), + } + } + groups +} + #[cfg(feature = "hw-interrupts")] impl KvmVm { /// Create the in-kernel IRQ chip and register an irqfd for GSI 0. @@ -228,6 +269,20 @@ impl KvmVm { } Ok(VcpuExit::MmioRead(addr, _)) => return Ok(VmExit::MmioRead(addr)), Ok(VcpuExit::MmioWrite(addr, _)) => return Ok(VmExit::MmioWrite(addr)), + // Complete filtered access through the default run path. + Ok(VcpuExit::X86Rdmsr(msr_exit)) => { + let msr_index = msr_exit.index; + *msr_exit.error = 1; + self.complete_filtered_msr_exit()?; + return Ok(VmExit::MsrRead(msr_index)); + } + Ok(VcpuExit::X86Wrmsr(msr_exit)) => { + let msr_index = msr_exit.index; + let value = msr_exit.data; + *msr_exit.error = 1; + self.complete_filtered_msr_exit()?; + return Ok(VmExit::MsrWrite { msr_index, value }); + } #[cfg(gdb)] Ok(VcpuExit::Debug(debug_exit)) => { return Ok(VmExit::Debug { @@ -250,6 +305,23 @@ impl KvmVm { } } + /// Finish a filtered MSR access after its `error` flag was set. + /// Reentering `KVM_RUN` injects `#GP` for the pending access. + /// immediate_exit halts the guest right after, so it never runs past + /// the fault. That reentry returns `EINTR`, which is expected here. + fn complete_filtered_msr_exit(&mut self) -> std::result::Result<(), RunVcpuError> { + self.vcpu_fd.set_kvm_immediate_exit(1); + // `.err()` drops the `Ok(VcpuExit)`, which borrows `vcpu_fd`, keeping + // only the owned error so the borrow ends before the next call. + let err = self.vcpu_fd.run().err(); + self.vcpu_fd.set_kvm_immediate_exit(0); + match err { + None => Ok(()), + Some(e) if e.errno() == libc::EINTR => Ok(()), + Some(e) => Err(RunVcpuError::Unknown(e.into())), + } + } + #[cfg(feature = "hw-interrupts")] fn handle_pv_timer_config(&mut self, data: &[u8]) { use super::super::x86_64::hw_interrupts::handle_pv_timer_config; @@ -275,6 +347,21 @@ impl KvmVm { Ok(VcpuExit::IoOut(port, data)) => Ok(VmExit::IoOut(port, data.to_vec())), Ok(VcpuExit::MmioRead(addr, _)) => Ok(VmExit::MmioRead(addr)), Ok(VcpuExit::MmioWrite(addr, _)) => Ok(VmExit::MmioWrite(addr)), + // Reentering KVM_RUN completes the failed MSR exit and injects #GP. + // immediate_exit prevents further guest execution. + Ok(VcpuExit::X86Rdmsr(msr_exit)) => { + let msr_index = msr_exit.index; + *msr_exit.error = 1; + self.complete_filtered_msr_exit()?; + Ok(VmExit::MsrRead(msr_index)) + } + Ok(VcpuExit::X86Wrmsr(msr_exit)) => { + let msr_index = msr_exit.index; + let value = msr_exit.data; + *msr_exit.error = 1; + self.complete_filtered_msr_exit()?; + Ok(VmExit::MsrWrite { msr_index, value }) + } #[cfg(gdb)] Ok(VcpuExit::Debug(debug_exit)) => Ok(VmExit::Debug { dr6: debug_exit.dr6, @@ -292,6 +379,150 @@ impl KvmVm { ))), } } + + /// Installs a deny filter containing the validated allow list. + /// Requires `KVM_CAP_X86_USER_SPACE_MSR` and `KVM_CAP_X86_MSR_FILTER`. + pub(crate) fn configure_msr_access( + &self, + allowed: &[u32], + ) -> std::result::Result<(), CreateVmError> { + let hv = KVM.as_ref().map_err(|e| e.clone())?; + if !hv.check_extension(Cap::X86UserSpaceMsr) || !hv.check_extension(Cap::X86MsrFilter) { + tracing::error!( + "KVM does not support KVM_CAP_X86_USER_SPACE_MSR or KVM_CAP_X86_MSR_FILTER." + ); + return Err(CreateVmError::MsrFilterNotSupported); + } + + // Every permitted guest write must have restorable host state. + for &msr in allowed { + self.validate_allowed_msr(msr)?; + } + + // Tell KVM to exit to userspace on filtered MSR access. + let cap = kvm_enable_cap { + cap: Cap::X86UserSpaceMsr as u32, + args: [MsrExitReason::Filter.bits() as u64, 0, 0, 0], + ..Default::default() + }; + self.vm_fd + .enable_cap(&cap) + .map_err(|e| CreateVmError::InitializeVm(e.into()))?; + + // Each contiguous group consumes one KVM filter range. + let groups = coalesce_msr_ranges(allowed); + if groups.len() > KVM_MSR_FILTER_MAX_RANGES { + return Err(CreateVmError::TooManyMsrRanges(groups.len())); + } + + // The bitmaps must live through set_msr_filter. + let bitmaps: Vec> = groups + .iter() + .map(|(_, count)| { + let mut bytes = vec![0u8; count.div_ceil(8)]; + for bit in 0..*count { + bytes[bit / 8] |= 1 << (bit % 8); + } + bytes + }) + .collect(); + + // Default deny requires at least one range. + static DENY_BITMAP: [u8; 1] = [0u8]; + let ranges: Vec = if groups.is_empty() { + vec![MsrFilterRange { + flags: MsrFilterRangeFlags::READ | MsrFilterRangeFlags::WRITE, + base: 0, + msr_count: 1, + bitmap: &DENY_BITMAP, + }] + } else { + groups + .iter() + .zip(bitmaps.iter()) + .map(|((base, count), bitmap)| MsrFilterRange { + flags: MsrFilterRangeFlags::READ | MsrFilterRangeFlags::WRITE, + base: *base, + msr_count: *count as u32, + bitmap: bitmap.as_slice(), + }) + .collect() + }; + + self.vm_fd + .set_msr_filter(MsrFilterDefaultAction::DENY, &ranges) + .map_err(|e| CreateVmError::InitializeVm(e.into()))?; + Ok(()) + } + + /// Validates that an allowed MSR has restorable host state. + fn validate_allowed_msr(&self, msr: u32) -> std::result::Result<(), CreateVmError> { + if !is_resettable_msr(msr) { + return Err(CreateVmError::MsrNotAllowable { + msr, + reason: "MSR is not a resettable MSR".to_string(), + }); + } + if !host_msr_indices().contains(&msr) { + return Err(CreateVmError::MsrNotAllowable { + msr, + reason: "MSR is not supported by the host".to_string(), + }); + } + let value = self + .read_msr(msr) + .map_err(|e| CreateVmError::MsrNotAllowable { + msr, + reason: format!("MSR is not readable: {e}"), + })?; + self.write_msr(msr, value) + .map_err(|e| CreateVmError::MsrNotAllowable { + msr, + reason: format!("MSR is not resettable: {e}"), + })?; + Ok(()) + } + + /// Reads one vCPU MSR without the guest filter. + fn read_msr(&self, index: u32) -> std::result::Result { + let mut msrs = Msrs::from_entries(&[kvm_msr_entry { + index, + ..Default::default() + }]) + .map_err(|e| RegisterError::MsrBuild(format!("{e:?}")))?; + let n = self + .vcpu_fd + .get_msrs(&mut msrs) + .map_err(|e| RegisterError::GetMsrs(e.into()))?; + if n != 1 { + return Err(RegisterError::MsrShortCount { + expected: 1, + actual: n, + }); + } + Ok(msrs.as_slice()[0].data) + } + + /// Writes one vCPU MSR without the guest filter. + fn write_msr(&self, index: u32, data: u64) -> std::result::Result<(), RegisterError> { + let msrs = Msrs::from_entries(&[kvm_msr_entry { + index, + data, + ..Default::default() + }]) + .map_err(|e| RegisterError::MsrBuild(format!("{e:?}")))?; + let n = self + .vcpu_fd + .set_msrs(&msrs) + .map_err(|e| RegisterError::SetMsrs(e.into()))?; + if n != 1 { + return Err(RegisterError::MsrShortCount { + expected: 1, + actual: n, + }); + } + Ok(()) + } } impl VirtualMachine for KvmVm { @@ -403,6 +634,66 @@ impl VirtualMachine for KvmVm { Ok(()) } + fn msrs(&self, indices: &[u32]) -> std::result::Result, RegisterError> { + if indices.is_empty() { + return Ok(Vec::new()); + } + let entries: Vec = indices + .iter() + .map(|&index| kvm_msr_entry { + index, + ..Default::default() + }) + .collect(); + let mut msrs = + Msrs::from_entries(&entries).map_err(|e| RegisterError::MsrBuild(format!("{e:?}")))?; + let n = self + .vcpu_fd + .get_msrs(&mut msrs) + .map_err(|e| RegisterError::GetMsrs(e.into()))?; + if n != indices.len() { + return Err(RegisterError::MsrShortCount { + expected: indices.len(), + actual: n, + }); + } + Ok(msrs + .as_slice() + .iter() + .map(|e| MsrEntry { + index: e.index, + value: e.data, + }) + .collect()) + } + + fn set_msrs(&self, msrs: &[MsrEntry]) -> std::result::Result<(), RegisterError> { + let entries: Vec = msrs + .iter() + .map(|e| kvm_msr_entry { + index: e.index, + data: e.value, + ..Default::default() + }) + .collect(); + if entries.is_empty() { + return Ok(()); + } + let kvm_msrs = + Msrs::from_entries(&entries).map_err(|e| RegisterError::MsrBuild(format!("{e:?}")))?; + let n = self + .vcpu_fd + .set_msrs(&kvm_msrs) + .map_err(|e| RegisterError::SetMsrs(e.into()))?; + if n != entries.len() { + return Err(RegisterError::MsrShortCount { + expected: entries.len(), + actual: n, + }); + } + Ok(()) + } + #[allow(dead_code)] fn xsave(&self) -> std::result::Result, RegisterError> { let xsave = self @@ -570,10 +861,45 @@ impl DebuggableVm for KvmVm { } #[cfg(test)] -#[cfg(feature = "hw-interrupts")] -mod hw_interrupt_tests { +mod tests { use super::*; + #[test] + fn coalesces_unsorted_contiguous_indices() { + assert_eq!( + coalesce_msr_ranges(&[0x176, 0x174, 0x175]), + vec![(0x174, 3)] + ); + } + + #[test] + fn deduplicates_indices() { + assert_eq!( + coalesce_msr_ranges(&[0x174, 0x174, 0x176]), + vec![(0x174, 1), (0x176, 1)] + ); + } + + #[test] + fn preserves_sixteen_range_boundary() { + let indices: Vec = (0..KVM_MSR_FILTER_MAX_RANGES as u32) + .map(|index| index * 2) + .collect(); + let ranges = coalesce_msr_ranges(&indices); + assert_eq!(ranges.len(), KVM_MSR_FILTER_MAX_RANGES); + assert!(ranges.iter().all(|(_, count)| *count == 1)); + } + + #[test] + fn scattered_indices_exceed_sixteen_range_limit() { + let indices: Vec = (0..=KVM_MSR_FILTER_MAX_RANGES as u32) + .map(|index| index * 2) + .collect(); + let ranges = coalesce_msr_ranges(&indices); + assert!(ranges.len() > KVM_MSR_FILTER_MAX_RANGES); + } + + #[cfg(feature = "hw-interrupts")] #[test] fn halt_port_is_not_standard_device() { // VmAction::Halt port must not overlap in-kernel PIC/PIT/speaker ports diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs index dac344711..4cd8fca02 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs @@ -21,9 +21,13 @@ use tracing::{Span, instrument}; #[cfg(gdb)] use crate::hypervisor::gdb::DebugError; +#[cfg(target_arch = "x86_64")] +use crate::hypervisor::regs::MsrEntry; use crate::hypervisor::regs::{ CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, }; +#[cfg(all(target_arch = "x86_64", any(mshv3, target_os = "windows")))] +use crate::hypervisor::regs::{MSR_MTRR_CAP, hyperv_mtrr_reset_indices, is_resettable_msr}; use crate::mem::memory_region::MemoryRegion; #[cfg(feature = "trace_guest")] use crate::sandbox::trace::TraceContext as SandboxTraceContext; @@ -139,6 +143,12 @@ pub(crate) enum VmExit { MmioRead(u64), /// The vCPU tried to write to the given (unmapped) addr MmioWrite(u64), + /// The vCPU tried to read from the given MSR + #[cfg(all(target_arch = "x86_64", kvm))] + MsrRead(u32), + /// The vCPU tried to write to the given MSR with the given value + #[cfg(all(target_arch = "x86_64", kvm))] + MsrWrite { msr_index: u32, value: u64 }, /// The vCPU execution has been cancelled Cancelled(), /// The vCPU has exited for a reason that is not handled by Hyperlight @@ -183,6 +193,30 @@ pub enum CreateVmError { HypervisorNotAvailable(HypervisorError), #[error("Initialize VM failed: {0}")] InitializeVm(HypervisorError), + #[cfg(all(kvm, target_arch = "x86_64"))] + #[error("KVM MSR filtering requires KVM_CAP_X86_USER_SPACE_MSR and KVM_CAP_X86_MSR_FILTER")] + MsrFilterNotSupported, + #[cfg(target_arch = "x86_64")] + #[error("MSR {msr:#x} cannot be allowed: {reason}")] + MsrNotAllowable { msr: u32, reason: String }, + #[cfg(target_arch = "x86_64")] + #[error("Failed to read IA32_MTRRCAP: {0}")] + GetMtrrCap(RegisterError), + #[cfg(target_arch = "x86_64")] + #[error("Guest-visible MTRRs cannot be reset: {0}")] + RequiredMtrrsNotResettable(RegisterError), + #[cfg(target_arch = "x86_64")] + #[error("Guest exposes {advertised} variable MTRR pairs, expected at most {maximum}")] + UnexpectedVariableMtrrCount { advertised: u8, maximum: u8 }, + #[cfg(all(kvm, target_arch = "x86_64"))] + #[error("Too many allowed MSR filter ranges: {0}. Maximum is 16")] + TooManyMsrRanges(usize), + #[cfg(target_os = "windows")] + #[error("Get Partition Property failed: {0}")] + GetPartitionProperty(HypervisorError), + #[cfg(target_os = "windows")] + #[error("WHP exposes {advertised} processor feature banks, expected {expected}")] + UnexpectedProcessorFeatureBankCount { advertised: u32, expected: u32 }, #[error("Set Partition Property failed: {0}")] SetPartitionProperty(HypervisorError), #[cfg(target_os = "windows")] @@ -241,6 +275,38 @@ pub enum RegisterError { }, #[error("Invalid xsave alignment")] InvalidXsaveAlignment, + #[cfg(target_arch = "x86_64")] + #[error("MSR operation not supported on this hypervisor")] + MsrsUnsupported, + #[cfg(target_arch = "x86_64")] + #[error("Failed to build MSR list: {0}")] + MsrBuild(String), + #[cfg(target_arch = "x86_64")] + #[error("Failed to get MSRs: {0}")] + GetMsrs(HypervisorError), + #[cfg(target_arch = "x86_64")] + #[error("Failed to set MSRs: {0}")] + SetMsrs(HypervisorError), + #[cfg(target_arch = "x86_64")] + #[error("Snapshot allows MSRs the destination does not: {missing:x?}")] + SnapshotMsrNotAllowed { + /// Allowed MSR indices present in the snapshot but not the destination. + missing: Vec, + }, + #[cfg(target_arch = "x86_64")] + #[error("Snapshot MSR index {index:#x} is not in this VM's reset set")] + InvalidSnapshotMsrIndex { + /// Architectural MSR index supplied by the snapshot. + index: u32, + }, + #[cfg(all(kvm, target_arch = "x86_64"))] + #[error("MSR batch short count: expected {expected}, applied {actual}")] + MsrShortCount { + /// Number of MSRs requested + expected: usize, + /// Number of MSRs actually applied before KVM stopped + actual: usize, + }, #[cfg(target_os = "windows")] #[error("Failed to get xsave size: {0}")] GetXsaveSize(#[from] HypervisorError), @@ -359,6 +425,13 @@ pub(crate) trait VirtualMachine: Debug + Send { #[allow(dead_code)] fn set_debug_regs(&self, drs: &CommonDebugRegs) -> std::result::Result<(), RegisterError>; + /// Reads the requested MSRs. + #[cfg(target_arch = "x86_64")] + fn msrs(&self, indices: &[u32]) -> std::result::Result, RegisterError>; + /// Writes the supplied MSRs. + #[cfg(target_arch = "x86_64")] + fn set_msrs(&self, msrs: &[MsrEntry]) -> std::result::Result<(), RegisterError>; + /// Get xsave #[allow(dead_code)] #[cfg(not(target_arch = "aarch64"))] @@ -385,6 +458,51 @@ pub(crate) trait VirtualMachine: Debug + Send { fn partition_handle(&self) -> windows::Win32::System::Hypervisor::WHV_PARTITION_HANDLE; } +/// Validates that each allowed MSR is restorable on a filterless (MSHV/WHP) +/// host: reset replays a captured value, so the host must read and write it. +/// Rejects e.g. a variable MTRR above the host VCNT. +#[cfg(all(target_arch = "x86_64", any(mshv3, target_os = "windows")))] +pub(crate) fn validate_allowed_msrs( + vm: &dyn VirtualMachine, + allowed: &[u32], +) -> std::result::Result<(), CreateVmError> { + for &msr in allowed { + if !is_resettable_msr(msr) { + return Err(CreateVmError::MsrNotAllowable { + msr, + reason: "MSR is not a resettable MSR".to_string(), + }); + } + let captured = vm + .msrs(&[msr]) + .map_err(|_| CreateVmError::MsrNotAllowable { + msr, + reason: "MSR cannot be read on this host".to_string(), + })?; + vm.set_msrs(&captured) + .map_err(|_| CreateVmError::MsrNotAllowable { + msr, + reason: "MSR cannot be written on this host".to_string(), + })?; + } + Ok(()) +} + +/// Returns every guest-visible MTRR a filterless (MSHV/WHP) host must reset. +#[cfg(all(target_arch = "x86_64", any(mshv3, target_os = "windows")))] +pub(crate) fn mtrr_reset_indices( + vm: &dyn VirtualMachine, +) -> std::result::Result, CreateVmError> { + let mtrr_cap = vm + .msrs(&[MSR_MTRR_CAP]) + .map_err(CreateVmError::GetMtrrCap)?[0] + .value; + let indices = hyperv_mtrr_reset_indices(mtrr_cap)?; + vm.msrs(&indices) + .map_err(CreateVmError::RequiredMtrrsNotResettable)?; + Ok(indices) +} + #[cfg(test)] mod tests { diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs index 1fd50d29a..88f2ae1f3 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs @@ -31,8 +31,10 @@ use mshv_bindings::{ hv_message_type_HVMSG_X64_HALT, hv_message_type_HVMSG_X64_IO_PORT_INTERCEPT, hv_partition_property_code_HV_PARTITION_PROPERTY_SYNTHETIC_PROC_FEATURES, hv_partition_synthetic_processor_features, hv_register_assoc, - hv_register_name_HV_X64_REGISTER_RIP, hv_register_value, mshv_create_partition_v2, - mshv_user_mem_region, + hv_register_name_HV_X64_REGISTER_MSR_MTRR_PHYS_BASE0, + hv_register_name_HV_X64_REGISTER_MSR_MTRR_PHYS_MASK0, hv_register_name_HV_X64_REGISTER_RIP, + hv_register_name_HV_X64_REGISTER_U_XSS, hv_register_value, mshv_create_partition_v2, + mshv_user_mem_region, msr_to_hv_reg_name as mshv_msr_to_hv_reg_name, }; #[cfg(feature = "hw-interrupts")] use mshv_bindings::{ @@ -50,7 +52,8 @@ use tracing_opentelemetry::OpenTelemetrySpanExt; use crate::hypervisor::gdb::{DebugError, DebuggableVm}; use crate::hypervisor::regs::{ CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, FP_CONTROL_WORD_DEFAULT, - MXCSR_DEFAULT, + MSR_APERF, MSR_BNDCFGS, MSR_MPERF, MSR_TSC_DEADLINE, MSR_TSX_CTRL, MSR_UMWAIT_CONTROL, + MSR_VIRT_SPEC_CTRL, MSR_XFD, MSR_XFD_ERR, MSR_XSS, MXCSR_DEFAULT, MsrEntry, }; #[cfg(test)] use crate::hypervisor::virtual_machine::XSAVE_BUFFER_SIZE; @@ -77,6 +80,84 @@ pub(crate) fn is_hypervisor_present() -> bool { } } +/// Maps an MSR index to the Hyper-V register used for host reset. +fn msr_to_hv_register_name(index: u32) -> Result { + const HV_X64_REGISTER_BNDCFGS: u32 = 0x0008_007C; + const HV_X64_REGISTER_MCOUNT: u32 = 0x0008_007E; + const HV_X64_REGISTER_A_COUNT: u32 = 0x0008_007F; + const HV_X64_REGISTER_TSX_CTRL: u32 = 0x0008_0088; + const HV_X64_REGISTER_TSC_DEADLINE: u32 = 0x0008_0095; + const HV_X64_REGISTER_UMWAIT_CONTROL: u32 = 0x0008_0098; + const HV_X64_REGISTER_XFD: u32 = 0x0008_0099; + const HV_X64_REGISTER_XFD_ERR: u32 = 0x0008_009A; + const HV_X64_REGISTER_VIRT_SPEC_CTRL: u32 = 0x0008_0086; + + if (0x200..=0x21F).contains(&index) { + let pair = (index - 0x200) / 2; + return Ok(if index & 1 == 0 { + hv_register_name_HV_X64_REGISTER_MSR_MTRR_PHYS_BASE0 + pair + } else { + hv_register_name_HV_X64_REGISTER_MSR_MTRR_PHYS_MASK0 + pair + }); + } + if index == MSR_XSS { + return Ok(hv_register_name_HV_X64_REGISTER_U_XSS); + } + if index == MSR_MPERF { + return Ok(HV_X64_REGISTER_MCOUNT); + } + if index == MSR_APERF { + return Ok(HV_X64_REGISTER_A_COUNT); + } + if index == MSR_TSX_CTRL { + return Ok(HV_X64_REGISTER_TSX_CTRL); + } + if index == MSR_TSC_DEADLINE { + return Ok(HV_X64_REGISTER_TSC_DEADLINE); + } + if index == MSR_UMWAIT_CONTROL { + return Ok(HV_X64_REGISTER_UMWAIT_CONTROL); + } + if index == MSR_BNDCFGS { + return Ok(HV_X64_REGISTER_BNDCFGS); + } + if index == MSR_XFD { + return Ok(HV_X64_REGISTER_XFD); + } + if index == MSR_XFD_ERR { + return Ok(HV_X64_REGISTER_XFD_ERR); + } + if index == MSR_VIRT_SPEC_CTRL { + return Ok(HV_X64_REGISTER_VIRT_SPEC_CTRL); + } + mshv_msr_to_hv_reg_name(index) +} + +#[cfg(test)] +mod msr_mapping_tests { + use super::*; + use crate::hypervisor::regs::core_reset_indices; + + #[test] + fn maps_all_stateful_msrs() { + for index in core_reset_indices() { + assert!( + msr_to_hv_register_name(index).is_ok(), + "missing MSR mapping for {index:#x}" + ); + } + assert_eq!(msr_to_hv_register_name(MSR_MPERF), Ok(0x0008_007E)); + assert_eq!(msr_to_hv_register_name(MSR_APERF), Ok(0x0008_007F)); + assert_eq!(msr_to_hv_register_name(MSR_TSX_CTRL), Ok(0x0008_0088)); + assert_eq!(msr_to_hv_register_name(MSR_TSC_DEADLINE), Ok(0x0008_0095)); + assert_eq!(msr_to_hv_register_name(MSR_UMWAIT_CONTROL), Ok(0x0008_0098)); + assert_eq!(msr_to_hv_register_name(MSR_BNDCFGS), Ok(0x0008_007C)); + assert_eq!(msr_to_hv_register_name(MSR_XFD), Ok(0x0008_0099)); + assert_eq!(msr_to_hv_register_name(MSR_XFD_ERR), Ok(0x0008_009A)); + assert_eq!(msr_to_hv_register_name(MSR_VIRT_SPEC_CTRL), Ok(0x0008_0086)); + } +} + /// A MSHV implementation of a single-vcpu VM #[derive(Debug)] pub(crate) struct MshvVm { @@ -102,13 +183,13 @@ impl MshvVm { let mshv = MSHV.as_ref().map_err(|e| e.clone())?; #[allow(unused_mut)] - let mut pr: mshv_create_partition_v2 = Default::default(); + let mut pr = mshv_create_partition_v2::default(); // Enable LAPIC for hw-interrupts — required for interrupt delivery // via request_virtual_interrupt. #[cfg(feature = "hw-interrupts")] { use mshv_bindings::MSHV_PT_BIT_LAPIC; - pr.pt_flags = 1u64 << MSHV_PT_BIT_LAPIC; + pr.pt_flags |= 1u64 << MSHV_PT_BIT_LAPIC; } // It's important to use create_vm_with_args() (not create_vm()), // because create_vm() sets up a SynIC partition by default. @@ -432,6 +513,55 @@ impl VirtualMachine for MshvVm { Ok(()) } + fn msrs(&self, indices: &[u32]) -> std::result::Result, RegisterError> { + if indices.is_empty() { + return Ok(Vec::new()); + } + let mut registers: Vec = indices + .iter() + .map(|&index| { + Ok(hv_register_assoc { + name: msr_to_hv_register_name(index) + .map_err(|_| RegisterError::MsrsUnsupported)?, + ..Default::default() + }) + }) + .collect::>()?; + self.vcpu_fd + .get_reg(&mut registers) + .map_err(|e| RegisterError::GetMsrs(e.into()))?; + Ok(registers + .iter() + .zip(indices) + .map(|(register, &index)| MsrEntry { + index, + // SAFETY: get_reg initialized each association as a 64-bit register. + value: unsafe { register.value.reg64 }, + }) + .collect()) + } + + fn set_msrs(&self, msrs: &[MsrEntry]) -> std::result::Result<(), RegisterError> { + let registers: Vec = msrs + .iter() + .map(|entry| { + Ok(hv_register_assoc { + name: msr_to_hv_register_name(entry.index) + .map_err(|_| RegisterError::MsrsUnsupported)?, + value: hv_register_value { reg64: entry.value }, + ..Default::default() + }) + }) + .collect::>()?; + if registers.is_empty() { + return Ok(()); + } + self.vcpu_fd + .set_reg(®isters) + .map_err(|e| RegisterError::SetMsrs(e.into()))?; + Ok(()) + } + #[allow(dead_code)] fn xsave(&self) -> std::result::Result, RegisterError> { let xsave = self diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs index 6de2b29f1..7f3ab0656 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs @@ -33,9 +33,13 @@ use windows_result::HRESULT; use crate::hypervisor::gdb::{DebugError, DebuggableVm}; use crate::hypervisor::regs::{ Align16, CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, - FP_CONTROL_WORD_DEFAULT, MXCSR_DEFAULT, WHP_DEBUG_REGS_NAMES, WHP_DEBUG_REGS_NAMES_LEN, - WHP_FPU_NAMES, WHP_FPU_NAMES_LEN, WHP_REGS_NAMES, WHP_REGS_NAMES_LEN, WHP_SREGS_NAMES, - WHP_SREGS_NAMES_LEN, + FP_CONTROL_WORD_DEFAULT, MSR_APERF, MSR_BNDCFGS, MSR_CSTAR, MSR_INTERRUPT_SSP_TABLE_ADDR, + MSR_KERNEL_GS_BASE, MSR_LSTAR, MSR_MPERF, MSR_MTRR_CAP, MSR_PAT, MSR_PL0_SSP, MSR_PL1_SSP, + MSR_PL2_SSP, MSR_PL3_SSP, MSR_S_CET, MSR_SFMASK, MSR_SPEC_CTRL, MSR_STAR, MSR_SYSENTER_CS, + MSR_SYSENTER_EIP, MSR_SYSENTER_ESP, MSR_TSC_DEADLINE, MSR_TSX_CTRL, MSR_U_CET, + MSR_UMWAIT_CONTROL, MSR_XFD, MSR_XFD_ERR, MXCSR_DEFAULT, MsrEntry, WHP_DEBUG_REGS_NAMES, + WHP_DEBUG_REGS_NAMES_LEN, WHP_FPU_NAMES, WHP_FPU_NAMES_LEN, WHP_REGS_NAMES, WHP_REGS_NAMES_LEN, + WHP_SREGS_NAMES, WHP_SREGS_NAMES_LEN, }; use crate::hypervisor::surrogate_process::SurrogateProcess; use crate::hypervisor::surrogate_process_manager::{ @@ -73,6 +77,143 @@ pub(crate) fn is_hypervisor_present() -> bool { } } +/// Maps an MSR index to the WHP register used for host reset. +fn msr_to_whv_register_name(index: u32) -> Option { + Some(match index { + MSR_MTRR_CAP => WHvX64RegisterMsrMtrrCap, + MSR_SYSENTER_CS => WHvX64RegisterSysenterCs, + MSR_SYSENTER_ESP => WHvX64RegisterSysenterEsp, + MSR_SYSENTER_EIP => WHvX64RegisterSysenterEip, + MSR_PAT => WHvX64RegisterPat, + MSR_STAR => WHvX64RegisterStar, + MSR_LSTAR => WHvX64RegisterLstar, + MSR_CSTAR => WHvX64RegisterCstar, + MSR_SFMASK => WHvX64RegisterSfmask, + MSR_KERNEL_GS_BASE => WHvX64RegisterKernelGsBase, + MSR_SPEC_CTRL => WHvX64RegisterSpecCtrl, + MSR_U_CET => WHvX64RegisterUCet, + MSR_S_CET => WHvX64RegisterSCet, + MSR_PL0_SSP => WHvX64RegisterPl0Ssp, + MSR_PL1_SSP => WHvX64RegisterPl1Ssp, + MSR_PL2_SSP => WHvX64RegisterPl2Ssp, + MSR_PL3_SSP => WHvX64RegisterPl3Ssp, + MSR_INTERRUPT_SSP_TABLE_ADDR => WHvX64RegisterInterruptSspTableAddr, + // TSC and TSC offset/aux. + 0x10 => WHvX64RegisterTsc, + 0x3B => WHvX64RegisterTscAdjust, + 0xC000_0103 => WHvX64RegisterTscAux, + MSR_MPERF => WHvX64RegisterMCount, + MSR_APERF => WHvX64RegisterACount, + MSR_TSX_CTRL => WHvX64RegisterTsxCtrl, + MSR_XFD => WHvX64RegisterXfd, + MSR_XFD_ERR => WHvX64RegisterXfdErr, + MSR_UMWAIT_CONTROL => WHvX64RegisterUmwaitControl, + MSR_TSC_DEADLINE => WHvX64RegisterTscDeadline, + MSR_BNDCFGS => WHvX64RegisterBndcfgs, + // XSAVE supervisor state mask (IA32_XSS). + 0xDA0 => WHvX64RegisterXss, + // MTRRs: def type, variable base/mask pairs 0..=9, fixed ranges. + 0x2FF => WHvX64RegisterMsrMtrrDefType, + 0x200 => WHvX64RegisterMsrMtrrPhysBase0, + 0x201 => WHvX64RegisterMsrMtrrPhysMask0, + 0x202 => WHvX64RegisterMsrMtrrPhysBase1, + 0x203 => WHvX64RegisterMsrMtrrPhysMask1, + 0x204 => WHvX64RegisterMsrMtrrPhysBase2, + 0x205 => WHvX64RegisterMsrMtrrPhysMask2, + 0x206 => WHvX64RegisterMsrMtrrPhysBase3, + 0x207 => WHvX64RegisterMsrMtrrPhysMask3, + 0x208 => WHvX64RegisterMsrMtrrPhysBase4, + 0x209 => WHvX64RegisterMsrMtrrPhysMask4, + 0x20A => WHvX64RegisterMsrMtrrPhysBase5, + 0x20B => WHvX64RegisterMsrMtrrPhysMask5, + 0x20C => WHvX64RegisterMsrMtrrPhysBase6, + 0x20D => WHvX64RegisterMsrMtrrPhysMask6, + 0x20E => WHvX64RegisterMsrMtrrPhysBase7, + 0x20F => WHvX64RegisterMsrMtrrPhysMask7, + 0x210 => WHvX64RegisterMsrMtrrPhysBase8, + 0x211 => WHvX64RegisterMsrMtrrPhysMask8, + 0x212 => WHvX64RegisterMsrMtrrPhysBase9, + 0x213 => WHvX64RegisterMsrMtrrPhysMask9, + 0x214 => WHvX64RegisterMsrMtrrPhysBaseA, + 0x215 => WHvX64RegisterMsrMtrrPhysMaskA, + 0x216 => WHvX64RegisterMsrMtrrPhysBaseB, + 0x217 => WHvX64RegisterMsrMtrrPhysMaskB, + 0x218 => WHvX64RegisterMsrMtrrPhysBaseC, + 0x219 => WHvX64RegisterMsrMtrrPhysMaskC, + 0x21A => WHvX64RegisterMsrMtrrPhysBaseD, + 0x21B => WHvX64RegisterMsrMtrrPhysMaskD, + 0x21C => WHvX64RegisterMsrMtrrPhysBaseE, + 0x21D => WHvX64RegisterMsrMtrrPhysMaskE, + 0x21E => WHvX64RegisterMsrMtrrPhysBaseF, + 0x21F => WHvX64RegisterMsrMtrrPhysMaskF, + 0x250 => WHvX64RegisterMsrMtrrFix64k00000, + 0x258 => WHvX64RegisterMsrMtrrFix16k80000, + 0x259 => WHvX64RegisterMsrMtrrFix16kA0000, + 0x268 => WHvX64RegisterMsrMtrrFix4kC0000, + 0x269 => WHvX64RegisterMsrMtrrFix4kC8000, + 0x26A => WHvX64RegisterMsrMtrrFix4kD0000, + 0x26B => WHvX64RegisterMsrMtrrFix4kD8000, + 0x26C => WHvX64RegisterMsrMtrrFix4kE0000, + 0x26D => WHvX64RegisterMsrMtrrFix4kE8000, + 0x26E => WHvX64RegisterMsrMtrrFix4kF0000, + 0x26F => WHvX64RegisterMsrMtrrFix4kF8000, + _ => return None, + }) +} + +#[cfg(test)] +mod msr_mapping_tests { + use super::*; + use crate::hypervisor::regs::{MSR_DEBUGCTL, MSR_VIRT_SPEC_CTRL, core_reset_indices}; + + #[test] + fn maps_all_stateful_msrs_except_debugctl_and_virt_spec_ctrl() { + for index in core_reset_indices() { + // WHP models neither DEBUGCTL nor VIRT_SPEC_CTRL as a named register. + // Both are safe: WHP does not expose their guest-writable features, so + // a guest cannot leave retained state in them. + if index != MSR_DEBUGCTL && index != MSR_VIRT_SPEC_CTRL { + assert!( + msr_to_whv_register_name(index).is_some(), + "missing MSR mapping for {index:#x}" + ); + } + } + + assert!(msr_to_whv_register_name(MSR_DEBUGCTL).is_none()); + assert!(msr_to_whv_register_name(MSR_VIRT_SPEC_CTRL).is_none()); + assert_eq!( + msr_to_whv_register_name(MSR_MPERF), + Some(WHvX64RegisterMCount) + ); + assert_eq!( + msr_to_whv_register_name(MSR_APERF), + Some(WHvX64RegisterACount) + ); + assert_eq!( + msr_to_whv_register_name(MSR_TSX_CTRL), + Some(WHvX64RegisterTsxCtrl) + ); + assert_eq!(msr_to_whv_register_name(MSR_XFD), Some(WHvX64RegisterXfd)); + assert_eq!( + msr_to_whv_register_name(MSR_XFD_ERR), + Some(WHvX64RegisterXfdErr) + ); + assert_eq!( + msr_to_whv_register_name(MSR_UMWAIT_CONTROL), + Some(WHvX64RegisterUmwaitControl) + ); + assert_eq!( + msr_to_whv_register_name(MSR_TSC_DEADLINE), + Some(WHvX64RegisterTscDeadline) + ); + assert_eq!( + msr_to_whv_register_name(MSR_BNDCFGS), + Some(WHvX64RegisterBndcfgs) + ); + } +} + /// Helper: release a host-side file mapping view and its handle. /// Called from both `unmap_memory` and `WhpVm::drop`. fn release_file_mapping(view_base: *mut c_void, mapping_handle: HandleWrapper) { @@ -177,6 +318,9 @@ impl WhpVm { #[cfg(feature = "hw-interrupts")] Self::enable_lapic_emulation(p)?; + // Hyper-V permits MSR intercepts only for unimplemented indices. + // Implemented MSR isolation therefore depends on reset. + WHvSetupPartition(p).map_err(|e| CreateVmError::InitializeVm(e.into()))?; WHvCreateVirtualProcessor(p, 0, 0) .map_err(|e| CreateVmError::CreateVcpuFd(e.into()))?; @@ -667,6 +811,59 @@ impl VirtualMachine for WhpVm { } } + fn msrs(&self, indices: &[u32]) -> std::result::Result, RegisterError> { + if indices.is_empty() { + return Ok(Vec::new()); + } + // Callers validate every index before reaching this mapping. + let names: Vec = indices + .iter() + .map(|&i| msr_to_whv_register_name(i).ok_or(RegisterError::MsrsUnsupported)) + .collect::>()?; + // SAFETY: WHV_REGISTER_VALUE is a union of plain-old-data fields, so an + // all-zero value is a valid initial state. + let mut values: Vec> = + vec![unsafe { std::mem::zeroed() }; names.len()]; + // SAFETY: names and values have equal length. The call fills each value + // slot from the partition's vp 0 for the given register names. + unsafe { + WHvGetVirtualProcessorRegisters( + self.partition, + 0, + names.as_ptr(), + names.len() as u32, + values.as_mut_ptr() as *mut WHV_REGISTER_VALUE, + ) + .map_err(|e| RegisterError::GetMsrs(e.into()))?; + } + Ok(indices + .iter() + .zip(values) + .map(|(&index, v)| MsrEntry { + index, + // SAFETY: each register was read as a 64-bit MSR value. + value: unsafe { v.0.Reg64 }, + }) + .collect()) + } + + fn set_msrs(&self, msrs: &[MsrEntry]) -> std::result::Result<(), RegisterError> { + let regs: Vec<(WHV_REGISTER_NAME, Align16)> = msrs + .iter() + .map(|e| { + msr_to_whv_register_name(e.index) + .map(|name| (name, Align16(WHV_REGISTER_VALUE { Reg64: e.value }))) + .ok_or(RegisterError::MsrsUnsupported) + }) + .collect::>()?; + if regs.is_empty() { + return Ok(()); + } + self.set_registers(®s) + .map_err(|e| RegisterError::SetMsrs(e.into()))?; + Ok(()) + } + fn debug_regs(&self) -> std::result::Result { let mut whp_debug_regs_values: [Align16; WHP_DEBUG_REGS_NAMES_LEN] = Default::default(); diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index 7c74fea91..f8a3754ab 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -31,6 +31,8 @@ use super::shared_mem::{ ExclusiveSharedMemory, GuestSharedMemory, HostSharedMemory, ReadonlySharedMemory, SharedMemory, }; use crate::hypervisor::regs::CommonSpecialRegisters; +#[cfg(target_arch = "x86_64")] +use crate::hypervisor::regs::MsrEntry; use crate::mem::memory_region::MemoryRegion; #[cfg(crashdump)] use crate::mem::memory_region::{CrashDumpRegion, MemoryRegionFlags, MemoryRegionType}; @@ -306,6 +308,8 @@ where root_pt_gpas: &[u64], rsp_gva: u64, sregs: CommonSpecialRegisters, + #[cfg(target_arch = "x86_64")] msrs: Option>, + #[cfg(target_arch = "x86_64")] allowed_msrs: Option>, next_action: NextAction, host_functions: HostFunctionDetails, ) -> Result { @@ -319,6 +323,10 @@ where root_pt_gpas, rsp_gva, sregs, + #[cfg(target_arch = "x86_64")] + msrs, + #[cfg(target_arch = "x86_64")] + allowed_msrs, next_action, self.original_entrypoint, self.snapshot_count, diff --git a/src/hyperlight_host/src/sandbox/config.rs b/src/hyperlight_host/src/sandbox/config.rs index f12387a0b..ba5fffdb2 100644 --- a/src/hyperlight_host/src/sandbox/config.rs +++ b/src/hyperlight_host/src/sandbox/config.rs @@ -29,6 +29,18 @@ pub struct DebugInfo { pub port: u16, } +/// Errors returned when configuring guest MSR access. +#[cfg(target_arch = "x86_64")] +#[derive(Clone, Copy, Debug, Eq, PartialEq, thiserror::Error)] +pub enum AllowMsrError { + /// The requested allow list exceeds its fixed capacity. + #[error("MSR allow list exceeds its maximum of {maximum} distinct entries")] + CapacityExceeded { + /// Maximum number of distinct allowed MSRs. + maximum: usize, + }, +} + /// The complete set of configuration needed to create a Sandbox #[derive(Copy, Clone, Debug, Eq, PartialEq)] #[repr(C)] @@ -74,6 +86,12 @@ pub struct SandboxConfiguration { interrupt_vcpu_sigrtmin_offset: u8, /// How much writable memory to offer the guest scratch_size: usize, + /// Requested allow list stored inline to keep this type `Copy`. + #[cfg(target_arch = "x86_64")] + allowed_msrs: [u32; Self::MAX_ALLOWED_MSRS], + /// Number of valid entries in `allowed_msrs`. + #[cfg(target_arch = "x86_64")] + allowed_msrs_count: usize, } impl SandboxConfiguration { @@ -93,6 +111,11 @@ impl SandboxConfiguration { pub const DEFAULT_HEAP_SIZE: u64 = 131072; /// The default size of the scratch region pub const DEFAULT_SCRATCH_SIZE: usize = 0x48000; + /// Maximum number of distinct requested MSRs. + /// KVM supports at most 16 MSR filter ranges. Each index may require its + /// own range, so 16 is the portable limit across backends. + #[cfg(target_arch = "x86_64")] + pub const MAX_ALLOWED_MSRS: usize = 16; #[allow(clippy::too_many_arguments)] /// Create a new configuration for a sandbox with the given sizes. @@ -118,6 +141,10 @@ impl SandboxConfiguration { guest_debug_info, #[cfg(crashdump)] guest_core_dump, + #[cfg(target_arch = "x86_64")] + allowed_msrs: [0; Self::MAX_ALLOWED_MSRS], + #[cfg(target_arch = "x86_64")] + allowed_msrs_count: 0, } } @@ -159,6 +186,49 @@ impl SandboxConfiguration { self.interrupt_vcpu_sigrtmin_offset } + /// Adds MSRs to the sandbox allow list. + /// VM creation verifies that the backend can restore them. + /// + /// Duplicate indices, within the slice or against the existing list, are + /// ignored and do not count toward capacity. + /// + /// # Errors + /// + /// Returns [`AllowMsrError::CapacityExceeded`] if the distinct entries + /// would exceed [`Self::MAX_ALLOWED_MSRS`]. The allow list is unchanged on + /// error. + #[cfg(target_arch = "x86_64")] + #[instrument(skip_all, parent = Span::current(), level= "Trace")] + pub fn allow_msrs(&mut self, indices: &[u32]) -> Result<&mut Self, AllowMsrError> { + let additional = indices + .iter() + .enumerate() + .filter(|(position, index)| { + !self.allowed_msrs[..self.allowed_msrs_count].contains(index) + && !indices[..*position].contains(index) + }) + .count(); + if additional > Self::MAX_ALLOWED_MSRS - self.allowed_msrs_count { + return Err(AllowMsrError::CapacityExceeded { + maximum: Self::MAX_ALLOWED_MSRS, + }); + } + for &index in indices { + if !self.allowed_msrs[..self.allowed_msrs_count].contains(&index) { + self.allowed_msrs[self.allowed_msrs_count] = index; + self.allowed_msrs_count += 1; + } + } + Ok(self) + } + + /// Returns the requested allow list. + #[cfg(target_arch = "x86_64")] + #[instrument(skip_all, parent = Span::current(), level= "Trace")] + pub(crate) fn get_allowed_msrs(&self) -> &[u32] { + &self.allowed_msrs[..self.allowed_msrs_count] + } + /// Sets the offset from `SIGRTMIN` to determine the real-time signal used for /// interrupting the VCPU thread. /// @@ -261,7 +331,63 @@ impl Default for SandboxConfiguration { #[cfg(test)] mod tests { - use super::SandboxConfiguration; + use super::{AllowMsrError, SandboxConfiguration}; + + #[test] + #[cfg(target_arch = "x86_64")] + fn msr_allow_list_reports_overflow() { + let mut cfg = SandboxConfiguration::default(); + for index in 0..SandboxConfiguration::MAX_ALLOWED_MSRS as u32 { + cfg.allow_msrs(&[index]).unwrap(); + } + + cfg.allow_msrs(&[0]).unwrap(); + assert_eq!( + cfg.allow_msrs(&[SandboxConfiguration::MAX_ALLOWED_MSRS as u32]), + Err(AllowMsrError::CapacityExceeded { + maximum: SandboxConfiguration::MAX_ALLOWED_MSRS, + }) + ); + } + + #[test] + #[cfg(target_arch = "x86_64")] + fn bulk_msr_allow_list_overflow_is_atomic() { + let mut cfg = SandboxConfiguration::default(); + cfg.allow_msrs(&[1, 2]).unwrap(); + let oversized: Vec = (3..=SandboxConfiguration::MAX_ALLOWED_MSRS as u32 + 1).collect(); + + assert!(matches!( + cfg.allow_msrs(&oversized), + Err(AllowMsrError::CapacityExceeded { .. }) + )); + assert_eq!(cfg.get_allowed_msrs(), &[1, 2]); + } + + #[test] + #[cfg(target_arch = "x86_64")] + fn allow_msrs_dedups_and_preserves_order() { + let mut cfg = SandboxConfiguration::default(); + cfg.allow_msrs(&[0x10]).unwrap(); + cfg.allow_msrs(&[0x20, 0x20, 0x10, 0x30, 0x20]).unwrap(); + // 0x10 already present, 0x20 and 0x30 added once each in first-seen order. + assert_eq!(cfg.get_allowed_msrs(), &[0x10, 0x20, 0x30]); + } + + #[test] + #[cfg(target_arch = "x86_64")] + fn allow_msrs_duplicates_do_not_count_toward_capacity() { + let mut cfg = SandboxConfiguration::default(); + let fill: Vec = (0..SandboxConfiguration::MAX_ALLOWED_MSRS as u32 - 1).collect(); + cfg.allow_msrs(&fill).unwrap(); + // One slot remains. Three copies of one new index count as a single + // distinct entry and fit. + cfg.allow_msrs(&[u32::MAX, u32::MAX, u32::MAX]).unwrap(); + assert_eq!( + cfg.get_allowed_msrs().len(), + SandboxConfiguration::MAX_ALLOWED_MSRS + ); + } #[test] fn overrides() { diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 8e60c68e7..025826f7d 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -159,7 +159,9 @@ impl MultiUseSandbox { /// interrupt behavior. Memory layout fields /// (`input_data_size`, `output_data_size`, `heap_size`, `scratch_size`) /// are always taken from the snapshot. Any values supplied in - /// `config` for those fields are ignored. + /// `config` for those fields are ignored. On x86_64 the `config` MSR + /// allow list must be a superset of the one the snapshot was taken with, + /// or the load fails with an MSR mismatch. /// /// # Examples /// @@ -304,6 +306,15 @@ impl MultiUseSandbox { crate::hypervisor::hyperlight_vm::HyperlightVmError::Restore(e.into()), ) })?; + + // Restore captured MSR state. + #[cfg(target_arch = "x86_64")] + vm.restore_msrs(snapshot.msrs(), snapshot.allowed_msrs()) + .map_err(|e| { + crate::HyperlightError::HyperlightVmError( + crate::hypervisor::hyperlight_vm::HyperlightVmError::Restore(e), + ) + })?; } #[cfg(gdb)] @@ -385,6 +396,13 @@ impl MultiUseSandbox { .vm .get_snapshot_sregs() .map_err(|e| HyperlightError::HyperlightVmError(e.into()))?; + #[cfg(target_arch = "x86_64")] + let msrs = self + .vm + .get_msr_reset_state() + .map_err(|e| HyperlightError::HyperlightVmError(e.into()))?; + #[cfg(target_arch = "x86_64")] + let allowed_msrs = self.vm.get_msr_allow_list(); let next_action = self.vm.get_next_action(); let host_functions = (&*self.host_funcs.try_lock().map_err(|e| { crate::new_error!("Error locking host_funcs at {}:{}: {}", file!(), line!(), e) @@ -396,6 +414,10 @@ impl MultiUseSandbox { &root_pt_gpas, stack_top_gpa, sregs, + #[cfg(target_arch = "x86_64")] + Some(msrs), + #[cfg(target_arch = "x86_64")] + Some(allowed_msrs), next_action, host_functions, )?; @@ -417,6 +439,10 @@ impl MultiUseSandbox { /// [`SnapshotHostFunctionMismatch`](crate::HyperlightError::SnapshotHostFunctionMismatch) /// carrying the missing names and signature differences. /// + /// On x86_64 this sandbox's MSR allow list must be a superset of the one + /// the snapshot was taken with, or the restore poisons with an MSR + /// mismatch. + /// /// ## Poison State Recovery /// /// This method automatically clears any poison state when successful. This is safe because: @@ -543,6 +569,15 @@ impl MultiUseSandbox { HyperlightVmError::Restore(e) })?; + // Restore captured MSR state. + #[cfg(target_arch = "x86_64")] + self.vm + .restore_msrs(snapshot.msrs(), snapshot.allowed_msrs()) + .map_err(|e| { + self.poisoned = true; + HyperlightVmError::Restore(e) + })?; + self.vm.set_stack_top(snapshot.stack_top_gva()); self.vm.set_next_action(snapshot.next_action()); // Carry the guest ELF entry point across restore so a later @@ -2725,6 +2760,1223 @@ mod tests { ); } + #[cfg(target_arch = "x86_64")] + mod msr_tests { + use super::*; + use crate::HostFunctions; + use crate::hypervisor::hyperlight_vm::{CreateHyperlightVmError, HyperlightVmError}; + use crate::hypervisor::virtual_machine::{ + CreateVmError, RegisterError, ResetVcpuError, VmError, + }; + use crate::sandbox::snapshot::Snapshot; + + const KERNEL_GS_BASE: u32 = 0xC000_0102; + const SYSENTER_CS: u32 = 0x174; + + fn assert_msr_not_allowable(error: &HyperlightError, expected: u32) { + assert!( + matches!( + error, + HyperlightError::HyperlightVmError(HyperlightVmError::Create( + CreateHyperlightVmError::Vm(VmError::CreateVm( + CreateVmError::MsrNotAllowable { msr, .. } + )) + )) if *msr == expected + ), + "expected MsrNotAllowable for {expected:#x}, got: {error:?}" + ); + } + + fn assert_invalid_snapshot_msr(error: &HyperlightError) { + assert!( + matches!( + error, + HyperlightError::HyperlightVmError(HyperlightVmError::Restore( + ResetVcpuError::Register(RegisterError::InvalidSnapshotMsrIndex { .. }) + )) + ), + "expected InvalidSnapshotMsrIndex, got: {error:?}" + ); + } + + fn assert_msr_not_allowed(error: &HyperlightError) { + assert!( + matches!( + error, + HyperlightError::HyperlightVmError(HyperlightVmError::Restore( + ResetVcpuError::Register(RegisterError::SnapshotMsrNotAllowed { .. }) + )) + ), + "expected SnapshotMsrNotAllowed, got: {error:?}" + ); + } + + #[test] + fn kernel_gs_base_does_not_leak_through_swapgs() { + let mut sandbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + let original: u64 = sandbox.call("ReadKernelGsBaseViaSwapgs", ()).unwrap(); + let sentinel = if original == 0x0000_7AAA_5555_AAAA { + 0x0000_6BBB_4444_BBBB + } else { + 0x0000_7AAA_5555_AAAA + }; + let snapshot = sandbox.snapshot().unwrap(); + + sandbox + .call::<()>("WriteKernelGsBaseViaSwapgs", sentinel) + .unwrap(); + assert_eq!( + sandbox + .call::("ReadKernelGsBaseViaSwapgs", ()) + .unwrap(), + sentinel + ); + + sandbox.restore(snapshot).unwrap(); + assert_eq!( + sandbox + .call::("ReadKernelGsBaseViaSwapgs", ()) + .unwrap(), + original, + "KERNEL_GS_BASE leaked across restore" + ); + } + + #[test] + fn snapshot_msr_values_survive_full_in_memory_lifecycle() { + let mut config = SandboxConfiguration::default(); + config.allow_msrs(&[KERNEL_GS_BASE]).unwrap(); + let mut source = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(config), + ) + .unwrap() + .evolve() + .unwrap(); + let first = 0x1111; + let second = 0x2222; + let third = 0x3333; + + source + .call::<()>("WriteMSR", (KERNEL_GS_BASE, first)) + .unwrap(); + assert_eq!( + source.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + first + ); + let first_snapshot = source.snapshot().unwrap(); + + source + .call::<()>("WriteMSR", (KERNEL_GS_BASE, second)) + .unwrap(); + assert_eq!( + source.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + second + ); + source.restore(first_snapshot.clone()).unwrap(); + assert_eq!( + source.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + first + ); + + let mut clone = MultiUseSandbox::from_snapshot( + first_snapshot.clone(), + HostFunctions::default(), + Some(config), + ) + .unwrap(); + assert_eq!(clone.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), first); + + clone + .call::<()>("WriteMSR", (KERNEL_GS_BASE, third)) + .unwrap(); + assert_eq!(clone.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), third); + let third_snapshot = clone.snapshot().unwrap(); + source.restore(third_snapshot.clone()).unwrap(); + assert_eq!( + source.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + third + ); + + let mut second_clone = MultiUseSandbox::from_snapshot( + third_snapshot, + HostFunctions::default(), + Some(config), + ) + .unwrap(); + assert_eq!( + second_clone.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + third + ); + second_clone.restore(first_snapshot).unwrap(); + assert_eq!( + second_clone.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + first + ); + } + + #[test] + fn equivalent_msr_configs_are_order_independent_across_sandboxes() { + let source_order = [KERNEL_GS_BASE, SYSENTER_CS]; + let target_order = [SYSENTER_CS, KERNEL_GS_BASE]; + let mut source_config = SandboxConfiguration::default(); + source_config.allow_msrs(&source_order).unwrap(); + let mut source = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(source_config), + ) + .unwrap() + .evolve() + .unwrap(); + source + .call::<()>("WriteMSR", (KERNEL_GS_BASE, 0x4444u64)) + .unwrap(); + assert_eq!( + source.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + 0x4444 + ); + source + .call::<()>("WriteMSR", (SYSENTER_CS, 0x5555u64)) + .unwrap(); + assert_eq!(source.call::("ReadMSR", SYSENTER_CS).unwrap(), 0x5555); + let snapshot = source.snapshot().unwrap(); + + let mut target_config = SandboxConfiguration::default(); + target_config.allow_msrs(&target_order).unwrap(); + let mut target = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(target_config), + ) + .unwrap() + .evolve() + .unwrap(); + target + .call::<()>("WriteMSR", (KERNEL_GS_BASE, 0xAAAAu64)) + .unwrap(); + assert_eq!( + target.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + 0xAAAA + ); + target + .call::<()>("WriteMSR", (SYSENTER_CS, 0xBBBBu64)) + .unwrap(); + assert_eq!(target.call::("ReadMSR", SYSENTER_CS).unwrap(), 0xBBBB); + target.restore(snapshot.clone()).unwrap(); + assert_eq!( + target.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + 0x4444 + ); + assert_eq!(target.call::("ReadMSR", SYSENTER_CS).unwrap(), 0x5555); + + let mut clone = MultiUseSandbox::from_snapshot( + snapshot, + HostFunctions::default(), + Some(target_config), + ) + .unwrap(); + assert_eq!( + clone.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + 0x4444 + ); + assert_eq!(clone.call::("ReadMSR", SYSENTER_CS).unwrap(), 0x5555); + } + + /// A restore succeeds when the destination allow list is a superset + /// of the snapshot's. The snapshot's allowed MSR keeps its captured + /// value. An MSR the destination adds resets to the baseline. + #[test] + fn snapshot_restores_into_superset_allow_list() { + const SYSENTER_ESP: u32 = 0x175; + let sentinel: u64 = 0x1234; + let mut source_config = SandboxConfiguration::default(); + source_config.allow_msrs(&[SYSENTER_CS]).unwrap(); + let mut source = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(source_config), + ) + .unwrap() + .evolve() + .unwrap(); + source + .call::<()>("WriteMSR", (SYSENTER_CS, sentinel)) + .unwrap(); + let snapshot = source.snapshot().unwrap(); + + let mut dest_config = SandboxConfiguration::default(); + dest_config + .allow_msrs(&[SYSENTER_CS, SYSENTER_ESP]) + .unwrap(); + + let mut clone = MultiUseSandbox::from_snapshot( + snapshot.clone(), + HostFunctions::default(), + Some(dest_config), + ) + .unwrap(); + assert_eq!(clone.call::("ReadMSR", SYSENTER_CS).unwrap(), sentinel); + let baseline: u64 = clone.call("ReadMSR", SYSENTER_ESP).unwrap(); + + let mut target = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(dest_config), + ) + .unwrap() + .evolve() + .unwrap(); + target + .call::<()>("WriteMSR", (SYSENTER_ESP, baseline ^ 0x55)) + .unwrap(); + target.restore(snapshot).unwrap(); + assert_eq!( + target.call::("ReadMSR", SYSENTER_CS).unwrap(), + sentinel + ); + // An MSR the destination adds resets to its baseline. + assert_eq!( + target.call::("ReadMSR", SYSENTER_ESP).unwrap(), + baseline + ); + } + + /// A restore is rejected when the snapshot allows an MSR the + /// destination does not. Both restore paths fail and poison the + /// sandbox. + #[test] + fn snapshot_rejects_non_superset_allow_list() { + const SYSENTER_ESP: u32 = 0x175; + let mut source_config = SandboxConfiguration::default(); + source_config.allow_msrs(&[SYSENTER_CS]).unwrap(); + let mut source = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(source_config), + ) + .unwrap() + .evolve() + .unwrap(); + source + .call::<()>("WriteMSR", (SYSENTER_CS, 0x1234u64)) + .unwrap(); + let snapshot = source.snapshot().unwrap(); + + // A destination that allows nothing, and one that allows a + // disjoint MSR, both fail the superset check. + for dest in [&[][..], &[SYSENTER_ESP][..]] { + let mut config = SandboxConfiguration::default(); + config.allow_msrs(dest).unwrap(); + + let err = MultiUseSandbox::from_snapshot( + snapshot.clone(), + HostFunctions::default(), + Some(config), + ) + .expect_err("from_snapshot must reject a non-superset allow list"); + assert_msr_not_allowed(&err); + + let mut target = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(config), + ) + .unwrap() + .evolve() + .unwrap(); + let err = target + .restore(snapshot.clone()) + .expect_err("restore must reject a non-superset allow list"); + assert_msr_not_allowed(&err); + assert!(target.poisoned()); + assert!(matches!( + target.call::("Echo", "hi".to_string()), + Err(HyperlightError::PoisonedSandbox) + )); + } + } + + #[test] + fn from_pre_init_snapshot_uses_local_msr_reset_set() { + let mut config = SandboxConfiguration::default(); + config.allow_msrs(&[KERNEL_GS_BASE]).unwrap(); + let snapshot = Arc::new( + Snapshot::from_env( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + config, + ) + .unwrap(), + ); + assert!(snapshot.msrs().is_none()); + + let mut sandbox = MultiUseSandbox::from_snapshot( + snapshot.clone(), + HostFunctions::default(), + Some(config), + ) + .unwrap(); + let baseline: u64 = sandbox.call("ReadMSR", KERNEL_GS_BASE).unwrap(); + sandbox + .call::<()>("WriteMSR", (KERNEL_GS_BASE, baseline ^ 0x55)) + .unwrap(); + assert_eq!( + sandbox.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + baseline ^ 0x55 + ); + } + + #[test] + #[cfg(target_arch = "x86_64")] + fn snapshot_without_msrs_uses_destination_reset_set() { + let mut source = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + let snapshot = source.snapshot().unwrap(); + source.snapshot = None; + let Ok(mut snap) = Arc::try_unwrap(snapshot) else { + panic!("snapshot should be uniquely owned"); + }; + // A snapshot without MSRs uses the destination baseline. + snap.set_msrs(None); + snap.set_allowed_msrs(None); + let snapshot = Arc::new(snap); + + let mut config = SandboxConfiguration::default(); + config.allow_msrs(&[KERNEL_GS_BASE]).unwrap(); + let mut target = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(config), + ) + .unwrap() + .evolve() + .unwrap(); + let baseline: u64 = target.call("ReadMSR", KERNEL_GS_BASE).unwrap(); + target + .call::<()>("WriteMSR", (KERNEL_GS_BASE, baseline ^ 0x55)) + .unwrap(); + assert_eq!( + target.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + baseline ^ 0x55 + ); + target.restore(snapshot.clone()).unwrap(); + assert_eq!( + target.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + baseline + ); + target + .call::<()>("WriteMSR", (KERNEL_GS_BASE, baseline ^ 0xAA)) + .unwrap(); + assert_eq!( + target.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + baseline ^ 0xAA + ); + + let mut clone = + MultiUseSandbox::from_snapshot(snapshot, HostFunctions::default(), Some(config)) + .unwrap(); + let clone_baseline: u64 = clone.call("ReadMSR", KERNEL_GS_BASE).unwrap(); + clone + .call::<()>("WriteMSR", (KERNEL_GS_BASE, clone_baseline ^ 0xCC)) + .unwrap(); + assert_eq!( + clone.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + clone_baseline ^ 0xCC + ); + } + + #[test] + fn malformed_snapshot_msrs_poison_and_trusted_restore_recovers() { + let indices = [SYSENTER_CS, KERNEL_GS_BASE]; + let mut config = SandboxConfiguration::default(); + config.allow_msrs(&indices).unwrap(); + let mut source = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(config), + ) + .unwrap() + .evolve() + .unwrap(); + let snapshot = source.snapshot().unwrap(); + source.snapshot = None; + let Ok(mut snapshot) = Arc::try_unwrap(snapshot) else { + panic!("snapshot should be uniquely owned"); + }; + let mut msrs = snapshot.msrs().unwrap().clone(); + msrs[0].index = 0xDEAD; + snapshot.set_msrs(Some(msrs)); + let snapshot = Arc::new(snapshot); + + let error = MultiUseSandbox::from_snapshot( + snapshot.clone(), + HostFunctions::default(), + Some(config), + ) + .expect_err("from_snapshot must reject malformed snapshot MSRs"); + assert_invalid_snapshot_msr(&error); + + let mut target = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(config), + ) + .unwrap() + .evolve() + .unwrap(); + let trusted_value: u64 = target.call("ReadMSR", KERNEL_GS_BASE).unwrap(); + let recovery_snapshot = target.snapshot().unwrap(); + target + .call::<()>("WriteMSR", (KERNEL_GS_BASE, trusted_value ^ 0x55)) + .unwrap(); + let error = target + .restore(snapshot) + .expect_err("restore must reject malformed snapshot MSRs"); + assert_invalid_snapshot_msr(&error); + assert!(target.poisoned()); + assert!(matches!( + target.call::("Echo", "hi".to_string()), + Err(HyperlightError::PoisonedSandbox) + )); + + target.restore(recovery_snapshot).unwrap(); + assert!(!target.poisoned()); + assert_eq!( + target.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), + trusted_value + ); + } + + #[test] + #[cfg(kvm)] + fn denied_msr_access_poisons_sandbox() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + match get_available_hypervisor() { + Some(HypervisorType::Kvm) => {} + _ => { + return; + } + } + + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + let snapshot = sbox.snapshot().unwrap(); + let msr_index: u32 = 0xC000_0102; // IA32_KERNEL_GS_BASE + + let result = sbox.call::("ReadMSR", msr_index); + assert!( + matches!( + &result, + Err(HyperlightError::MsrReadViolation(idx)) if *idx == msr_index + ), + "RDMSR 0x{:X}: expected MsrReadViolation, got: {:?}", + msr_index, + result + ); + assert!(sbox.poisoned()); + + sbox.restore(snapshot.clone()).unwrap(); + + let result = sbox.call::<()>("WriteMSR", (msr_index, 0x5u64)); + assert!( + matches!( + &result, + Err(HyperlightError::MsrWriteViolation(idx, _)) if *idx == msr_index + ), + "WRMSR 0x{:X}: expected MsrWriteViolation, got: {:?}", + msr_index, + result + ); + assert!(sbox.poisoned()); + } + + /// A write-only command cannot enter the reset set. + #[test] + #[cfg(target_arch = "x86_64")] + fn test_allow_non_resettable_msr_fails_creation() { + let mut cfg = SandboxConfiguration::default(); + cfg.allow_msrs(&[0x49]).unwrap(); // IA32_PRED_CMD, a write-only command MSR + + let err = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(cfg), + ) + .unwrap() + .evolve() + .unwrap_err(); + + assert_msr_not_allowable(&err, 0x49); + } + + /// Host support cannot authorize an unclassified MSR. + #[test] + #[cfg(kvm)] + fn unclassified_allowed_msr_rejected_at_creation() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Kvm)) { + return; + } + + let mut cfg = SandboxConfiguration::default(); + cfg.allow_msrs(&[0x1A0]).unwrap(); // IA32_MISC_ENABLE: host-probeable, not in MSR_TABLE + + let err = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(cfg), + ) + .unwrap() + .evolve() + .expect_err("an unclassified allowed MSR must be rejected at creation"); + + assert_msr_not_allowable(&err, 0x1A0); + } + + #[test] + #[cfg(target_arch = "x86_64")] + fn test_multiple_allowed_msrs_reset_across_restore() { + // Resettable MSRs the guest may write once allowed. + let msrs: [u32; 4] = [0x174, 0x175, 0x176, 0xC000_0102]; + let mut cfg = SandboxConfiguration::default(); + cfg.allow_msrs(&msrs).unwrap(); + + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(cfg), + ) + .unwrap() + .evolve() + .unwrap(); + + let baseline_snapshot = sbox.snapshot().unwrap(); + + let value: u64 = 0x1000; + for &msr in &msrs { + sbox.call::<()>("WriteMSR", (msr, value)).unwrap(); + let read_value: u64 = sbox.call("ReadMSR", msr).unwrap(); + assert_eq!(read_value, value, "MSR 0x{msr:X} should be writable"); + } + + sbox.restore(baseline_snapshot).unwrap(); + for &msr in &msrs { + let read_value: u64 = sbox.call("ReadMSR", msr).unwrap(); + assert_ne!( + read_value, value, + "MSR 0x{msr:X} should be reset to baseline across restore" + ); + } + } + + /// An allowed guest write must not survive restore. + #[test] + #[cfg(target_arch = "x86_64")] + fn test_allowed_msr_does_not_leak_across_restore() { + let msr_index: u32 = 0xC000_0102; // IA32_KERNEL_GS_BASE + let sentinel: u64 = 0xCAFE_F00D; + + let mut cfg = SandboxConfiguration::default(); + cfg.allow_msrs(&[msr_index]).unwrap(); + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(cfg), + ) + .unwrap() + .evolve() + .unwrap(); + + let baseline = sbox.snapshot().unwrap(); + let original: u64 = sbox.call("ReadMSR", msr_index).unwrap(); + assert_ne!( + original, sentinel, + "test sentinel must differ from the baseline value" + ); + + sbox.call::<()>("WriteMSR", (msr_index, sentinel)).unwrap(); + assert_eq!( + sbox.call::("ReadMSR", msr_index).unwrap(), + sentinel, + "sentinel should be observable before restore" + ); + sbox.restore(baseline).unwrap(); + + let after: u64 = sbox.call("ReadMSR", msr_index).unwrap(); + assert_ne!(after, sentinel, "sentinel leaked across restore"); + assert_eq!(after, original, "MSR not reset to its baseline value"); + } + + /// KVM denies DEBUGCTL through its filter and x2APIC through xAPIC mode. + #[test] + #[cfg(all(kvm, target_arch = "x86_64"))] + fn test_debugctl_and_x2apic_msr_denied_by_default() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Kvm)) { + return; + } + + let cases: [(u32, bool); 2] = [(0x1D9, true), (0x800, false)]; + for (msr_index, expect_filter_violation) in cases { + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + let result = sbox.call::<()>("WriteMSR", (msr_index, 0x1u64)); + if expect_filter_violation { + assert!( + matches!( + &result, + Err(HyperlightError::MsrWriteViolation(idx, _)) if *idx == msr_index + ), + "WRMSR 0x{msr_index:X}: expected MsrWriteViolation, got: {result:?}" + ); + } else { + assert!( + matches!(&result, Err(HyperlightError::GuestAborted(_, _))), + "WRMSR 0x{msr_index:X}: expected direct #GP, got: {result:?}" + ); + } + assert!( + sbox.poisoned(), + "sandbox should be poisoned after a denied WRMSR to 0x{msr_index:X}" + ); + } + } + + /// A rejected MSR restore poisons the sandbox on every backend. + /// + /// The host register set rejects the noncanonical KERNEL_GS_BASE value + /// on KVM, MSHV, and WHP alike, leaving the sandbox poisoned. + #[test] + #[cfg(target_arch = "x86_64")] + fn rejected_msr_restore_poisons_sandbox() { + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + let baseline = sbox.snapshot().unwrap(); + sbox.snapshot = None; + let Ok(mut snap) = Arc::try_unwrap(baseline) else { + panic!("snapshot should be uniquely owned after clearing the cache"); + }; + // A noncanonical KERNEL_GS_BASE value the host set rejects. + let mut msrs = snap.msrs().unwrap().clone(); + let kernel_gs_base = msrs + .iter_mut() + .find(|entry| entry.index == 0xC000_0102) + .expect("KERNEL_GS_BASE should be in the reset set"); + kernel_gs_base.value = 0xDEAD_0000_0000_0000; + snap.set_msrs(Some(msrs)); + let snap = Arc::new(snap); + + let err = sbox + .restore(snap) + .expect_err("restore should fail on a rejected MSR set"); + assert!( + format!("{err:?}").to_lowercase().contains("msr") + || format!("{err:?}").to_lowercase().contains("restore"), + "expected an MSR restore error, got: {err:?}" + ); + assert!( + sbox.poisoned(), + "sandbox should be poisoned after failed restore" + ); + + let call = sbox.call::("Echo", "hi".to_string()); + assert!( + matches!(call, Err(HyperlightError::PoisonedSandbox)), + "poisoned sandbox should reject guest calls, got: {call:?}" + ); + } + + /// Unresettable feature-class MSRs must not retain guest writes. PMU, + /// LBR, and FRED are perfmon or feature gated. The AMD virtualization + /// MSRs are gated on nested-virt capability the sandbox never requests. + #[test] + #[cfg(target_arch = "x86_64")] + fn unresettable_msr_classes_do_not_leak() { + let cases: &[(u32, &str)] = &[ + (0xC1, "PMU IA32_PMC0"), + (0x186, "PMU IA32_PERFEVTSEL0"), + (0x38F, "PMU IA32_PERF_GLOBAL_CTRL"), + (0x1C8, "LBR_SELECT"), + (0x14CE, "arch-LBR IA32_LBR_CTL"), + (0x1D4, "FRED IA32_FRED_CONFIG"), + (0xC001_0114, "AMD VM_CR"), + (0xC001_0117, "AMD VM_HSAVE_PA"), + ]; + + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + for &(msr, _name) in cases { + assert_msr_write_does_not_survive_restore(&mut sbox, msr, 0x1); + } + } + + /// A guest write to IA32_MISC_ENABLE leaves no retained state. Hyper-V + /// drops the write on Intel and faults it on AMD. + #[test] + #[cfg(target_arch = "x86_64")] + fn misc_enable_guest_write_does_not_survive_restore() { + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + assert_msr_write_does_not_survive_restore(&mut sbox, 0x1A0, 1u64 << 40); + } + + /// Every stateful table entry needs runtime reset coverage. + #[test] + #[cfg(target_arch = "x86_64")] + fn runtime_msr_table_entries_are_justified() { + use crate::hypervisor::regs::core_reset_indices; + + #[cfg(kvm)] + let kernel_gs_uses_instruction_side_effect = matches!( + crate::hypervisor::virtual_machine::get_available_hypervisor(), + Some(crate::hypervisor::virtual_machine::HypervisorType::Kvm) + ); + #[cfg(not(kvm))] + let kernel_gs_uses_instruction_side_effect = false; + + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + let reset_indices: Vec = sbox + .snapshot() + .unwrap() + .msrs() + .expect("filterless backend should have an MSR reset set") + .iter() + .map(|entry| entry.index) + .collect(); + + for index in core_reset_indices() { + if !reset_indices.contains(&index) { + assert_omitted_msr_does_not_retain(&mut sbox, index); + } else if kernel_gs_uses_instruction_side_effect && index == KERNEL_GS_BASE { + // Direct WRMSR is denied. The dedicated SWAPGS test proves + // the instruction-side mutation is restored. + } else if (index == 0x10 && !kernel_gs_uses_instruction_side_effect) + || matches!(index, 0xE7 | 0xE8) + { + assert_guest_counter_is_writable_and_restored(&mut sbox, index); + } else if let Some(sentinel) = positive_write_sentinel(index) { + assert_guest_msr_is_writable_and_restored(&mut sbox, index, sentinel); + } else { + assert!( + reset_exception_reason(index).is_some(), + "MSR 0x{index:X} is in the reset set without positive guest-write coverage or an explicit reason" + ); + } + } + } + + fn assert_omitted_msr_does_not_retain(sbox: &mut MultiUseSandbox, index: u32) { + let baseline = sbox.snapshot().unwrap(); + let original: u64 = match sbox.call("ReadMSR", index) { + Ok(value) => value, + Err(_) => { + assert!(sbox.poisoned(), "0x{index:X}: fault did not poison sandbox"); + sbox.restore(baseline).unwrap(); + return; + } + }; + let preferred = positive_write_sentinel(index).unwrap_or(original ^ 1); + let candidates = [preferred, original ^ 1, original ^ 2, 0, 1, 0x1000]; + + for candidate in candidates { + if candidate == original { + continue; + } + if sbox.call::<()>("WriteMSR", (index, candidate)).is_err() { + assert!(sbox.poisoned(), "0x{index:X}: fault did not poison sandbox"); + sbox.restore(baseline.clone()).unwrap(); + continue; + } + let written: u64 = sbox.call("ReadMSR", index).unwrap_or_else(|error| { + panic!("0x{index:X}: read after successful write failed: {error:?}") + }); + if written != original { + sbox.restore(baseline).unwrap(); + let after: u64 = sbox.call("ReadMSR", index).unwrap(); + assert_eq!( + after, original, + "0x{index:X}: guest retained a write but the MSR is absent from the reset set" + ); + return; + } + sbox.restore(baseline.clone()).unwrap(); + } + } + + fn positive_write_sentinel(index: u32) -> Option { + match index { + 0x174 => Some(0x10), // SYSENTER_CS + 0x175 | 0x176 => Some(0x1000), // SYSENTER_ESP/EIP + 0x277 => Some(0x0007_0406_0007_0406), // PAT + 0xC000_0081 => Some(0x001B_0008_0000_0000), // STAR + 0xC000_0082 | 0xC000_0083 => Some(0x1000), // LSTAR/CSTAR + 0xC000_0084 => Some(0x200), // SFMASK + 0xC000_0102 => Some(0x1000), // KERNEL_GS_BASE + 0x3B => Some(0x1000), // TSC_ADJUST + 0xC000_0103 => Some(0x5), // TSC_AUX + 0x2FF => Some(0xC00), // MTRR_DEF_TYPE + 0x200..=0x21F if index & 1 == 0 => Some(0x6), // MTRR_PHYSBASEn + 0x200..=0x21F => Some(0x800), // MTRR_PHYSMASKn + 0x250 | 0x258 | 0x259 | 0x268..=0x26F => Some(0x0606_0606_0606_0606), + _ => None, + } + } + + fn reset_exception_reason(index: u32) -> Option<&'static str> { + match index { + 0x10 => Some("KVM denies direct guest TSC MSR access"), + 0x1D9 => Some("DEBUGCTL support depends on exposed debug features"), + 0x48 => Some("SPEC_CTRL writable bits depend on mitigation features"), + 0x6A0 | 0x6A2 | 0x6A4..=0x6A8 => { + Some("CET writable state depends on exposed CET features") + } + 0x122 => Some("TSX_CTRL writable bits depend on exposed TSX features"), + 0x1C4 | 0x1C5 => Some("XFD writable bits depend on exposed XSAVE features"), + 0xE1 => Some("UMWAIT_CONTROL writable bits depend on exposed WAITPKG features"), + 0x6E0 => Some("TSC_DEADLINE writable bits depend on exposed APIC-timer features"), + 0xD90 => Some("BNDCFGS writable bits depend on exposed MPX features"), + 0xDA0 => Some("XSS writable bits depend on exposed XSAVE features"), + 0xC001_011F => { + Some("VIRT_SPEC_CTRL writable bits depend on exposed AMD SSBD virtualization") + } + _ => None, + } + } + + fn assert_guest_msr_is_writable_and_restored( + sbox: &mut MultiUseSandbox, + index: u32, + sentinel: u64, + ) { + let baseline = sbox.snapshot().unwrap(); + let original: u64 = sbox + .call("ReadMSR", index) + .unwrap_or_else(|error| panic!("0x{index:X}: guest RDMSR failed: {error:?}")); + let value = if original == sentinel { 0 } else { sentinel }; + + sbox.call::<()>("WriteMSR", (index, value)) + .unwrap_or_else(|error| panic!("0x{index:X}: guest WRMSR failed: {error:?}")); + let written: u64 = sbox + .call("ReadMSR", index) + .unwrap_or_else(|error| panic!("0x{index:X}: guest read-back failed: {error:?}")); + assert_eq!(written, value, "0x{index:X}: guest write did not stick"); + + sbox.restore(baseline).unwrap(); + let restored: u64 = sbox.call("ReadMSR", index).unwrap(); + assert_eq!( + restored, original, + "0x{index:X}: restore did not recover the baseline" + ); + } + + fn assert_guest_counter_is_writable_and_restored(sbox: &mut MultiUseSandbox, index: u32) { + let baseline = sbox.snapshot().unwrap(); + let original: u64 = sbox.call("ReadMSR", index).unwrap(); + let jump = original.wrapping_add(1 << 60); + + sbox.call::<()>("WriteMSR", (index, jump)).unwrap(); + let written: u64 = sbox.call("ReadMSR", index).unwrap(); + assert!( + written >= jump / 2, + "0x{index:X}: guest write did not stick" + ); + + sbox.restore(baseline).unwrap(); + let restored: u64 = sbox.call("ReadMSR", index).unwrap(); + assert!( + restored < jump / 2, + "0x{index:X}: restore did not pull the counter below the guest-written jump" + ); + } + + /// Verifies that a guest MSR write faults or resets to its baseline. + #[cfg(target_arch = "x86_64")] + fn assert_msr_write_does_not_survive_restore( + sbox: &mut MultiUseSandbox, + msr: u32, + sentinel: u64, + ) { + let baseline = sbox.snapshot().unwrap(); + let original: u64 = match sbox.call("ReadMSR", msr) { + Ok(v) => v, + Err(_) => { + assert!( + sbox.poisoned(), + "0x{msr:X}: a faulting RDMSR should poison the sandbox" + ); + sbox.restore(baseline).unwrap(); + return; + } + }; + assert_ne!( + original, sentinel, + "0x{msr:X}: sentinel must differ from baseline" + ); + + if sbox.call::<()>("WriteMSR", (msr, sentinel)).is_err() { + assert!( + sbox.poisoned(), + "0x{msr:X}: a faulting WRMSR should poison the sandbox" + ); + sbox.restore(baseline).unwrap(); + return; + } + + sbox.restore(baseline).unwrap(); + let after: u64 = sbox.call("ReadMSR", msr).unwrap(); + assert_eq!( + after, original, + "0x{msr:X}: MSR leaked across restore (expected 0x{original:X}, got 0x{after:X})" + ); + } + + /// Audits Hyper-V MSR bitmap ranges for guest state retained by restore. + #[test] + #[ignore = "slow host-dependent hardware MSR audit"] + #[cfg(target_arch = "x86_64")] + fn test_no_msr_leaks_across_restore_full_window_sweep() { + // Free-running counters use a magnitude check after restore. + const FREE_RUNNING: &[u32] = &[ + 0x10, // IA32_TIME_STAMP_COUNTER + 0xE7, // IA32_MPERF + 0xE8, // IA32_APERF + ]; + + #[cfg(kvm)] + if matches!( + crate::hypervisor::virtual_machine::get_available_hypervisor(), + Some(crate::hypervisor::virtual_machine::HypervisorType::Kvm) + ) { + return; + } + + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + let baseline = sbox.snapshot().unwrap(); + + // At least one retained write must exercise restore. + let mut readable = 0usize; + let mut exercised: Vec = Vec::new(); + let mut read_only: Vec = Vec::new(); + let mut masked_only: Vec = Vec::new(); + // Collect all free-running leaks for one diagnostic. + let mut free_running_leaked: Vec = Vec::new(); + + // Architectural and low model-specific indices. + let low = 0x0000_0000u32..=0x0000_1FFF; + // Hyper-V synthetic indices. + let hyperv_synthetic = 0x4000_0000u32..=0x4000_1FFF; + // Extended and AMD model-specific indices. + let extended = 0xC000_0000u32..=0xC001_FFFF; + let windows = low.chain(hyperv_synthetic).chain(extended); + for msr in windows { + let original: u64 = match sbox.call("ReadMSR", msr) { + Ok(v) => v, + Err(_) => { + sbox.restore(baseline.clone()).unwrap(); + continue; + } + }; + readable += 1; + + // A large jump distinguishes reset from normal counter progress. + if FREE_RUNNING.contains(&msr) { + let jump = original.wrapping_add(1 << 60); + if sbox.call::<()>("WriteMSR", (msr, jump)).is_err() { + sbox.restore(baseline.clone()).unwrap(); + read_only.push(msr); + continue; + } + let planted = match sbox.call::("ReadMSR", msr) { + Ok(v) => v, + Err(_) => { + sbox.restore(baseline.clone()).unwrap(); + masked_only.push(msr); + continue; + } + }; + if planted < jump / 2 { + sbox.restore(baseline.clone()).unwrap(); + masked_only.push(msr); + continue; + } + sbox.restore(baseline.clone()).unwrap(); + let after: u64 = sbox.call("ReadMSR", msr).unwrap(); + if after < jump / 2 { + exercised.push(msr); + } else { + free_running_leaked.push(msr); + } + continue; + } + + // Multiple candidates cover MSRs with restricted writable bits. + let candidates = [ + original ^ 0x55, + original ^ 0x1, + original ^ (1 << 12), + original ^ (1 << 20), + original ^ (1 << 32), + original.wrapping_add(1), + 0, + ]; + let mut planted = false; + let mut saw_write = false; + for cand in candidates { + if cand == original { + continue; + } + if sbox.call::<()>("WriteMSR", (msr, cand)).is_err() { + sbox.restore(baseline.clone()).unwrap(); + continue; + } + saw_write = true; + match sbox.call::("ReadMSR", msr) { + Ok(v) if v != original => { + planted = true; + break; + } + _ => { + sbox.restore(baseline.clone()).unwrap(); + } + } + } + + if planted { + sbox.restore(baseline.clone()).unwrap(); + match sbox.call::("ReadMSR", msr) { + Ok(after) => assert_eq!( + after, original, + "0x{msr:X}: a guest MSR write leaked across restore \ + (expected 0x{original:X}, got 0x{after:X})" + ), + Err(e) => panic!("0x{msr:X}: read-back after restore failed: {e:?}"), + } + exercised.push(msr); + } else if saw_write { + masked_only.push(msr); + } else { + read_only.push(msr); + } + } + + let fmt = |v: &[u32]| { + v.iter() + .map(|m| format!("0x{m:X}")) + .collect::>() + .join(", ") + }; + eprintln!( + "full-window MSR sweep: readable={readable} exercised={} masked_only={} read_only={}", + exercised.len(), + masked_only.len(), + read_only.len() + ); + eprintln!(" exercised: [{}]", fmt(&exercised)); + eprintln!(" masked_only: [{}]", fmt(&masked_only)); + eprintln!(" read_only: [{}]", fmt(&read_only)); + eprintln!(" free_running_leaked: [{}]", fmt(&free_running_leaked)); + assert!( + free_running_leaked.is_empty(), + "free-running MSRs not reset across restore on this backend: [{}]", + fmt(&free_running_leaked) + ); + assert!( + !exercised.is_empty(), + "sweep was vacuous: no guest MSR write ever retained a value that restore \ + then rolled back, so the rollback path was never exercised" + ); + } + + /// A host TSC write must reset the guest-visible MSHV counter. + #[test] + #[cfg(all(mshv3, target_arch = "x86_64"))] + fn mshv_host_tsc_writeback_resets_guest_tsc() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Mshv)) { + return; + } + + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + let base = sbox.vm.capture_msrs_for_test(&[0x10]).unwrap()[0].value; + + let jump = base.wrapping_add(1 << 60); + sbox.call::<()>("WriteMSR", (0x10u32, jump)).unwrap(); + let planted: u64 = sbox.call("ReadMSR", 0x10u32).unwrap(); + assert!( + planted >= jump, + "guest TSC write did not take (planted=0x{planted:X} jump=0x{jump:X})" + ); + + assert!( + sbox.vm.try_set_msr_for_test(0x10, base), + "host set of HV_X64_REGISTER_TSC failed" + ); + + let after: u64 = sbox.call("ReadMSR", 0x10u32).unwrap(); + eprintln!( + "mshv TSC writeback probe: base=0x{base:X} jump=0x{jump:X} planted=0x{planted:X} after=0x{after:X}" + ); + assert!( + after < jump / 2, + "host TSC write-back did NOT reset the guest TSC (after=0x{after:X} still near \ + jump=0x{jump:X}); the reset approach is not viable on this host" + ); + } + } + /// Tests for [`MultiUseSandbox::from_snapshot`] in-memory. mod from_snapshot { use std::sync::Arc; diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs index fef71263a..4286cb993 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs @@ -21,6 +21,8 @@ use serde::{Deserialize, Serialize}; use super::media_types::SNAPSHOT_ABI_VERSION; use crate::hypervisor::regs::CommonSpecialRegisters; +#[cfg(target_arch = "x86_64")] +use crate::hypervisor::regs::MsrEntry; use crate::mem::layout::SandboxMemoryLayout; // --- Arch and hypervisor identifiers -------------------------------- @@ -188,6 +190,17 @@ pub(super) struct OciSnapshotConfig { /// Special registers captured from the paused vCPU, restored /// verbatim when resuming the call. pub(super) sregs: CommonSpecialRegisters, + /// MSR reset state captured from the paused vCPU. `None` uses the + /// destination sandbox's reset set. + #[cfg(target_arch = "x86_64")] + #[serde(default)] + pub(super) msrs: Option>, + /// Allow list of the sandbox that captured this snapshot. Present + /// exactly when `msrs` is, and checked against the destination allow + /// list on load. + #[cfg(target_arch = "x86_64")] + #[serde(default)] + pub(super) allowed_msrs: Option>, pub(super) layout: MemoryLayout, /// Total size of the memory blob in bytes (including the guest /// page-table tail, if any). Equal to `self.memory.mem_size()`. @@ -636,6 +649,49 @@ mod tests { assert_eq!(restored, expected); } + /// Captured MSRs survive the serde round-trip through the config, + /// including index and value for every entry. + #[cfg(target_arch = "x86_64")] + #[test] + fn msrs_round_trip_preserves_every_entry() { + let original = gating_config_with_msrs(Some(vec![ + MsrEntry { + index: 0xC000_0102, + value: 0xDEAD_BEEF, + }, + MsrEntry { + index: 0x10, + value: 0x1234_5678_9ABC_DEF0, + }, + ])); + let json = serde_json::to_vec(&original).unwrap(); + let restored: OciSnapshotConfig = serde_json::from_slice(&json).unwrap(); + assert_eq!(restored.msrs, original.msrs); + } + + /// A config JSON with no MSR keys deserializes both fields to `None`. + #[cfg(target_arch = "x86_64")] + #[test] + fn config_without_msr_keys_deserializes_to_none() { + let with = gating_config_with_msrs(Some(vec![MsrEntry { + index: 0x10, + value: 1, + }])); + let mut json: serde_json::Value = + serde_json::from_slice(&serde_json::to_vec(&with).unwrap()).unwrap(); + assert!(json.as_object_mut().unwrap().remove("msrs").is_some()); + assert!( + json.as_object_mut() + .unwrap() + .remove("allowed_msrs") + .is_some() + ); + + let restored: OciSnapshotConfig = serde_json::from_value(json).unwrap(); + assert_eq!(restored.msrs, None); + assert_eq!(restored.allowed_msrs, None); + } + /// Every `ParameterType` survives the round-trip through its serde /// mirror, guarding against a transposed variant in either match. #[test] @@ -721,6 +777,10 @@ mod tests { entrypoint_addr: SandboxMemoryLayout::BASE_ADDRESS as u64, original_entrypoint_addr: 0, sregs: distinct_sregs(), + #[cfg(target_arch = "x86_64")] + msrs: None, + #[cfg(target_arch = "x86_64")] + allowed_msrs: None, layout: MemoryLayout { input_data_size: 0, output_data_size: 0, @@ -738,6 +798,17 @@ mod tests { } } + /// `gating_config` with a chosen MSR set, for serde tests. + #[cfg(target_arch = "x86_64")] + fn gating_config_with_msrs(msrs: Option>) -> OciSnapshotConfig { + let allowed_msrs = msrs.as_ref().map(|_| Vec::new()); + OciSnapshotConfig { + msrs, + allowed_msrs, + ..gating_config() + } + } + /// A snapshot built for a different architecture is rejected. #[test] fn validate_for_load_rejects_arch_mismatch() { @@ -934,6 +1005,20 @@ mod schema_pin { 11 ] }, + "msrs": [ + { + "index": 16, + "value": 42 + }, + { + "index": 3221225474, + "value": 3735928559 + } + ], + "allowed_msrs": [ + 16, + 3221225474 + ], "layout": { "input_data_size": 1, "output_data_size": 2, diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs index 7ea72d919..9e03873a4 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs @@ -611,6 +611,10 @@ impl Snapshot { entrypoint_addr, original_entrypoint_addr: self.original_entrypoint, sregs: *sregs, + #[cfg(target_arch = "x86_64")] + msrs: self.msrs.clone(), + #[cfg(target_arch = "x86_64")] + allowed_msrs: self.allowed_msrs.clone(), layout: MemoryLayout { input_data_size: l.input_data_size(), output_data_size: l.output_data_size(), @@ -876,6 +880,15 @@ impl Snapshot { // 8. Build the next action + sregs back from the config. let next_action = NextAction::Call(cfg.entrypoint_addr); + // `msrs` and `allowed_msrs` travel together. A config with one but + // not the other cannot enforce the allow-list check on restore. + #[cfg(target_arch = "x86_64")] + if cfg.msrs.is_some() != cfg.allowed_msrs.is_some() { + return Err(crate::new_error!( + "snapshot config inconsistent: msrs and allowed_msrs must both be present or both absent" + )); + } + // 9. Reconstitute host_functions metadata. let snapshot_generation = cfg.snapshot_generation; let host_funcs_vec: Vec< @@ -897,6 +910,10 @@ impl Snapshot { load_info: crate::mem::exe::LoadInfo::dummy(), stack_top_gva: cfg.stack_top_gva, sregs: Some(cfg.sregs), + #[cfg(target_arch = "x86_64")] + msrs: cfg.msrs, + #[cfg(target_arch = "x86_64")] + allowed_msrs: cfg.allowed_msrs, next_action, original_entrypoint: cfg.original_entrypoint_addr, snapshot_generation, diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index e577b0d77..6babfa3e4 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -208,6 +208,200 @@ fn snapshot_generation_round_trip() { assert_eq!(loaded3.snapshot_generation(), gen3); } +/// A disk round-trip preserves captured MSR reset state. +#[cfg(target_arch = "x86_64")] +#[test] +fn msrs_round_trip_via_disk() { + let snap = create_snapshot(); + let original = snap.msrs().cloned(); + assert!( + original.as_ref().is_some_and(|m| !m.is_empty()), + "a running snapshot should capture a non-empty MSR reset set" + ); + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("snap"); + snap.save(&path, &OciTag::new("latest").unwrap()).unwrap(); + + let loaded = Snapshot::checked_load(&path, OciTag::new("latest").unwrap()).unwrap(); + assert_eq!(loaded.msrs(), original.as_ref()); + // The allow list travels with the MSRs and is Some exactly when they are. + assert_eq!(loaded.allowed_msrs(), Some(&[][..])); +} + +/// A config with no `msrs` key loads and uses the destination reset set. +#[cfg(target_arch = "x86_64")] +#[test] +fn snapshot_without_msrs_key_loads_and_runs() { + let (_dir, path) = save_for_mutation(); + rewrite_config(&path, |cfg| { + let obj = cfg.as_object_mut().unwrap(); + assert!( + obj.remove("msrs").is_some(), + "a running x86_64 snapshot config should carry msrs to remove" + ); + assert!( + obj.remove("allowed_msrs").is_some(), + "a running x86_64 snapshot config should carry allowed_msrs to remove" + ); + }); + + let loaded = Snapshot::checked_load(&path, OciTag::new("latest").unwrap()).unwrap(); + assert_eq!(loaded.msrs(), None); + + let mut sbox = + MultiUseSandbox::from_snapshot(Arc::new(loaded), HostFunctions::default(), None).unwrap(); + assert_eq!(sbox.call::("GetStatic", ()).unwrap(), 0); +} + +/// A snapshot whose reset set includes an allowed MSR carries that +/// MSR's guest-written value across a disk round-trip. Loading with a +/// matching allow config restores the value, and restore-in-place +/// returns it after an overwrite. +#[cfg(target_arch = "x86_64")] +#[test] +fn disk_snapshot_restores_allowed_msr_value() { + const SYSENTER_CS: u32 = 0x174; + let sentinel: u64 = 0xDEAD_BEEF; + + let mut cfg = crate::sandbox::SandboxConfiguration::default(); + cfg.allow_msrs(&[SYSENTER_CS]).unwrap(); + let mut source = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().unwrap()), + Some(cfg), + ) + .unwrap() + .evolve() + .unwrap(); + source + .call::<()>("WriteMSR", (SYSENTER_CS, sentinel)) + .unwrap(); + let snap = source.snapshot().unwrap(); + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("snap"); + snap.save(&path, &OciTag::new("latest").unwrap()).unwrap(); + let loaded = Arc::new(Snapshot::checked_load(&path, OciTag::new("latest").unwrap()).unwrap()); + + let mut cfg = crate::sandbox::SandboxConfiguration::default(); + cfg.allow_msrs(&[SYSENTER_CS]).unwrap(); + let mut sbox = + MultiUseSandbox::from_snapshot(loaded.clone(), HostFunctions::default(), Some(cfg)) + .unwrap(); + assert_eq!(sbox.call::("ReadMSR", SYSENTER_CS).unwrap(), sentinel); + + sbox.call::<()>("WriteMSR", (SYSENTER_CS, sentinel ^ 0x55)) + .unwrap(); + sbox.restore(loaded).unwrap(); + assert_eq!(sbox.call::("ReadMSR", SYSENTER_CS).unwrap(), sentinel); +} + +/// A disk snapshot restores into a destination with a superset allow list. +#[cfg(target_arch = "x86_64")] +#[test] +fn disk_snapshot_restores_into_superset_allow_list() { + const SYSENTER_CS: u32 = 0x174; + const SYSENTER_ESP: u32 = 0x175; + let sentinel: u64 = 0xDEAD_BEEF; + + let mut cfg = crate::sandbox::SandboxConfiguration::default(); + cfg.allow_msrs(&[SYSENTER_CS]).unwrap(); + let mut source = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().unwrap()), + Some(cfg), + ) + .unwrap() + .evolve() + .unwrap(); + source + .call::<()>("WriteMSR", (SYSENTER_CS, sentinel)) + .unwrap(); + let snap = source.snapshot().unwrap(); + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("snap"); + snap.save(&path, &OciTag::new("latest").unwrap()).unwrap(); + let loaded = Arc::new(Snapshot::checked_load(&path, OciTag::new("latest").unwrap()).unwrap()); + + let mut dest_cfg = crate::sandbox::SandboxConfiguration::default(); + dest_cfg.allow_msrs(&[SYSENTER_CS, SYSENTER_ESP]).unwrap(); + let mut sbox = + MultiUseSandbox::from_snapshot(loaded, HostFunctions::default(), Some(dest_cfg)).unwrap(); + assert_eq!(sbox.call::("ReadMSR", SYSENTER_CS).unwrap(), sentinel); +} + +/// A disk snapshot is rejected when it allows an MSR the destination does not. +#[cfg(target_arch = "x86_64")] +#[test] +fn disk_snapshot_non_superset_allow_list_rejected() { + const SYSENTER_CS: u32 = 0x174; + let sentinel: u64 = 0x1234; + + let mut cfg = crate::sandbox::SandboxConfiguration::default(); + cfg.allow_msrs(&[SYSENTER_CS]).unwrap(); + let mut source = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().unwrap()), + Some(cfg), + ) + .unwrap() + .evolve() + .unwrap(); + source + .call::<()>("WriteMSR", (SYSENTER_CS, sentinel)) + .unwrap(); + let snap = source.snapshot().unwrap(); + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("snap"); + snap.save(&path, &OciTag::new("latest").unwrap()).unwrap(); + let loaded = Arc::new(Snapshot::checked_load(&path, OciTag::new("latest").unwrap()).unwrap()); + + // The destination allows nothing, so the snapshot's allowed MSR is not + // a subset of the destination allow list. + let err = MultiUseSandbox::from_snapshot(loaded.clone(), HostFunctions::default(), None) + .expect_err("loading into a non-superset allow list must fail"); + assert!( + format!("{err:?}").contains("SnapshotMsrNotAllowed"), + "expected an MSR allow-list mismatch, got: {err:?}" + ); + + let mut target = create_test_sandbox(); + let err = target + .restore(loaded) + .expect_err("restore into a non-superset allow list must fail"); + assert!( + format!("{err:?}").contains("SnapshotMsrNotAllowed"), + "expected an MSR allow-list mismatch, got: {err:?}" + ); + assert!(target.poisoned()); +} + +/// A config with `msrs` but no `allowed_msrs` cannot enforce the +/// allow-list check, so the load rejects it. +#[cfg(target_arch = "x86_64")] +#[test] +fn disk_snapshot_msrs_without_allow_list_rejected() { + let (_dir, path) = save_for_mutation(); + rewrite_config(&path, |cfg| { + assert!( + cfg.as_object_mut() + .unwrap() + .remove("allowed_msrs") + .is_some(), + "a running x86_64 snapshot config should carry allowed_msrs to remove" + ); + }); + + let err = unwrap_err_snapshot(Snapshot::checked_load( + &path, + OciTag::new("latest").unwrap(), + )); + assert!( + format!("{err:?}").contains("allowed_msrs"), + "expected an msrs/allowed_msrs consistency error, got: {err:?}" + ); +} + #[test] fn pre_init_snapshot_cannot_be_persisted() { let snap = Snapshot::from_env( diff --git a/src/hyperlight_host/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index c062efad7..c3cfc2c17 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -32,6 +32,8 @@ use tracing::{Span, instrument}; use crate::Result; use crate::hypervisor::regs::CommonSpecialRegisters; +#[cfg(target_arch = "x86_64")] +use crate::hypervisor::regs::MsrEntry; use crate::mem::exe::{ExeInfo, LoadInfo}; use crate::mem::layout::SandboxMemoryLayout; use crate::mem::memory_region::{GuestMemoryRegion, MemoryRegion, MemoryRegionFlags}; @@ -92,6 +94,18 @@ pub struct Snapshot { /// tables are relocated during snapshot. sregs: Option, + /// MSR reset state captured from a running vCPU. + /// None for a pre-init snapshot or an old on-disk snapshot + /// predating MSR capture. + #[cfg(target_arch = "x86_64")] + msrs: Option>, + + /// The allow list of the sandbox that captured this snapshot, + /// sorted and deduplicated. Present exactly when `msrs` is present. + /// A restore requires the destination allow list to be a superset. + #[cfg(target_arch = "x86_64")] + allowed_msrs: Option>, + /// The next action that should be performed on this snapshot next_action: NextAction, @@ -392,6 +406,10 @@ impl Snapshot { load_info, stack_top_gva: exn_stack_top_gva, sregs: None, + #[cfg(target_arch = "x86_64")] + msrs: None, + #[cfg(target_arch = "x86_64")] + allowed_msrs: None, next_action: NextAction::Initialise(entrypoint_gva), original_entrypoint: entrypoint_gva, snapshot_generation: 0, @@ -419,6 +437,8 @@ impl Snapshot { root_pt_gpas: &[u64], stack_top_gva: u64, sregs: CommonSpecialRegisters, + #[cfg(target_arch = "x86_64")] msrs: Option>, + #[cfg(target_arch = "x86_64")] allowed_msrs: Option>, next_action: NextAction, original_entrypoint: u64, snapshot_generation: u64, @@ -573,6 +593,10 @@ impl Snapshot { load_info, stack_top_gva, sregs: Some(sregs), + #[cfg(target_arch = "x86_64")] + msrs, + #[cfg(target_arch = "x86_64")] + allowed_msrs, next_action, original_entrypoint, snapshot_generation, @@ -617,6 +641,30 @@ impl Snapshot { self.sregs.as_ref() } + /// Returns the captured MSR reset state. + #[cfg(target_arch = "x86_64")] + pub(crate) fn msrs(&self) -> Option<&Vec> { + self.msrs.as_ref() + } + + /// Returns the capturing sandbox's allow list. + #[cfg(target_arch = "x86_64")] + pub(crate) fn allowed_msrs(&self) -> Option<&[u32]> { + self.allowed_msrs.as_deref() + } + + /// Stores captured MSR reset state. + #[cfg(all(test, target_arch = "x86_64"))] + pub(crate) fn set_msrs(&mut self, msrs: Option>) { + self.msrs = msrs; + } + + /// Stores the capturing sandbox's allow list. + #[cfg(all(test, target_arch = "x86_64"))] + pub(crate) fn set_allowed_msrs(&mut self, allowed: Option>) { + self.allowed_msrs = allowed; + } + pub(crate) fn next_action(&self) -> NextAction { self.next_action } @@ -782,6 +830,10 @@ mod tests { &[pt_base], 0, default_sregs(), + #[cfg(target_arch = "x86_64")] + None, + #[cfg(target_arch = "x86_64")] + None, super::NextAction::None, 0, 1, @@ -800,6 +852,10 @@ mod tests { &[pt_base], 0, default_sregs(), + #[cfg(target_arch = "x86_64")] + None, + #[cfg(target_arch = "x86_64")] + None, super::NextAction::None, 0, 2, diff --git a/src/tests/rust_guests/simpleguest/src/main.rs b/src/tests/rust_guests/simpleguest/src/main.rs index 2da2eb7ca..efe6badab 100644 --- a/src/tests/rust_guests/simpleguest/src/main.rs +++ b/src/tests/rust_guests/simpleguest/src/main.rs @@ -1095,6 +1095,87 @@ fn call_host_expect_error(hostfuncname: String) -> Result<()> { Ok(()) } +#[guest_function("ReadMSR")] +#[cfg(target_arch = "x86_64")] +fn read_msr(msr: u32) -> u64 { + let (read_eax, read_edx): (u32, u32); + // SAFETY: The test guest runs at CPL0, and the operands declare every register used. + unsafe { + core::arch::asm!( + "rdmsr", + in("ecx") msr, + out("eax") read_eax, + out("edx") read_edx, + options(nostack, nomem) + ); + } + ((read_edx as u64) << 32) | (read_eax as u64) +} + +#[guest_function("WriteKernelGsBaseViaSwapgs")] +#[cfg(target_arch = "x86_64")] +fn write_kernel_gs_base_via_swapgs(value: u64) { + // SAFETY: The test guest runs at CPL0, and CR4 is restored before returning. + unsafe { + core::arch::asm!( + "mov {original_cr4}, cr4", + "mov {enabled_cr4}, {original_cr4}", + "or {enabled_cr4}, {fsgsbase}", + "mov cr4, {enabled_cr4}", + "wrgsbase {value}", + "swapgs", + "mov cr4, {original_cr4}", + original_cr4 = out(reg) _, + enabled_cr4 = out(reg) _, + fsgsbase = const 1 << 16, + value = in(reg) value, + options(nostack) + ); + } +} + +#[guest_function("ReadKernelGsBaseViaSwapgs")] +#[cfg(target_arch = "x86_64")] +fn read_kernel_gs_base_via_swapgs() -> u64 { + let value: u64; + // SAFETY: The test guest runs at CPL0, and SWAPGS and CR4 are restored before returning. + unsafe { + core::arch::asm!( + "mov {original_cr4}, cr4", + "mov {enabled_cr4}, {original_cr4}", + "or {enabled_cr4}, {fsgsbase}", + "mov cr4, {enabled_cr4}", + "swapgs", + "rdgsbase {value}", + "swapgs", + "mov cr4, {original_cr4}", + original_cr4 = out(reg) _, + enabled_cr4 = out(reg) _, + value = lateout(reg) value, + fsgsbase = const 1 << 16, + options(nostack) + ); + } + value +} + +#[guest_function("WriteMSR")] +#[cfg(target_arch = "x86_64")] +fn write_msr(msr: u32, value: u64) { + let eax = (value & 0xFFFFFFFF) as u32; + let edx = ((value >> 32) & 0xFFFFFFFF) as u32; + // SAFETY: The test guest runs at CPL0, and the operands declare every register used. + unsafe { + core::arch::asm!( + "wrmsr", + in("ecx") msr, + in("eax") eax, + in("edx") edx, + options(nostack, nomem) + ); + } +} + #[hyperlight_guest_bin::main] #[instrument(skip_all, parent = Span::current(), level= "Trace")] fn main() { From 0bd2a0a279389b741a1cb57be1f19f2b6f6f3131 Mon Sep 17 00:00:00 2001 From: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> Date: Thu, 23 Jul 2026 09:15:35 -0700 Subject: [PATCH 2/4] Pr feedback Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> --- docs/msr.md | 443 ++++++++---------- docs/snapshot-versioning.md | 13 +- src/hyperlight_host/src/error.rs | 14 - .../src/hypervisor/hyperlight_vm/mod.rs | 24 - .../src/hypervisor/hyperlight_vm/x86_64.rs | 95 +--- .../src/hypervisor/regs/x86_64/msrs.rs | 59 ++- .../hypervisor/regs/x86_64/special_regs.rs | 1 + .../hypervisor/virtual_machine/kvm/x86_64.rs | 214 ++------- .../src/hypervisor/virtual_machine/mod.rs | 48 +- .../hypervisor/virtual_machine/mshv/x86_64.rs | 20 +- .../src/hypervisor/virtual_machine/whp.rs | 89 ++-- src/hyperlight_host/src/mem/mgr.rs | 9 +- src/hyperlight_host/src/sandbox/config.rs | 15 +- .../src/sandbox/initialized_multi_use.rs | 418 ++++++++--------- .../src/sandbox/snapshot/file/config.rs | 91 ++-- .../src/sandbox/snapshot/file/mod.rs | 33 +- .../src/sandbox/snapshot/file_tests.rs | 22 +- .../src/sandbox/snapshot/mod.rs | 63 +-- src/tests/rust_guests/simpleguest/src/main.rs | 12 + 19 files changed, 679 insertions(+), 1004 deletions(-) diff --git a/docs/msr.md b/docs/msr.md index 316f097c5..636647001 100644 --- a/docs/msr.md +++ b/docs/msr.md @@ -2,303 +2,228 @@ ## Requirement -A snapshot restore must remove all model-specific register (MSR) state written -after the snapshot. The guest must observe the MSR values saved with the -restored state. +A snapshot represents the state of a VM at a point in time. This includes MSR +state that can affect later execution. -## Reset set +After `MultiUseSandbox::restore`, the destination sandbox's MSR state must match +the supplied snapshot, regardless of prior execution in the sandbox. -The reset set contains every MSR whose guest-written value can persist. Each -running snapshot stores values for this set. A snapshot created from a guest -binary has no saved MSR values, so restore uses the baseline captured when the -VM was created. +## How restore works -`MSR_TABLE` lists the MSRs that hold retained state restore must write. A -write-only command MSR holds no state, so it is absent from the table. +Each backend provides the MSR indices it must reset. VM creation reads these +indices before guest execution and stores the values as the destination +baseline. -The resolved reset set contains the backend core set, required MTRRs, and the -validated allow list. Hyperlight sorts and deduplicates the indices before -capturing the initialization baseline. +A snapshot captured from a VM stores: -The required invariant is: +* The value of every MSR in the source VM's reset set. +* The source sandbox's MSR allow list. -```text -guest-writable retained state => host-readable and host-writable state -``` +Restore resolves the snapshot against the destination reset set. A value stored +in the snapshot is restored directly. An index present only in the destination +uses the destination baseline. This supports restoring into a sandbox whose +allow list is a superset of the source allow list. -Host-readable state need not be guest-writable. Extra reset entries are safe. -`EFER`, `APIC_BASE`, `FS_BASE`, and `GS_BASE` belong to the special-register -state. +The backend writes the complete resolved set before guest execution resumes. A +read, validation, or write failure aborts restore and poisons the sandbox. -The two halves are established differently. Resolution checks the -host-readable half at run time: a candidate index enters the set only when the -host read succeeds, so an unreadable MSR is dropped. Nothing checks the -host-writable half at run time. It holds by construction. Every reset MSR is -stored in VP register state that the Hyper-V host interface both reads and -writes, except `KERNEL_GS_BASE`, a real register the host reads and writes -directly. Round-trip tests plant a guest value, restore, and assert that it does -not survive. Every future entry must remain host-readable and host-writable. +## Why the reset set is backend-specific -## Reset set justification +KVM can deny individual guest `RDMSR` and `WRMSR` operations. Its reset set can +therefore be limited to allowed MSRs and state changed through other CPU +instructions. -Each entry is grounded in how the Hyper-V hypervisor handles a guest access to -that register, confirmed against the Hyper-V source. +MSHV and WHP do not provide Hyperlight with an equivalent per-MSR filter. +Their reset sets must include every retained MSR state reachable through the +partition's exposed CPU features. Hyperlight keeps a shared candidate table for +these Hyper-V backends and lets each backend select entries it can map and read. -A register is reset when the guest can write it and Hyper-V keeps the written -value. Hyper-V keeps it in one of two ways: it stores the value in the VP's -saved register state, or it lets the guest write the real register directly. -Only `FS_BASE`, `GS_BASE`, and `KERNEL_GS_BASE` are written directly. Every -other register below is intercepted and stored. Hyper-V stores the value on both -Intel and AMD hosts. +The backend owns discovery because register mappings and capabilities differ. +Sorting, deduplication, baseline capture, snapshot validation, and fallback to +the destination baseline are shared in `MsrResetState`. -Interception alone is not the test. Hyper-V also intercepts registers the guest -can only read, or that return a host-derived value. Those keep no -guest-controlled state and are not reset. Each row below names the guest state -that persists. +Every reset entry must represent guest-writable retained state that the host can +read and write. The shared candidate table is derived from the Hyper-V source +and must be audited when that source, register mappings, or feature exposure +changes. VM creation probes host reads. Host write support currently holds by +table construction and round-trip tests. -| MSR (index) | Retained guest state | -| --- | --- | -| SYSENTER CS, ESP, EIP (`0x174`-`0x176`) | Guest write retained. | -| STAR, LSTAR, CSTAR, SFMASK (`0xC000_0081`-`0xC000_0084`) | Guest write retained (syscall targets). | -| KERNEL_GS_BASE (`0xC000_0102`) | Guest write retained. Written to the real register, and reachable through `SWAPGS` without a `WRMSR`. | -| PAT (`0x277`) | Guest write retained. | -| DEBUGCTL (`0x1D9`) | Guest write retained. | -| SPEC_CTRL (`0x48`) | Guest write retained. | -| CET U_CET, S_CET, PL0-3_SSP, INTERRUPT_SSP_TABLE_ADDR (`0x6A0`, `0x6A2`, `0x6A4`-`0x6A8`) | Guest write retained. | -| XSS (`0xDA0`) | Guest write retained. | -| TSC (`0x10`) | Guest write retained. Hyper-V forbids intercepting its implemented TSC, so restore rewrites the captured value. | -| TSC_ADJUST (`0x3B`) | Guest write retained, independent of TSC. | -| TSC_AUX (`0xC000_0103`) | Guest write retained. | -| MTRRs (`0x2FF`, `0x200`-`0x21F`, `0x250`, `0x258`-`0x259`, `0x268`-`0x26F`) | Guest write retained (memory-type state). | -| TSX_CTRL (`0x122`) | Guest write retained. | -| XFD, XFD_ERR (`0x1C4`, `0x1C5`) | Guest write retained. | -| UMWAIT_CONTROL (`0xE1`) | Guest write retained. Intel only. | -| TSC_DEADLINE (`0x6E0`) | Guest write retained. | -| BNDCFGS (`0xD90`) | Guest write retained when the host supports MPX. A guest access faults otherwise. | -| MPERF, APERF (`0xE7`, `0xE8`) | Guest write retained in per-VP counters. | - -Write-only command MSRs hold no state. A guest write performs an action and -leaves nothing to restore, so they are absent from the reset table and cannot -be allowed. - -| MSR (index) | Behavior | -| --- | --- | -| PRED_CMD (`0x49`) | Guest write issues a prediction barrier. | -| FLUSH_CMD (`0x10B`) | Guest write flushes caches. | +## Snapshot validation and access policy + +Snapshot state is untrusted. Every `MsrEntry` stored in a snapshot must name an +index in the destination VM's reset set. This prevents a snapshot from using +the backend register interface to write arbitrary host-visible registers. + +The allow list is separate from the captured values: + +* `msrs` contains the complete source reset state. +* `allowed_msrs` records which MSRs the source guest could access through the + configured policy. -Some registers are deliberately absent because guest access cannot leave state -outside another reset mechanism. +The source allow list must be a subset of the destination allow list. The +destination configuration remains authoritative. Snapshot data cannot expand +the destination policy or replace its KVM filter. -| MSR (index) | Why excluded | +`SandboxConfiguration::allow_msrs` accepts at most 16 distinct indices. KVM +also supports at most 16 contiguous filter ranges. Each allowed index must be +resettable, host-readable, and host-writable. Write-only command MSRs such as +`PRED_CMD` and `FLUSH_CMD` hold no resettable state and cannot be allowed. + +MSHV and WHP cannot enforce the allow list during guest execution. They retain +it in snapshots so restore compatibility has the same meaning on every +backend. + +## State restored elsewhere + +Not all architecturally MSR-backed state uses the MSR reset path. + +| State | Restore owner | | --- | --- | -| MISC_ENABLE (`0x1A0`) | Intercepted, but Hyper-V discards a guest write and returns a fixed value. No retained state. On AMD the access faults. See below. | -| FRED (`0x1CC`-`0x1D4`) | Retained only when the host exposes FRED, which Hyperlight does not. A guest access faults otherwise. | -| PASID (`0xD93`) | MSHV exposes ENQCMD on capable Intel hosts. PASID is a supervisor XSAVE component, so XSAVE reset clears it. WHP does not expose ENQCMD. | -| PMU: PMC0, PERFEVTSEL0, FIXED_CTR_CTRL, PERF_GLOBAL_CTRL (`0xC1`, `0x186`, `0x38D`, `0x38F`) | Heads of the performance-monitoring class. Hyper-V leaves these unimplemented for the guest and installs guest-accessible descriptors, sized to the CPU counter count, only when perfmon is enabled. Hyperlight enables no perfmon, so a guest access faults and retains nothing. | -| LBR: LBR_SELECT, LASTBRANCH_TOS, LBR_CTL, LBR_DEPTH (`0x1C8`, `0x1C9`, `0x14CE`, `0x14CF`) | Last-branch registers, gated with perfmon. A guest access faults and retains nothing while perfmon stays off. | +| `EFER`, `APIC_BASE`, `FS_BASE`, `GS_BASE` | Special-register snapshot state. | +| PASID (`0xD93`) on MSHV | XSAVE state. | + +Keeping one owner avoids restoring the same state through two backend APIs. -Hyper-V virtualizes `BNDCFGS`, `FRED`, and `PASID` only when the matching CPU -feature is exposed. `BNDCFGS` is reset because MPX is exposed by default on -capable hosts. `FRED` stays excluded because Hyperlight does not expose it. -MSHV can expose ENQCMD, but its XSAVE state mask then includes PASID. Hyperlight -clears PASID during XSAVE reset before restoring MSRs. The performance-monitoring -and last-branch registers remain inaccessible while perfmon is off. +## KVM -## Snapshot validation +KVM installs a default-deny MSR filter. The configured allow list supplies the +only permitted filter ranges. VM creation rejects an allowed index unless KVM +lists it and the host can read and write it. -Snapshot MSR entries are untrusted. A snapshot records the reset values and the -capturing sandbox's allow list. `validate_snapshot` enforces two rules against -the destination VM's reset set: +The KVM reset set contains: -* The snapshot's allow list must be a subset of the destination's. A - destination that allows at least as much accepts the snapshot. -* Every supplied index must belong to the destination reset set. +* Every allowed MSR. +* `KERNEL_GS_BASE`, because `WRGSBASE` followed by `SWAPGS` can change it + without `WRMSR`. +* `TSC`, so restore rewinds guest time on every backend. -Indices the destination resets but the snapshot omits take the destination's -creation-time baseline. A rejected restore poisons the sandbox before the guest -can run. Equivalent allow lists produce the same sorted reset set, regardless of -insertion order. +The default-deny filter also covers KVM's custom MSR namespace +`0x4B56_4D00..=0x4B56_4DFF`. -## Restore across allow lists +Some CPU state does not pass through the filter. Hyperlight addresses those +paths separately: -A restore or `from_snapshot` succeeds when the destination allow list is a -superset of the snapshot's, on every backend. The snapshot's allowed MSRs keep -their captured values. An MSR the destination allows but the snapshot did not -resets to the destination baseline. A non-superset allow list is rejected -uniformly. +* VMX and SVM are removed from guest CPUID, so nested VMCS and VMCB state is + unreachable. Their setup MSRs remain denied. +* Hyperlight keeps the APIC in xAPIC mode. Guest writes to `APIC_BASE` and the + x2APIC range `0x800..=0x8FF` are denied. Snapshot restore also rejects an + `APIC_BASE` value with the x2APIC enable bit set. +* `FS_BASE` and `GS_BASE` are restored through special-register state. -The rule is backend independent even though each backend sizes its reset set -differently. KVM derives its reset set from the allow list. MSHV and WHP reset -the full host table. The allow-list subset check gates the restore before either -reset set is applied, so a flow that succeeds on one backend succeeds on all. +A denied guest access raises `#GP`. Hyperlight reports `GuestAborted` and +poisons the sandbox. -The superset check is the common rule across backends. MSHV and WHP accept any -allow list on their own. The shared check gives every backend KVM's constraint. +## Hyper-V backends -## Allow list +MSHV and WHP build their reset sets from: -`SandboxConfiguration::allow_msrs` adds indices to the requested allow list. It -enforces capacity only. VM creation verifies that each index is resettable and -supported by the selected backend. +* Retained-state candidates that the backend maps and can read. +* MTRRs required by the virtual CPU's `MTRRCAP`. +* The validated allow list. -KVM requires the index in `KVM_GET_MSR_INDEX_LIST` and a successful host read -and write. MSHV and WHP require a named-register mapping and a successful host -read. +The shared candidate table is not an intercept list. Hyper-V also intercepts +read-only, command, and host-derived MSRs that retain no guest-controlled +value. An entry belongs in the table only when guest execution can leave state +that affects later execution. -At most 16 distinct MSRs may be requested. KVM also limits the resulting -contiguous filter groups to 16. +### MSHV -## KVM +MSHV enables the processor features supported by the host unless a partition +feature mask disables them. Its guest-visible MSR surface therefore varies by +host CPU. -KVM installs a deny filter over the full MSR space. Allowed indices form the -only guest `RDMSR` and `WRMSR` paths through that filter. A denied access exits -to Hyperlight, injects `#GP`, and poisons the sandbox. The denied write stores -no state. +MSHV maps `IA32_XSS` through `MSR_IA32_REGISTER_U_XSS`. It maps `IA32_MPERF` +and `IA32_APERF` through the per-VP `MCount` and `ACount` registers. TSX, +WAITPKG, CET, XFD, MPX, and deadline-timer state enter the reset set when the +host exposes and maps them. -The KVM reset set contains the allow list plus `KERNEL_GS_BASE` and `TSC`. -`KERNEL_GS_BASE` is required because `WRGSBASE` followed by `SWAPGS` changes it -without `WRMSR`. `TSC` gives restore the same clock semantics on every backend. +The host's enumerated MSR index list does not identify retained state, so it +does not define the reset set. -KVM does not filter x2APIC indices `0x800..=0x8FF`. Hyperlight keeps the APIC in -xAPIC mode, where MSR access to that range raises `#GP`. `APIC_BASE` is not an -allowable MSR, so a guest cannot enable x2APIC. Snapshots created by Hyperlight -therefore retain `APIC_BASE.EXTD = 0`. File snapshots serialize `APIC_BASE` -without semantic validation, so the caller must trust the snapshot source as -required by the snapshot format. +On capable Intel hosts MSHV can expose ENQCMD and PASID. PASID is a supervisor +XSAVE component, so XSAVE restore owns it. -## MTRRs +### WHP -MSHV and WHP read `IA32_MTRRCAP` when the VM is created. The required set -contains `MTRR_DEF_TYPE`, each variable pair reported by `VCNT`, and all fixed -MTRRs. +WHP's default feature banks disable speculation control, experimental +`DEBUGCTL` bits, and performance monitoring. Its supported feature mask omits +ENQCMD and defines no FRED feature, so WHP does not expose PASID or FRED. -Hyper-V accepts fixed-MTRR writes even when `MTRRCAP.FIX` is clear. All fixed -MTRRs are therefore required. Hyper-V supports at most 16 variable pairs. VM -creation fails when the count is larger or a required MTRR cannot be read. +WHP maps supported MSRs to `WHV_REGISTER_NAME` values. The same mapping is used +for snapshot reads and restore writes. -## MSHV +### MTRRs -MSHV has no per-MSR filter. Hyper-V permits an MSR intercept only for an -unimplemented index, which already faults for the guest, and cannot intercept -the implemented MSRs that hold retained state. Isolation therefore comes from -reset, not a deny filter. +MSHV and WHP read `IA32_MTRRCAP` during VM creation. The reset set includes +`MTRR_DEF_TYPE`, every variable base and mask pair reported by `VCNT`, and all +fixed MTRRs. Hyper-V accepts fixed-MTRR writes even when `MTRRCAP.FIX` is +clear. -The MSHV reset set contains every table entry that has a Hyper-V -register mapping and can be read, plus the allow list. +VM creation fails if `VCNT` exceeds the supported maximum of 16 or required +MTRRs cannot be read. -`msr_to_hv_reg_name` determines which indices the get and set path can reach. -The enumerated host index list does not identify retained state, so it does not -define the reset set. +### TSC -MSHV maps `IA32_XSS` through `MSR_IA32_REGISTER_U_XSS`. It maps `IA32_MPERF` -and `IA32_APERF` to the per-VP `MCount` and `ACount` registers. TSX control, -XFD, MPX (`BNDCFGS`), WAITPKG (`UMWAIT_CONTROL`), and the TSC deadline timer -enter the reset set when their host-register probes succeed. - -MSHV enables every host-supported processor feature unless the caller supplies -an explicit disabled-feature mask. Hyperlight supplies no mask. On capable -Intel hosts this can expose ENQCMD and its PASID MSR. MSHV reports PASID in the -partition XSAVE state mask, and Hyperlight's XSAVE reset clears it. - -## WHP - -WHP has no per-MSR filter. Its reset set contains every table entry -that has a WHP register name and can be read, plus the allow list. - -WHP uses Germanium compatibility. Speculation control is off in its default -feature banks, and perfmon (the PMU and architectural LBR) is a separate -property WHP leaves off. Experimental `DEBUGCTL` bits stay disabled. The WHP -API defines no FRED feature and its supported feature mask omits ENQCMD, so WHP -cannot expose FRED or PASID. - -Each guest MSR write is either captured for restore or unsupported by the -partition. Unsupported writes store no state. - -## TSC - -MSHV and WHP expose `TSC` as a host-writable register. Hyper-V stores `TSC` and -`TSC_ADJUST` independently, so restoring `TSC_ADJUST` cannot undo a guest -`WRMSR(TSC)`. - -While time is running, Hyper-V preserves `TSC - TSC_ADJUST`: writing `TSC` -adds the same delta to `TSC_ADJUST`, and writing `TSC_ADJUST` adds its delta to -the internal TSC offset. Restoring `TSC` followed by `TSC_ADJUST` therefore -cancels any guest-controlled delta. Freezing partition time is not required for -isolation. - -Hyper-V does not permit an intercept for its implemented `TSC` MSR. Restore -must therefore write the captured `TSC` value. KVM also restores `TSC` so all -backends rewind guest time with the rest of the snapshot state. - -## Feature exposure - -On MSHV and WHP a guest reaches an MSR only when the hypervisor exposes that -CPU feature to the partition. This gives three cases: - -* Not exposed. Features the partition does not enable, such as the - performance-monitoring unit, last-branch records, and FRED. Hyper-V may still - model the register, but a guest access faults and stores no state until the - feature is exposed. -* Exposed by default. Features the host CPU supports, such as TSC deadline, - UMWAIT, TSX control, CET, `MPERF`/`APERF`, XFD, AMX, and MPX. Their MSRs - must be in the reset set. -* Reset through another state class. MSHV can expose ENQCMD and PASID on - capable Intel hosts. PASID is cleared by XSAVE reset, so it is not duplicated - in the MSR reset set. - -MSHV and WHP enable partition features differently. MSHV creates the partition -without an explicit feature mask, so it enables every processor feature the host -supports. WHP starts from the host-supported set with speculation control off. -MSHV exposes the broader surface and determines which registers the reset set -must cover. - -Perfmon is not part of either default. The performance-monitoring unit and the -last-branch registers are a separate opt-in partition property, off by default -on both backends. Hyperlight never enables it, so those registers stay -unreachable regardless of the enable-everything processor-feature default. - -Only reachable, retained MSRs need coverage, and retained state is always held -in a host-readable and writable register. The mapped registers therefore bound -the reset set: coverage is complete when every mapped register is in the reset -table and reset. - -## Host-addressable but not guest-writable - -A host register mapping does not imply the guest can write the MSR. -`IA32_MISC_ENABLE` (`0x1A0`) is the notable case. Hyper-V emulates it, discards -a guest write, and returns a fixed value to the guest regardless of what was -written. A guest cannot change it to any value, so it retains no guest state -and needs no reset. On AMD the guest access faults. - -## Failed access reporting - -A KVM-denied or Hyper-V-unsupported MSR access does not persist and poisons the -sandbox. The error type and its detail differ by backend. - -* KVM traps the access at the deny filter. Hyperlight reports - `MsrReadViolation` or `MsrWriteViolation`, naming the MSR index and, for a - write, the value. The report is host-verified. -* MSHV and WHP have no host MSR trap. An unsupported access faults inside the - guest as a general protection fault from Hyper-V, so Hyperlight reports - `GuestAborted`. The message records the fault and the faulting instruction but - does not identify the MSR. An exposed MSR can succeed even when absent from - the allow list. Its retained state must be in the reset table. - -Future work: the guest exception handler could decode a faulting `RDMSR` or -`WRMSR` and report the index, promoting the abort to a typed MSR violation on -MSHV and WHP. That index would be guest-reported and therefore advisory. It is -not implemented. - -## Limitations - -KVM's security boundary is structural because its deny filter bounds guest -writes. MSHV and WHP depend on the reset table and exposed processor -features. - -The filterless backend tests run on one CPU model per runner. Model-specific -state absent on that CPU is not exercised. A backend that exposes a new -retained MSR feature needs a matching table entry before Hyperlight can use it -safely. - -The ignored full-window audit probes fixed index ranges with a small set of -values. It cannot prove that every vendor MSR or accepted value is covered. +Hyper-V stores `TSC` and `TSC_ADJUST` independently. While time runs, it +preserves `TSC - TSC_ADJUST`: writing `TSC` also changes `TSC_ADJUST`, while +writing `TSC_ADJUST` changes the internal TSC offset. + +Restore writes `TSC` before `TSC_ADJUST`. The two writes remove the guest's +delta without freezing partition time. KVM also restores `TSC` so all backends +use the same guest-time semantics. + +## Retained-state inventory + +These MSRs are reset when supported by the selected backend and host. + +| MSR (index) | Retained state | +| --- | --- | +| SYSENTER CS, ESP, EIP (`0x174`-`0x176`) | System-call entry state. | +| STAR, LSTAR, CSTAR, SFMASK (`0xC000_0081`-`0xC000_0084`) | System-call target state. | +| KERNEL_GS_BASE (`0xC000_0102`) | Kernel GS base, including `SWAPGS` changes. | +| PAT (`0x277`) | Page attribute state. | +| DEBUGCTL (`0x1D9`) | Debug control state. | +| SPEC_CTRL (`0x48`), VIRT_SPEC_CTRL (`0xC001_011F`) | Speculation control state. | +| CET (`0x6A0`, `0x6A2`, `0x6A4`-`0x6A8`) | CET control and shadow-stack state. | +| XSS (`0xDA0`) | Extended supervisor state mask. | +| TSC, TSC_ADJUST, TSC_AUX (`0x10`, `0x3B`, `0xC000_0103`) | Guest clock state. | +| MTRRs (`0x2FF`, `0x200`-`0x21F`, `0x250`, `0x258`-`0x259`, `0x268`-`0x26F`) | Memory-type state. | +| TSX_CTRL (`0x122`) | TSX control state. | +| XFD, XFD_ERR (`0x1C4`, `0x1C5`) | Extended-feature disable state. | +| UMWAIT_CONTROL (`0xE1`) | WAITPKG control state. | +| TSC_DEADLINE (`0x6E0`) | Deadline-timer state. | +| BNDCFGS (`0xD90`) | MPX bounds configuration. | +| MPERF, APERF (`0xE7`, `0xE8`) | Per-VP performance counters. | + +These classes do not need MSR reset entries. + +| MSR (index) | Reason | +| --- | --- | +| PRED_CMD (`0x49`) | Write-only command. Issues a prediction barrier. | +| FLUSH_CMD (`0x10B`) | Write-only command. Flushes caches. | +| MISC_ENABLE (`0x1A0`) | Hyper-V discards writes. AMD faults the access. | +| FRED (`0x1CC`-`0x1D4`) | Hyperlight exposes no FRED feature. | +| PMU (`0xC1`, `0x186`, `0x38D`, `0x38F`) | Hyperlight leaves perfmon disabled. | +| LBR (`0x1C8`, `0x1C9`, `0x14CE`, `0x14CF`) | Hyperlight leaves perfmon disabled. | + +## Testing + +Focused tests cover: + +* Guest-written MSR values across snapshot, restore, and clone lifecycles. +* Source and destination allow-list compatibility. +* Backend reset-set discovery and snapshot index validation. +* `SWAPGS`, TSC, MTRR, and feature-gated Hyper-V state. +* KVM nested-virtualization, x2APIC, and custom-MSR denial. + +The ignored full-window audit probes additional Hyper-V MSR ranges on the CI +CPU. It is a regression tool, not a complete inventory of vendor MSRs. + +## Future work + +* Verify host write support for every resolved filterless reset entry during VM + creation. Allowed entries already receive a read and write check. +* Exercise MSHV and WHP on more CPU models. Their reachable MSR surfaces depend + on host features. +* Extend the inventory when Hyperlight enables new CPU features such as + perfmon, FRED, or nested virtualization. \ No newline at end of file diff --git a/docs/snapshot-versioning.md b/docs/snapshot-versioning.md index a5ff5e305..f44f44267 100644 --- a/docs/snapshot-versioning.md +++ b/docs/snapshot-versioning.md @@ -56,12 +56,15 @@ made before it was added remain loadable. At the next hard break, make the field required, remove `serde(default)`, and reject zero as an invalid entry point rather than treating it as unknown. -### Optional MSR fields +### Missing MSR state -The persisted `msrs` and `allowed_msrs` fields are optional so snapshots made -before MSR capture remain loadable. At the next hard break, make them required -vectors and remove `serde(default)` and the missing-field fallback. Keep the -in-memory fields optional while `Snapshot` represents pre-init state. +Configs written before MSR capture may omit the `msr_state` object. The loader +represents an omitted object as empty values and an empty allow list, which +restores the destination baseline. A present object requires both `msrs` and +`allowed_msrs`. + +At the next hard break, make `msr_state` required and remove its +`serde(default)` missing-field fallback. ## Enforcement diff --git a/src/hyperlight_host/src/error.rs b/src/hyperlight_host/src/error.rs index 0b982eed6..c6738374d 100644 --- a/src/hyperlight_host/src/error.rs +++ b/src/hyperlight_host/src/error.rs @@ -154,16 +154,6 @@ pub enum HyperlightError { #[error("Memory Access Violation at address {0:#x} of type {1}, but memory is marked as {2}")] MemoryAccessViolation(u64, MemoryRegionFlags, MemoryRegionFlags), - /// A denied guest MSR read. - #[cfg(all(target_arch = "x86_64", kvm))] - #[error("Guest read from denied MSR {0:#x}")] - MsrReadViolation(u32), - - /// A denied guest MSR write. - #[cfg(all(target_arch = "x86_64", kvm))] - #[error("Guest write of {1:#x} to denied MSR {0:#x}")] - MsrWriteViolation(u32, u64), - /// Memory Allocation Failed. #[error("Memory Allocation Failed with OS Error {0:?}.")] MemoryAllocationFailed(Option), @@ -362,10 +352,6 @@ impl HyperlightError { // as poisoning here too for defense in depth. | HyperlightError::HyperlightVmError(HyperlightVmError::Restore(_)) => true, - #[cfg(all(target_arch = "x86_64", kvm))] - HyperlightError::MsrReadViolation(_) - | HyperlightError::MsrWriteViolation(_, _) => true, - // These errors poison the sandbox because they can leave // it in an inconsistent state due to snapshot restore // failing partway through diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs index 7f8ed28aa..d5411d80e 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs @@ -163,16 +163,6 @@ impl DispatchGuestCallError { region_flags, }) => HyperlightError::MemoryAccessViolation(addr, access_type, region_flags), - #[cfg(all(target_arch = "x86_64", kvm))] - DispatchGuestCallError::Run(RunVmError::MsrReadViolation(msr_index)) => { - HyperlightError::MsrReadViolation(msr_index) - } - - #[cfg(all(target_arch = "x86_64", kvm))] - DispatchGuestCallError::Run(RunVmError::MsrWriteViolation { msr_index, value }) => { - HyperlightError::MsrWriteViolation(msr_index, value) - } - // Leave others as is other => HyperlightVmError::DispatchGuestCall(other).into(), }; @@ -223,12 +213,6 @@ pub enum RunVmError { MmioReadUnmapped(u64), #[error("MMIO WRITE access to unmapped address {0:#x}")] MmioWriteUnmapped(u64), - #[cfg(all(target_arch = "x86_64", kvm))] - #[error("Guest read from denied MSR {0:#x}")] - MsrReadViolation(u32), - #[cfg(all(target_arch = "x86_64", kvm))] - #[error("Guest write of {value:#x} to denied MSR {msr_index:#x}")] - MsrWriteViolation { msr_index: u32, value: u64 }, #[error("vCPU run failed: {0}")] RunVcpu(#[from] RunVcpuError), #[error("Unexpected VM exit: {0}")] @@ -759,14 +743,6 @@ impl HyperlightVm { } } } - #[cfg(all(target_arch = "x86_64", kvm))] - Ok(VmExit::MsrRead(msr_index)) => { - break Err(RunVmError::MsrReadViolation(msr_index)); - } - #[cfg(all(target_arch = "x86_64", kvm))] - Ok(VmExit::MsrWrite { msr_index, value }) => { - break Err(RunVmError::MsrWriteViolation { msr_index, value }); - } Ok(VmExit::Cancelled()) => { // If cancellation was not requested for this specific guest function call, // the vcpu was interrupted by a stale cancellation. This can occur when: diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs index 47b92125c..960c2fbf5 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -42,13 +42,8 @@ use crate::hypervisor::gdb::{ use crate::hypervisor::gdb::{DebugError, DebugMemoryAccessError}; use crate::hypervisor::regs::{ CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, MsrEntry, MsrResetState, - core_reset_indices, is_mtrr_reset_index, }; #[cfg(kvm)] -use crate::hypervisor::regs::{MSR_KERNEL_GS_BASE, MSR_TSC}; -#[cfg(any(not(gdb), mshv3, target_os = "windows"))] -use crate::hypervisor::virtual_machine::VirtualMachine; -#[cfg(kvm)] use crate::hypervisor::virtual_machine::kvm::KvmVm; #[cfg(mshv3)] use crate::hypervisor::virtual_machine::mshv::MshvVm; @@ -77,21 +72,6 @@ type BoxedVm = Box; #[cfg(not(gdb))] type BoxedVm = Box; -/// Determines the MTRR reset indices for an MSHV or WHP backend and validates -/// its allow list. Both hosts lack an MSR filter, so the allow list adds reset -/// state. -#[cfg(any(mshv3, target_os = "windows"))] -fn determine_reset_msrs( - vm: &dyn VirtualMachine, - allowed: &[u32], -) -> std::result::Result, VmError> { - use crate::hypervisor::virtual_machine::{mtrr_reset_indices, validate_allowed_msrs}; - - let required_mtrrs = mtrr_reset_indices(vm).map_err(VmError::CreateVm)?; - validate_allowed_msrs(vm, allowed).map_err(VmError::CreateVm)?; - Ok(required_mtrrs) -} - impl HyperlightVm { /// Create a new HyperlightVm instance (will not run vm until calling `initialise`) #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] @@ -108,27 +88,19 @@ impl HyperlightVm { #[cfg(crashdump)] rt_cfg: SandboxRuntimeConfig, #[cfg(feature = "mem_profile")] trace_info: MemTraceInfo, ) -> std::result::Result { - let (vm, required_mtrrs): (BoxedVm, Vec) = match get_available_hypervisor() { + let vm: BoxedVm = match get_available_hypervisor() { #[cfg(kvm)] Some(HypervisorType::Kvm) => { let kvm_vm = KvmVm::new().map_err(VmError::CreateVm)?; kvm_vm .configure_msr_access(config.get_allowed_msrs()) .map_err(VmError::CreateVm)?; - (Box::new(kvm_vm), Vec::new()) + Box::new(kvm_vm) } #[cfg(mshv3)] - Some(HypervisorType::Mshv) => { - let vm: BoxedVm = Box::new(MshvVm::new().map_err(VmError::CreateVm)?); - let required_mtrrs = determine_reset_msrs(vm.as_ref(), config.get_allowed_msrs())?; - (vm, required_mtrrs) - } + Some(HypervisorType::Mshv) => Box::new(MshvVm::new().map_err(VmError::CreateVm)?), #[cfg(target_os = "windows")] - Some(HypervisorType::Whp) => { - let vm: BoxedVm = Box::new(WhpVm::new().map_err(VmError::CreateVm)?); - let required_mtrrs = determine_reset_msrs(vm.as_ref(), config.get_allowed_msrs())?; - (vm, required_mtrrs) - } + Some(HypervisorType::Whp) => Box::new(WhpVm::new().map_err(VmError::CreateVm)?), None => return Err(CreateHyperlightVmError::NoHypervisorFound), }; @@ -168,9 +140,14 @@ impl HyperlightVm { }), }); - // The MSRs reset on restore and captured in snapshots. + let reset_indices = vm + .msr_reset_indices(config.get_allowed_msrs()) + .map_err(VmError::CreateVm)?; let msr_reset = - Self::capture_msr_reset_state(&vm, required_mtrrs, config.get_allowed_msrs())?; + MsrResetState::capture(reset_indices, config.get_allowed_msrs(), |indices| { + vm.msrs(indices) + }) + .map_err(VmError::Register)?; let snapshot_slot = 0u32; let scratch_slot = 1u32; @@ -236,50 +213,6 @@ impl HyperlightVm { Ok(ret) } - /// Determines this VM's MSR reset set: the MSRs reset on restore and - /// captured in snapshots. `required_mtrrs` is the VCNT-driven MTRR set - /// gathered at creation. `allowed` is the validated allow list. - fn capture_msr_reset_state( - vm: &BoxedVm, - required_mtrrs: Vec, - allowed: &[u32], - ) -> std::result::Result { - let core: Vec = match get_available_hypervisor() { - #[cfg(kvm)] - Some(HypervisorType::Kvm) => vec![MSR_KERNEL_GS_BASE, MSR_TSC], // GS_BASE is needed for correctness. TSC is to match mshv/whp behavior - #[cfg(mshv3)] - Some(HypervisorType::Mshv) => Self::probe_core_reset_indices(vm), - #[cfg(target_os = "windows")] - Some(HypervisorType::Whp) => Self::probe_core_reset_indices(vm), - // The VM already exists, so a hypervisor was found. The `None` - // arm in `new` returned before this point. - None => unreachable!(), - }; - let mut indices: Vec = core - .into_iter() - .chain(required_mtrrs) - .chain(allowed.iter().copied()) - .collect(); - indices.sort_unstable(); - indices.dedup(); - let baseline = vm.msrs(&indices).map_err(VmError::Register)?; - let mut allowed = allowed.to_vec(); - allowed.sort_unstable(); - allowed.dedup(); - Ok(MsrResetState::new(baseline, allowed)) - } - - /// Core stateful MSRs a filterless host can read. - /// - /// MTRRs come from the VCNT-driven required set, so they are excluded. - #[cfg(any(mshv3, target_os = "windows"))] - fn probe_core_reset_indices(vm: &BoxedVm) -> Vec { - core_reset_indices() - .filter(|i| !is_mtrr_reset_index(*i)) - .filter(|i| vm.msrs(&[*i]).is_ok()) - .collect() - } - /// Initialise the internally stored vCPU with the given PEB address and /// random number seed, then run it until a HLT instruction. #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] @@ -490,6 +423,12 @@ impl HyperlightVm { cr3: u64, sregs: &CommonSpecialRegisters, ) -> std::result::Result<(), RegisterError> { + if sregs.apic_base & crate::hypervisor::regs::APIC_BASE_X2APIC_ENABLE != 0 { + return Err(RegisterError::InvalidSnapshotApicBase { + value: sregs.apic_base, + }); + } + // Restore the full special registers from snapshot, but update CR3 // to point to the new (relocated) page tables let mut sregs = *sregs; diff --git a/src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs b/src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs index 82782a180..d930a3691 100644 --- a/src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs +++ b/src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs @@ -46,10 +46,22 @@ pub(crate) struct MsrResetState { } impl MsrResetState { - /// Builds the reset state from the creation-time baseline entries and the - /// requested allow list. - pub fn new(baseline: Vec, allowed: Vec) -> Self { - Self { baseline, allowed } + /// Captures and normalizes the reset state for a backend-provided index set. + pub fn capture( + mut indices: Vec, + allowed: &[u32], + read: impl FnOnce( + &[u32], + ) + -> Result, crate::hypervisor::virtual_machine::RegisterError>, + ) -> Result { + indices.sort_unstable(); + indices.dedup(); + let baseline = read(&indices)?; + let mut allowed = allowed.to_vec(); + allowed.sort_unstable(); + allowed.dedup(); + Ok(Self { baseline, allowed }) } /// The creation-time baseline entries. @@ -152,7 +164,7 @@ const HYPERV_VARIABLE_MTRR_COUNT: u8 = 16; // PRED_CMD (0x49) and FLUSH_CMD (0x10B) are intentionally absent: they are // write-only commands with no retained state, so they cannot be reset and // therefore cannot be allowed. -const MSR_TABLE: &[u32] = &[ +const NON_MTRR_RESETTABLE_MSRS: &[u32] = &[ // Guest and host access use the matching SYSENTER state. MSR_SYSENTER_CS, MSR_SYSENTER_ESP, @@ -176,7 +188,6 @@ const MSR_TABLE: &[u32] = &[ // Host probing omits CET state when CET is unavailable. MSR_U_CET, MSR_S_CET, - // Host probing omits shadow-stack state when it is unavailable. MSR_PL0_SSP, MSR_PL1_SSP, MSR_PL2_SSP, @@ -203,6 +214,9 @@ const MSR_TABLE: &[u32] = &[ MSR_UMWAIT_CONTROL, MSR_TSC_DEADLINE, MSR_BNDCFGS, +]; + +const MTRR_RESET_INDICES: &[u32] = &[ // Hyper-V accepts fixed-MTRR writes even when MTRRCAP.FIX is clear. MSR_MTRR_DEF_TYPE, 0x200, // PHYSBASE0 @@ -252,12 +266,20 @@ const MSR_TABLE: &[u32] = &[ /// Whether an MSR carries retained state eligible for the reset set. pub(crate) fn is_resettable_msr(index: u32) -> bool { - MSR_TABLE.contains(&index) + NON_MTRR_RESETTABLE_MSRS.contains(&index) || is_mtrr_reset_index(index) } -/// Returns core stateful indices for host filtering. -pub(crate) fn core_reset_indices() -> impl Iterator { - MSR_TABLE.iter().copied() +/// Returns non-MTRR candidates probed by filterless Hyper-V backends. +pub(crate) fn filterless_core_reset_candidates() -> impl Iterator { + NON_MTRR_RESETTABLE_MSRS.iter().copied() +} + +#[cfg(test)] +pub(crate) fn resettable_msr_indices() -> impl Iterator { + NON_MTRR_RESETTABLE_MSRS + .iter() + .chain(MTRR_RESET_INDICES) + .copied() } pub(crate) fn hyperv_mtrr_reset_indices( @@ -293,22 +315,7 @@ pub(crate) fn hyperv_mtrr_reset_indices( } pub(crate) fn is_mtrr_reset_index(index: u32) -> bool { - index == MSR_MTRR_DEF_TYPE - || (0x200..=0x21F).contains(&index) - || matches!( - index, - MSR_MTRR_FIX64K_00000 - | 0x258 - | 0x259 - | 0x268 - | 0x269 - | 0x26A - | 0x26B - | 0x26C - | 0x26D - | 0x26E - | 0x26F - ) + MTRR_RESET_INDICES.contains(&index) } #[cfg(test)] diff --git a/src/hyperlight_host/src/hypervisor/regs/x86_64/special_regs.rs b/src/hyperlight_host/src/hypervisor/regs/x86_64/special_regs.rs index adecfb24a..3786b41da 100644 --- a/src/hyperlight_host/src/hypervisor/regs/x86_64/special_regs.rs +++ b/src/hyperlight_host/src/hypervisor/regs/x86_64/special_regs.rs @@ -33,6 +33,7 @@ const CR0_PE: u64 = 1; const CR0_ET: u64 = 1 << 4; const CR0_WP: u64 = 1 << 16; const CR0_PG: u64 = 1 << 31; +pub(crate) const APIC_BASE_X2APIC_ENABLE: u64 = 1 << 10; mod amd64_consts { pub(crate) const CR4_PAE: u64 = 1 << 5; diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs index 87f9f111e..96238f806 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs @@ -14,20 +14,18 @@ See the License for the specific language governing permissions and limitations under the License. */ -use std::collections::HashSet; use std::sync::LazyLock; use hyperlight_common::outb::VmAction; #[cfg(gdb)] use kvm_bindings::kvm_guest_debug; use kvm_bindings::{ - Msrs, kvm_debugregs, kvm_enable_cap, kvm_fpu, kvm_msr_entry, kvm_regs, kvm_sregs, - kvm_userspace_memory_region, kvm_xsave, + Msrs, kvm_debugregs, kvm_fpu, kvm_msr_entry, kvm_regs, kvm_sregs, kvm_userspace_memory_region, + kvm_xsave, }; use kvm_ioctls::Cap::UserMemory; use kvm_ioctls::{ - Cap, Kvm, MsrExitReason, MsrFilterDefaultAction, MsrFilterRange, MsrFilterRangeFlags, VcpuExit, - VcpuFd, VmFd, + Cap, Kvm, MsrFilterDefaultAction, MsrFilterRange, MsrFilterRangeFlags, VcpuExit, VcpuFd, VmFd, }; use tracing::{Span, instrument}; #[cfg(feature = "trace_guest")] @@ -39,7 +37,7 @@ use vmm_sys_util::eventfd::EventFd; use crate::hypervisor::gdb::{DebugError, DebuggableVm}; use crate::hypervisor::regs::{ CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, FP_CONTROL_WORD_DEFAULT, - MXCSR_DEFAULT, MsrEntry, is_resettable_msr, + MSR_KERNEL_GS_BASE, MSR_TSC, MXCSR_DEFAULT, MsrEntry, }; #[cfg(test)] use crate::hypervisor::virtual_machine::XSAVE_BUFFER_SIZE; @@ -47,7 +45,7 @@ use crate::hypervisor::virtual_machine::XSAVE_BUFFER_SIZE; use crate::hypervisor::virtual_machine::x86_64::hw_interrupts::TimerThread; use crate::hypervisor::virtual_machine::{ CreateVmError, MapMemoryError, RegisterError, RunVcpuError, UnmapMemoryError, VirtualMachine, - VmExit, + VmExit, validate_allowed_msrs, }; use crate::mem::memory_region::MemoryRegion; #[cfg(feature = "trace_guest")] @@ -70,6 +68,10 @@ use crate::sandbox::trace::TraceContext as SandboxTraceContext; /// Extended Feature Identification, pp. 627--628 const CPUID_FUNCTION_PROCESSOR_CAPACITY_PARAMETERS_AND_EXTENDED_FEATURE_IDENTIFICATION: u32 = 0x8000_0008; +const CPUID_FUNCTION_FEATURE_INFORMATION: u32 = 0x1; +const CPUID_FUNCTION_EXTENDED_FEATURE_INFORMATION: u32 = 0x8000_0001; +const CPUID_FEATURE_VMX: u32 = 1 << 5; +const CPUID_FEATURE_SVM: u32 = 1 << 2; /// Return `true` if the KVM API is available, version 12, and has UserMemory capability, or `false` otherwise #[instrument(skip_all, parent = Span::current(), level = "Trace")] @@ -115,24 +117,6 @@ pub(crate) struct KvmVm { static KVM: LazyLock> = LazyLock::new(|| Kvm::new().map_err(|e| CreateVmError::HypervisorNotAvailable(e.into()))); -/// Cached host indices reported by `KVM_GET_MSR_INDEX_LIST`. -/// An empty set makes support checks fail closed. -static HOST_MSR_INDICES: LazyLock> = LazyLock::new(|| match KVM.as_ref() { - Ok(kvm) => match kvm.get_msr_index_list() { - Ok(list) => list.as_slice().iter().copied().collect(), - Err(e) => { - tracing::warn!("KVM_GET_MSR_INDEX_LIST failed: {e}"); - HashSet::new() - } - }, - Err(_) => HashSet::new(), -}); - -/// Returns the set of MSR indices the host KVM supports for get/set. -pub(crate) fn host_msr_indices() -> &'static HashSet { - &HOST_MSR_INDICES -} - /// KVM allows at most this many MSR filter ranges. const KVM_MSR_FILTER_MAX_RANGES: usize = 16; @@ -204,19 +188,26 @@ impl KvmVm { .create_vcpu(0) .map_err(|e| CreateVmError::CreateVcpuFd(e.into()))?; - // Set the CPUID leaf for MaxPhysAddr. KVM allows this to - // easily be overridden by the hypervisor and defaults it very - // low, while mshv passes it through from hardware unless an - // intercept is installed. + // Configure the guest CPUID for Hyperlight's supported CPU model. let mut kvm_cpuid = hv .get_supported_cpuid(kvm_bindings::KVM_MAX_CPUID_ENTRIES) .map_err(|e| CreateVmError::InitializeVm(e.into()))?; for entry in kvm_cpuid.as_mut_slice().iter_mut() { - if entry.function - == CPUID_FUNCTION_PROCESSOR_CAPACITY_PARAMETERS_AND_EXTENDED_FEATURE_IDENTIFICATION - { - entry.eax &= !0xff; - entry.eax |= hyperlight_common::layout::SCRATCH_TOP_GPA.ilog2() + 1; + match entry.function { + // Hyperlight does not support nested Intel virtualization. + CPUID_FUNCTION_FEATURE_INFORMATION => entry.ecx &= !CPUID_FEATURE_VMX, + // Hyperlight does not support nested AMD virtualization. + CPUID_FUNCTION_EXTENDED_FEATURE_INFORMATION => { + entry.ecx &= !CPUID_FEATURE_SVM; + } + // KVM allows MaxPhysAddr to be overridden and defaults it too low for + // Hyperlight's memory layout. MSHV passes it through from hardware + // unless an intercept is installed. + CPUID_FUNCTION_PROCESSOR_CAPACITY_PARAMETERS_AND_EXTENDED_FEATURE_IDENTIFICATION => { + entry.eax &= !0xff; + entry.eax |= hyperlight_common::layout::SCRATCH_TOP_GPA.ilog2() + 1; + } + _ => {} } } vcpu_fd @@ -269,20 +260,6 @@ impl KvmVm { } Ok(VcpuExit::MmioRead(addr, _)) => return Ok(VmExit::MmioRead(addr)), Ok(VcpuExit::MmioWrite(addr, _)) => return Ok(VmExit::MmioWrite(addr)), - // Complete filtered access through the default run path. - Ok(VcpuExit::X86Rdmsr(msr_exit)) => { - let msr_index = msr_exit.index; - *msr_exit.error = 1; - self.complete_filtered_msr_exit()?; - return Ok(VmExit::MsrRead(msr_index)); - } - Ok(VcpuExit::X86Wrmsr(msr_exit)) => { - let msr_index = msr_exit.index; - let value = msr_exit.data; - *msr_exit.error = 1; - self.complete_filtered_msr_exit()?; - return Ok(VmExit::MsrWrite { msr_index, value }); - } #[cfg(gdb)] Ok(VcpuExit::Debug(debug_exit)) => { return Ok(VmExit::Debug { @@ -305,23 +282,6 @@ impl KvmVm { } } - /// Finish a filtered MSR access after its `error` flag was set. - /// Reentering `KVM_RUN` injects `#GP` for the pending access. - /// immediate_exit halts the guest right after, so it never runs past - /// the fault. That reentry returns `EINTR`, which is expected here. - fn complete_filtered_msr_exit(&mut self) -> std::result::Result<(), RunVcpuError> { - self.vcpu_fd.set_kvm_immediate_exit(1); - // `.err()` drops the `Ok(VcpuExit)`, which borrows `vcpu_fd`, keeping - // only the owned error so the borrow ends before the next call. - let err = self.vcpu_fd.run().err(); - self.vcpu_fd.set_kvm_immediate_exit(0); - match err { - None => Ok(()), - Some(e) if e.errno() == libc::EINTR => Ok(()), - Some(e) => Err(RunVcpuError::Unknown(e.into())), - } - } - #[cfg(feature = "hw-interrupts")] fn handle_pv_timer_config(&mut self, data: &[u8]) { use super::super::x86_64::hw_interrupts::handle_pv_timer_config; @@ -347,21 +307,6 @@ impl KvmVm { Ok(VcpuExit::IoOut(port, data)) => Ok(VmExit::IoOut(port, data.to_vec())), Ok(VcpuExit::MmioRead(addr, _)) => Ok(VmExit::MmioRead(addr)), Ok(VcpuExit::MmioWrite(addr, _)) => Ok(VmExit::MmioWrite(addr)), - // Reentering KVM_RUN completes the failed MSR exit and injects #GP. - // immediate_exit prevents further guest execution. - Ok(VcpuExit::X86Rdmsr(msr_exit)) => { - let msr_index = msr_exit.index; - *msr_exit.error = 1; - self.complete_filtered_msr_exit()?; - Ok(VmExit::MsrRead(msr_index)) - } - Ok(VcpuExit::X86Wrmsr(msr_exit)) => { - let msr_index = msr_exit.index; - let value = msr_exit.data; - *msr_exit.error = 1; - self.complete_filtered_msr_exit()?; - Ok(VmExit::MsrWrite { msr_index, value }) - } #[cfg(gdb)] Ok(VcpuExit::Debug(debug_exit)) => Ok(VmExit::Debug { dr6: debug_exit.dr6, @@ -381,33 +326,18 @@ impl KvmVm { } /// Installs a deny filter containing the validated allow list. - /// Requires `KVM_CAP_X86_USER_SPACE_MSR` and `KVM_CAP_X86_MSR_FILTER`. + /// Requires `KVM_CAP_X86_MSR_FILTER`. pub(crate) fn configure_msr_access( &self, allowed: &[u32], ) -> std::result::Result<(), CreateVmError> { let hv = KVM.as_ref().map_err(|e| e.clone())?; - if !hv.check_extension(Cap::X86UserSpaceMsr) || !hv.check_extension(Cap::X86MsrFilter) { - tracing::error!( - "KVM does not support KVM_CAP_X86_USER_SPACE_MSR or KVM_CAP_X86_MSR_FILTER." - ); + if !hv.check_extension(Cap::X86MsrFilter) { + tracing::error!("KVM does not support KVM_CAP_X86_MSR_FILTER."); return Err(CreateVmError::MsrFilterNotSupported); } - // Every permitted guest write must have restorable host state. - for &msr in allowed { - self.validate_allowed_msr(msr)?; - } - - // Tell KVM to exit to userspace on filtered MSR access. - let cap = kvm_enable_cap { - cap: Cap::X86UserSpaceMsr as u32, - args: [MsrExitReason::Filter.bits() as u64, 0, 0, 0], - ..Default::default() - }; - self.vm_fd - .enable_cap(&cap) - .map_err(|e| CreateVmError::InitializeVm(e.into()))?; + validate_allowed_msrs(self, allowed)?; // Each contiguous group consumes one KVM filter range. let groups = coalesce_msr_ranges(allowed); @@ -454,75 +384,6 @@ impl KvmVm { .map_err(|e| CreateVmError::InitializeVm(e.into()))?; Ok(()) } - - /// Validates that an allowed MSR has restorable host state. - fn validate_allowed_msr(&self, msr: u32) -> std::result::Result<(), CreateVmError> { - if !is_resettable_msr(msr) { - return Err(CreateVmError::MsrNotAllowable { - msr, - reason: "MSR is not a resettable MSR".to_string(), - }); - } - if !host_msr_indices().contains(&msr) { - return Err(CreateVmError::MsrNotAllowable { - msr, - reason: "MSR is not supported by the host".to_string(), - }); - } - let value = self - .read_msr(msr) - .map_err(|e| CreateVmError::MsrNotAllowable { - msr, - reason: format!("MSR is not readable: {e}"), - })?; - self.write_msr(msr, value) - .map_err(|e| CreateVmError::MsrNotAllowable { - msr, - reason: format!("MSR is not resettable: {e}"), - })?; - Ok(()) - } - - /// Reads one vCPU MSR without the guest filter. - fn read_msr(&self, index: u32) -> std::result::Result { - let mut msrs = Msrs::from_entries(&[kvm_msr_entry { - index, - ..Default::default() - }]) - .map_err(|e| RegisterError::MsrBuild(format!("{e:?}")))?; - let n = self - .vcpu_fd - .get_msrs(&mut msrs) - .map_err(|e| RegisterError::GetMsrs(e.into()))?; - if n != 1 { - return Err(RegisterError::MsrShortCount { - expected: 1, - actual: n, - }); - } - Ok(msrs.as_slice()[0].data) - } - - /// Writes one vCPU MSR without the guest filter. - fn write_msr(&self, index: u32, data: u64) -> std::result::Result<(), RegisterError> { - let msrs = Msrs::from_entries(&[kvm_msr_entry { - index, - data, - ..Default::default() - }]) - .map_err(|e| RegisterError::MsrBuild(format!("{e:?}")))?; - let n = self - .vcpu_fd - .set_msrs(&msrs) - .map_err(|e| RegisterError::SetMsrs(e.into()))?; - if n != 1 { - return Err(RegisterError::MsrShortCount { - expected: 1, - actual: n, - }); - } - Ok(()) - } } impl VirtualMachine for KvmVm { @@ -657,12 +518,12 @@ impl VirtualMachine for KvmVm { actual: n, }); } - Ok(msrs - .as_slice() + Ok(indices .iter() - .map(|e| MsrEntry { - index: e.index, - value: e.data, + .zip(msrs.as_slice()) + .map(|(&index, entry)| MsrEntry { + index, + value: entry.data, }) .collect()) } @@ -694,6 +555,13 @@ impl VirtualMachine for KvmVm { Ok(()) } + fn msr_reset_indices(&self, allowed: &[u32]) -> std::result::Result, CreateVmError> { + Ok([MSR_KERNEL_GS_BASE, MSR_TSC] + .into_iter() + .chain(allowed.iter().copied()) + .collect()) + } + #[allow(dead_code)] fn xsave(&self) -> std::result::Result, RegisterError> { let xsave = self diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs index 4cd8fca02..dc820d699 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs @@ -21,13 +21,15 @@ use tracing::{Span, instrument}; #[cfg(gdb)] use crate::hypervisor::gdb::DebugError; -#[cfg(target_arch = "x86_64")] -use crate::hypervisor::regs::MsrEntry; use crate::hypervisor::regs::{ CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, }; #[cfg(all(target_arch = "x86_64", any(mshv3, target_os = "windows")))] -use crate::hypervisor::regs::{MSR_MTRR_CAP, hyperv_mtrr_reset_indices, is_resettable_msr}; +use crate::hypervisor::regs::{ + MSR_MTRR_CAP, filterless_core_reset_candidates, hyperv_mtrr_reset_indices, +}; +#[cfg(target_arch = "x86_64")] +use crate::hypervisor::regs::{MsrEntry, is_resettable_msr}; use crate::mem::memory_region::MemoryRegion; #[cfg(feature = "trace_guest")] use crate::sandbox::trace::TraceContext as SandboxTraceContext; @@ -143,12 +145,6 @@ pub(crate) enum VmExit { MmioRead(u64), /// The vCPU tried to write to the given (unmapped) addr MmioWrite(u64), - /// The vCPU tried to read from the given MSR - #[cfg(all(target_arch = "x86_64", kvm))] - MsrRead(u32), - /// The vCPU tried to write to the given MSR with the given value - #[cfg(all(target_arch = "x86_64", kvm))] - MsrWrite { msr_index: u32, value: u64 }, /// The vCPU execution has been cancelled Cancelled(), /// The vCPU has exited for a reason that is not handled by Hyperlight @@ -194,7 +190,7 @@ pub enum CreateVmError { #[error("Initialize VM failed: {0}")] InitializeVm(HypervisorError), #[cfg(all(kvm, target_arch = "x86_64"))] - #[error("KVM MSR filtering requires KVM_CAP_X86_USER_SPACE_MSR and KVM_CAP_X86_MSR_FILTER")] + #[error("KVM MSR filtering requires KVM_CAP_X86_MSR_FILTER")] MsrFilterNotSupported, #[cfg(target_arch = "x86_64")] #[error("MSR {msr:#x} cannot be allowed: {reason}")] @@ -258,6 +254,12 @@ pub enum RegisterError { GetSregs(HypervisorError), #[error("Failed to set special registers: {0}")] SetSregs(HypervisorError), + #[cfg(target_arch = "x86_64")] + #[error("Snapshot APIC_BASE {value:#x} enables unsupported x2APIC mode")] + InvalidSnapshotApicBase { + /// APIC_BASE value supplied by the snapshot. + value: u64, + }, #[error("Failed to get debug registers: {0}")] GetDebugRegs(HypervisorError), #[error("Failed to set debug registers: {0}")] @@ -431,6 +433,9 @@ pub(crate) trait VirtualMachine: Debug + Send { /// Writes the supplied MSRs. #[cfg(target_arch = "x86_64")] fn set_msrs(&self, msrs: &[MsrEntry]) -> std::result::Result<(), RegisterError>; + /// Returns the MSRs whose state this backend must reset. + #[cfg(target_arch = "x86_64")] + fn msr_reset_indices(&self, allowed: &[u32]) -> std::result::Result, CreateVmError>; /// Get xsave #[allow(dead_code)] @@ -458,10 +463,10 @@ pub(crate) trait VirtualMachine: Debug + Send { fn partition_handle(&self) -> windows::Win32::System::Hypervisor::WHV_PARTITION_HANDLE; } -/// Validates that each allowed MSR is restorable on a filterless (MSHV/WHP) -/// host: reset replays a captured value, so the host must read and write it. +/// Validates that each allowed MSR is restorable: reset replays a captured +/// value, so the host must read and write it. /// Rejects e.g. a variable MTRR above the host VCNT. -#[cfg(all(target_arch = "x86_64", any(mshv3, target_os = "windows")))] +#[cfg(target_arch = "x86_64")] pub(crate) fn validate_allowed_msrs( vm: &dyn VirtualMachine, allowed: &[u32], @@ -503,6 +508,23 @@ pub(crate) fn mtrr_reset_indices( Ok(indices) } +/// Builds the reset index set required by a filterless Hyper-V backend. +#[cfg(all(target_arch = "x86_64", any(mshv3, target_os = "windows")))] +pub(crate) fn hyperv_msr_reset_indices( + vm: &dyn VirtualMachine, + allowed: &[u32], +) -> std::result::Result, CreateVmError> { + validate_allowed_msrs(vm, allowed)?; + let mut indices: Vec = filterless_core_reset_candidates() + .filter(|index| vm.msrs(&[*index]).is_ok()) + .chain(mtrr_reset_indices(vm)?) + .chain(allowed.iter().copied()) + .collect(); + indices.sort_unstable(); + indices.dedup(); + Ok(indices) +} + #[cfg(test)] mod tests { diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs index 88f2ae1f3..469d9ad5c 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs @@ -136,11 +136,11 @@ fn msr_to_hv_register_name(index: u32) -> Result { #[cfg(test)] mod msr_mapping_tests { use super::*; - use crate::hypervisor::regs::core_reset_indices; + use crate::hypervisor::regs::resettable_msr_indices; #[test] fn maps_all_stateful_msrs() { - for index in core_reset_indices() { + for index in resettable_msr_indices() { assert!( msr_to_hv_register_name(index).is_ok(), "missing MSR mapping for {index:#x}" @@ -183,13 +183,13 @@ impl MshvVm { let mshv = MSHV.as_ref().map_err(|e| e.clone())?; #[allow(unused_mut)] - let mut pr = mshv_create_partition_v2::default(); - // Enable LAPIC for hw-interrupts — required for interrupt delivery - // via request_virtual_interrupt. + let mut pr: mshv_create_partition_v2 = Default::default(); + // The default has no partition flags. Hardware interrupts add only the + // LAPIC required by request_virtual_interrupt. #[cfg(feature = "hw-interrupts")] { use mshv_bindings::MSHV_PT_BIT_LAPIC; - pr.pt_flags |= 1u64 << MSHV_PT_BIT_LAPIC; + pr.pt_flags = 1u64 << MSHV_PT_BIT_LAPIC; } // It's important to use create_vm_with_args() (not create_vm()), // because create_vm() sets up a SynIC partition by default. @@ -562,6 +562,10 @@ impl VirtualMachine for MshvVm { Ok(()) } + fn msr_reset_indices(&self, allowed: &[u32]) -> std::result::Result, CreateVmError> { + crate::hypervisor::virtual_machine::hyperv_msr_reset_indices(self, allowed) + } + #[allow(dead_code)] fn xsave(&self) -> std::result::Result, RegisterError> { let xsave = self @@ -743,7 +747,7 @@ impl DebuggableVm for MshvVm { /// for use with the shared LAPIC helpers. #[cfg(feature = "hw-interrupts")] fn lapic_regs_as_u8(regs: &[::std::os::raw::c_char; 1024]) -> &[u8] { - // Safety: c_char (i8) and u8 have the same size and alignment; + // SAFETY: c_char (i8) and u8 have the same size and alignment; // LAPIC register values are treated as raw bytes. unsafe { &*(regs as *const [::std::os::raw::c_char; 1024] as *const [u8; 1024]) } } @@ -752,7 +756,7 @@ fn lapic_regs_as_u8(regs: &[::std::os::raw::c_char; 1024]) -> &[u8] { /// for use with the shared LAPIC helpers. #[cfg(feature = "hw-interrupts")] fn lapic_regs_as_u8_mut(regs: &mut [::std::os::raw::c_char; 1024]) -> &mut [u8] { - // Safety: same as above. + // SAFETY: same as above. unsafe { &mut *(regs as *mut [::std::os::raw::c_char; 1024] as *mut [u8; 1024]) } } diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs index 7f3ab0656..314b8fc01 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs @@ -164,11 +164,11 @@ fn msr_to_whv_register_name(index: u32) -> Option { #[cfg(test)] mod msr_mapping_tests { use super::*; - use crate::hypervisor::regs::{MSR_DEBUGCTL, MSR_VIRT_SPEC_CTRL, core_reset_indices}; + use crate::hypervisor::regs::{MSR_DEBUGCTL, MSR_VIRT_SPEC_CTRL, resettable_msr_indices}; #[test] fn maps_all_stateful_msrs_except_debugctl_and_virt_spec_ctrl() { - for index in core_reset_indices() { + for index in resettable_msr_indices() { // WHP models neither DEBUGCTL nor VIRT_SPEC_CTRL as a named register. // Both are safe: WHP does not expose their guest-writable features, so // a guest cannot leave retained state in them. @@ -352,6 +352,25 @@ impl WhpVm { }) } + fn get_registers( + &self, + names: &[WHV_REGISTER_NAME], + values: &mut [Align16], + ) -> windows_result::Result<()> { + assert_eq!(names.len(), values.len()); + // SAFETY: The slices have equal lengths, and `values` is aligned output + // storage for each requested register on virtual processor 0. + unsafe { + WHvGetVirtualProcessorRegisters( + self.partition, + 0, + names.as_ptr(), + names.len() as u32, + values.as_mut_ptr() as *mut WHV_REGISTER_VALUE, + ) + } + } + /// Helper for setting arbitrary registers. Makes sure the same number /// of names and values are passed (at the expense of some performance). fn set_registers( @@ -628,16 +647,8 @@ impl VirtualMachine for WhpVm { let names = [WHvX64RegisterDr6]; let mut out: [Align16; 1] = unsafe { std::mem::zeroed() }; - unsafe { - WHvGetVirtualProcessorRegisters( - self.partition, - 0, - names.as_ptr(), - 1, - out.as_mut_ptr() as *mut WHV_REGISTER_VALUE, - ) + self.get_registers(&names, &mut out) .map_err(|e| RunVcpuError::GetDr6(e.into()))?; - } unsafe { out[0].0.Reg64 } }; @@ -683,16 +694,8 @@ impl VirtualMachine for WhpVm { let mut whv_regs_values: [Align16; WHP_REGS_NAMES_LEN] = unsafe { std::mem::zeroed() }; - unsafe { - WHvGetVirtualProcessorRegisters( - self.partition, - 0, - WHP_REGS_NAMES.as_ptr(), - whv_regs_values.len() as u32, - whv_regs_values.as_mut_ptr() as *mut WHV_REGISTER_VALUE, - ) + self.get_registers(&WHP_REGS_NAMES, &mut whv_regs_values) .map_err(|e| RegisterError::GetRegs(e.into()))?; - } WHP_REGS_NAMES .into_iter() @@ -720,16 +723,8 @@ impl VirtualMachine for WhpVm { let mut whp_fpu_values: [Align16; WHP_FPU_NAMES_LEN] = unsafe { std::mem::zeroed() }; - unsafe { - WHvGetVirtualProcessorRegisters( - self.partition, - 0, - WHP_FPU_NAMES.as_ptr(), - whp_fpu_values.len() as u32, - whp_fpu_values.as_mut_ptr() as *mut WHV_REGISTER_VALUE, - ) + self.get_registers(&WHP_FPU_NAMES, &mut whp_fpu_values) .map_err(|e| RegisterError::GetFpu(e.into()))?; - } WHP_FPU_NAMES .into_iter() @@ -757,16 +752,8 @@ impl VirtualMachine for WhpVm { let mut whp_sregs_values: [Align16; WHP_SREGS_NAMES_LEN] = unsafe { std::mem::zeroed() }; - unsafe { - WHvGetVirtualProcessorRegisters( - self.partition, - 0, - WHP_SREGS_NAMES.as_ptr(), - whp_sregs_values.len() as u32, - whp_sregs_values.as_mut_ptr() as *mut WHV_REGISTER_VALUE, - ) + self.get_registers(&WHP_SREGS_NAMES, &mut whp_sregs_values) .map_err(|e| RegisterError::GetSregs(e.into()))?; - } WHP_SREGS_NAMES .into_iter() @@ -824,18 +811,8 @@ impl VirtualMachine for WhpVm { // all-zero value is a valid initial state. let mut values: Vec> = vec![unsafe { std::mem::zeroed() }; names.len()]; - // SAFETY: names and values have equal length. The call fills each value - // slot from the partition's vp 0 for the given register names. - unsafe { - WHvGetVirtualProcessorRegisters( - self.partition, - 0, - names.as_ptr(), - names.len() as u32, - values.as_mut_ptr() as *mut WHV_REGISTER_VALUE, - ) + self.get_registers(&names, &mut values) .map_err(|e| RegisterError::GetMsrs(e.into()))?; - } Ok(indices .iter() .zip(values) @@ -864,20 +841,16 @@ impl VirtualMachine for WhpVm { Ok(()) } + fn msr_reset_indices(&self, allowed: &[u32]) -> std::result::Result, CreateVmError> { + crate::hypervisor::virtual_machine::hyperv_msr_reset_indices(self, allowed) + } + fn debug_regs(&self) -> std::result::Result { let mut whp_debug_regs_values: [Align16; WHP_DEBUG_REGS_NAMES_LEN] = Default::default(); - unsafe { - WHvGetVirtualProcessorRegisters( - self.partition, - 0, - WHP_DEBUG_REGS_NAMES.as_ptr(), - whp_debug_regs_values.len() as u32, - whp_debug_regs_values.as_mut_ptr() as *mut WHV_REGISTER_VALUE, - ) + self.get_registers(&WHP_DEBUG_REGS_NAMES, &mut whp_debug_regs_values) .map_err(|e| RegisterError::GetDebugRegs(e.into()))?; - } let whp_debug_regs: [(WHV_REGISTER_NAME, Align16); WHP_DEBUG_REGS_NAMES_LEN] = diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index f8a3754ab..c29a8f436 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -31,8 +31,6 @@ use super::shared_mem::{ ExclusiveSharedMemory, GuestSharedMemory, HostSharedMemory, ReadonlySharedMemory, SharedMemory, }; use crate::hypervisor::regs::CommonSpecialRegisters; -#[cfg(target_arch = "x86_64")] -use crate::hypervisor::regs::MsrEntry; use crate::mem::memory_region::MemoryRegion; #[cfg(crashdump)] use crate::mem::memory_region::{CrashDumpRegion, MemoryRegionFlags, MemoryRegionType}; @@ -308,8 +306,7 @@ where root_pt_gpas: &[u64], rsp_gva: u64, sregs: CommonSpecialRegisters, - #[cfg(target_arch = "x86_64")] msrs: Option>, - #[cfg(target_arch = "x86_64")] allowed_msrs: Option>, + #[cfg(target_arch = "x86_64")] msr_state: crate::sandbox::snapshot::SnapshotMsrState, next_action: NextAction, host_functions: HostFunctionDetails, ) -> Result { @@ -324,9 +321,7 @@ where rsp_gva, sregs, #[cfg(target_arch = "x86_64")] - msrs, - #[cfg(target_arch = "x86_64")] - allowed_msrs, + msr_state, next_action, self.original_entrypoint, self.snapshot_count, diff --git a/src/hyperlight_host/src/sandbox/config.rs b/src/hyperlight_host/src/sandbox/config.rs index ba5fffdb2..875ae0185 100644 --- a/src/hyperlight_host/src/sandbox/config.rs +++ b/src/hyperlight_host/src/sandbox/config.rs @@ -187,7 +187,20 @@ impl SandboxConfiguration { } /// Adds MSRs to the sandbox allow list. - /// VM creation verifies that the backend can restore them. + /// + /// Adds MSRs to the state captured by + /// [`MultiUseSandbox::snapshot`](crate::MultiUseSandbox::snapshot) and restored + /// by [`MultiUseSandbox::restore`](crate::MultiUseSandbox::restore). + /// + /// If this method is not called, only the platform's standard MSR set is + /// captured and restored. + /// + /// # Platform-specific behavior + /// + /// * KVM also uses this list to control guest MSR access. Access to unlisted + /// MSRs is denied. + /// * MSHV and WHP cannot restrict guest MSR access. An unlisted MSR is not + /// restored unless it belongs to the platform's standard MSR set. /// /// Duplicate indices, within the slice or against the existing list, are /// ignored and do not count toward capacity. diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 025826f7d..f6bd194b2 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -340,6 +340,10 @@ impl MultiUseSandbox { /// [`MultiUseSandbox::from_snapshot`] for the exact compatibility /// rules and the error variants returned on mismatch. /// + /// On x86_64, the snapshot includes the platform's standard MSR set and + /// each MSR selected with + /// [`SandboxConfiguration::allow_msrs`](crate::sandbox::SandboxConfiguration::allow_msrs). + /// /// ## Poisoned Sandbox /// /// This method will return [`crate::HyperlightError::PoisonedSandbox`] if the sandbox @@ -403,6 +407,8 @@ impl MultiUseSandbox { .map_err(|e| HyperlightError::HyperlightVmError(e.into()))?; #[cfg(target_arch = "x86_64")] let allowed_msrs = self.vm.get_msr_allow_list(); + #[cfg(target_arch = "x86_64")] + let msr_state = crate::sandbox::snapshot::SnapshotMsrState::new(msrs, allowed_msrs); let next_action = self.vm.get_next_action(); let host_functions = (&*self.host_funcs.try_lock().map_err(|e| { crate::new_error!("Error locking host_funcs at {}:{}: {}", file!(), line!(), e) @@ -415,9 +421,7 @@ impl MultiUseSandbox { stack_top_gpa, sregs, #[cfg(target_arch = "x86_64")] - Some(msrs), - #[cfg(target_arch = "x86_64")] - Some(allowed_msrs), + msr_state, next_action, host_functions, )?; @@ -439,6 +443,11 @@ impl MultiUseSandbox { /// [`SnapshotHostFunctionMismatch`](crate::HyperlightError::SnapshotHostFunctionMismatch) /// carrying the missing names and signature differences. /// + /// On x86_64, this restores the MSR state captured by + /// [`MultiUseSandbox::snapshot`]. + /// [`SandboxConfiguration::allow_msrs`](crate::sandbox::SandboxConfiguration::allow_msrs) + /// selects additional MSRs to capture and restore. + /// /// On x86_64 this sandbox's MSR allow list must be a superset of the one /// the snapshot was taken with, or the restore poisons with an MSR /// mismatch. @@ -2765,14 +2774,20 @@ mod tests { use super::*; use crate::HostFunctions; use crate::hypervisor::hyperlight_vm::{CreateHyperlightVmError, HyperlightVmError}; + use crate::hypervisor::regs::{ + APIC_BASE_X2APIC_ENABLE, MSR_APERF, MSR_BNDCFGS, MSR_CSTAR, MSR_DEBUGCTL, + MSR_INTERRUPT_SSP_TABLE_ADDR, MSR_KERNEL_GS_BASE as KERNEL_GS_BASE, MSR_LSTAR, + MSR_MPERF, MSR_MTRR_DEF_TYPE, MSR_MTRR_FIX64K_00000, MSR_PAT, MSR_PL0_SSP, MSR_PL1_SSP, + MSR_PL2_SSP, MSR_PL3_SSP, MSR_S_CET, MSR_SFMASK, MSR_SPEC_CTRL, MSR_STAR, + MSR_SYSENTER_CS as SYSENTER_CS, MSR_SYSENTER_EIP, MSR_SYSENTER_ESP, MSR_TSC, + MSR_TSC_ADJUST, MSR_TSC_AUX, MSR_TSC_DEADLINE, MSR_TSX_CTRL, MSR_U_CET, + MSR_UMWAIT_CONTROL, MSR_VIRT_SPEC_CTRL, MSR_XFD, MSR_XFD_ERR, MSR_XSS, + }; use crate::hypervisor::virtual_machine::{ CreateVmError, RegisterError, ResetVcpuError, VmError, }; use crate::sandbox::snapshot::Snapshot; - const KERNEL_GS_BASE: u32 = 0xC000_0102; - const SYSENTER_CS: u32 = 0x174; - fn assert_msr_not_allowable(error: &HyperlightError, expected: u32) { assert!( matches!( @@ -2787,18 +2802,6 @@ mod tests { ); } - fn assert_invalid_snapshot_msr(error: &HyperlightError) { - assert!( - matches!( - error, - HyperlightError::HyperlightVmError(HyperlightVmError::Restore( - ResetVcpuError::Register(RegisterError::InvalidSnapshotMsrIndex { .. }) - )) - ), - "expected InvalidSnapshotMsrIndex, got: {error:?}" - ); - } - fn assert_msr_not_allowed(error: &HyperlightError) { assert!( matches!( @@ -3128,126 +3131,43 @@ mod tests { } #[test] - #[cfg(target_arch = "x86_64")] - fn snapshot_without_msrs_uses_destination_reset_set() { - let mut source = UninitializedSandbox::new( - GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), - None, - ) - .unwrap() - .evolve() - .unwrap(); - let snapshot = source.snapshot().unwrap(); - source.snapshot = None; - let Ok(mut snap) = Arc::try_unwrap(snapshot) else { - panic!("snapshot should be uniquely owned"); - }; - // A snapshot without MSRs uses the destination baseline. - snap.set_msrs(None); - snap.set_allowed_msrs(None); - let snapshot = Arc::new(snap); + #[cfg(kvm)] + fn guest_cannot_enable_x2apic_through_apic_base() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; - let mut config = SandboxConfiguration::default(); - config.allow_msrs(&[KERNEL_GS_BASE]).unwrap(); - let mut target = UninitializedSandbox::new( - GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), - Some(config), - ) - .unwrap() - .evolve() - .unwrap(); - let baseline: u64 = target.call("ReadMSR", KERNEL_GS_BASE).unwrap(); - target - .call::<()>("WriteMSR", (KERNEL_GS_BASE, baseline ^ 0x55)) - .unwrap(); - assert_eq!( - target.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), - baseline ^ 0x55 - ); - target.restore(snapshot.clone()).unwrap(); - assert_eq!( - target.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), - baseline - ); - target - .call::<()>("WriteMSR", (KERNEL_GS_BASE, baseline ^ 0xAA)) - .unwrap(); - assert_eq!( - target.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), - baseline ^ 0xAA - ); + if !matches!(get_available_hypervisor(), Some(HypervisorType::Kvm)) { + return; + } - let mut clone = - MultiUseSandbox::from_snapshot(snapshot, HostFunctions::default(), Some(config)) - .unwrap(); - let clone_baseline: u64 = clone.call("ReadMSR", KERNEL_GS_BASE).unwrap(); - clone - .call::<()>("WriteMSR", (KERNEL_GS_BASE, clone_baseline ^ 0xCC)) - .unwrap(); - assert_eq!( - clone.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), - clone_baseline ^ 0xCC - ); - } + const MSR_IA32_APIC_BASE: u32 = 0x1B; + const MSR_X2APIC_BASE: u32 = 0x800; + const APIC_BASE_DEFAULT: u64 = 0xFEE0_0900; - #[test] - fn malformed_snapshot_msrs_poison_and_trusted_restore_recovers() { - let indices = [SYSENTER_CS, KERNEL_GS_BASE]; - let mut config = SandboxConfiguration::default(); - config.allow_msrs(&indices).unwrap(); - let mut source = UninitializedSandbox::new( + let mut sandbox = UninitializedSandbox::new( GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), - Some(config), + None, ) .unwrap() .evolve() .unwrap(); - let snapshot = source.snapshot().unwrap(); - source.snapshot = None; - let Ok(mut snapshot) = Arc::try_unwrap(snapshot) else { - panic!("snapshot should be uniquely owned"); - }; - let mut msrs = snapshot.msrs().unwrap().clone(); - msrs[0].index = 0xDEAD; - snapshot.set_msrs(Some(msrs)); - let snapshot = Arc::new(snapshot); + let snapshot = sandbox.snapshot().unwrap(); - let error = MultiUseSandbox::from_snapshot( - snapshot.clone(), - HostFunctions::default(), - Some(config), - ) - .expect_err("from_snapshot must reject malformed snapshot MSRs"); - assert_invalid_snapshot_msr(&error); + let x2apic_base = APIC_BASE_DEFAULT | APIC_BASE_X2APIC_ENABLE; + let result = sandbox.call::<()>("WriteMSR", (MSR_IA32_APIC_BASE, x2apic_base)); + assert!( + matches!(result, Err(HyperlightError::GuestAborted(_, _))), + "guest enabled x2APIC through APIC_BASE: {result:?}" + ); + assert!(sandbox.poisoned()); + sandbox.restore(snapshot).unwrap(); + assert!(!sandbox.poisoned()); - let mut target = UninitializedSandbox::new( - GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), - Some(config), - ) - .unwrap() - .evolve() - .unwrap(); - let trusted_value: u64 = target.call("ReadMSR", KERNEL_GS_BASE).unwrap(); - let recovery_snapshot = target.snapshot().unwrap(); - target - .call::<()>("WriteMSR", (KERNEL_GS_BASE, trusted_value ^ 0x55)) - .unwrap(); - let error = target - .restore(snapshot) - .expect_err("restore must reject malformed snapshot MSRs"); - assert_invalid_snapshot_msr(&error); - assert!(target.poisoned()); - assert!(matches!( - target.call::("Echo", "hi".to_string()), - Err(HyperlightError::PoisonedSandbox) - )); - - target.restore(recovery_snapshot).unwrap(); - assert!(!target.poisoned()); - assert_eq!( - target.call::("ReadMSR", KERNEL_GS_BASE).unwrap(), - trusted_value + let result = sandbox.call::<()>("WriteMSR", (MSR_X2APIC_BASE, 1u64)); + assert!( + matches!(result, Err(HyperlightError::GuestAborted(_, _))), + "x2APIC MSR access succeeded after restore: {result:?}" ); + assert!(sandbox.poisoned()); } #[test] @@ -3275,11 +3195,8 @@ mod tests { let result = sbox.call::("ReadMSR", msr_index); assert!( - matches!( - &result, - Err(HyperlightError::MsrReadViolation(idx)) if *idx == msr_index - ), - "RDMSR 0x{:X}: expected MsrReadViolation, got: {:?}", + matches!(&result, Err(HyperlightError::GuestAborted(_, _))), + "RDMSR 0x{:X}: expected direct #GP, got: {:?}", msr_index, result ); @@ -3289,17 +3206,69 @@ mod tests { let result = sbox.call::<()>("WriteMSR", (msr_index, 0x5u64)); assert!( - matches!( - &result, - Err(HyperlightError::MsrWriteViolation(idx, _)) if *idx == msr_index - ), - "WRMSR 0x{:X}: expected MsrWriteViolation, got: {:?}", + matches!(&result, Err(HyperlightError::GuestAborted(_, _))), + "WRMSR 0x{:X}: expected direct #GP, got: {:?}", msr_index, result ); assert!(sbox.poisoned()); } + #[test] + #[cfg(kvm)] + fn nested_virtualization_is_hidden_from_guest() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Kvm)) { + return; + } + + let mut sandbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + let features: u32 = sandbox.call("NestedVirtualizationCpuid", ()).unwrap(); + assert_eq!(features & 0b11, 0, "guest CPUID exposes VMX or SVM"); + } + + #[test] + #[cfg(kvm)] + fn nested_vmx_setup_msrs_are_denied() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Kvm)) { + return; + } + + let mut sandbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + let snapshot = sandbox.snapshot().unwrap(); + let vmx_basic: u32 = 0x480; + let result = sandbox.call::("ReadMSR", vmx_basic); + assert!( + matches!(result, Err(HyperlightError::GuestAborted(_, _))), + "RDMSR 0x{vmx_basic:X}: expected direct #GP, got: {result:?}" + ); + + sandbox.restore(snapshot).unwrap(); + let feature_control: u32 = 0x3A; + let result = sandbox.call::<()>("WriteMSR", (feature_control, 0x5u64)); + assert!( + matches!(result, Err(HyperlightError::GuestAborted(_, _))), + "WRMSR 0x{feature_control:X}: expected direct #GP, got: {result:?}" + ); + } + /// A write-only command cannot enter the reset set. #[test] #[cfg(target_arch = "x86_64")] @@ -3424,8 +3393,7 @@ mod tests { return; } - let cases: [(u32, bool); 2] = [(0x1D9, true), (0x800, false)]; - for (msr_index, expect_filter_violation) in cases { + for msr_index in [0x1D9_u32, 0x800] { let mut sbox = UninitializedSandbox::new( GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), None, @@ -3435,20 +3403,10 @@ mod tests { .unwrap(); let result = sbox.call::<()>("WriteMSR", (msr_index, 0x1u64)); - if expect_filter_violation { - assert!( - matches!( - &result, - Err(HyperlightError::MsrWriteViolation(idx, _)) if *idx == msr_index - ), - "WRMSR 0x{msr_index:X}: expected MsrWriteViolation, got: {result:?}" - ); - } else { - assert!( - matches!(&result, Err(HyperlightError::GuestAborted(_, _))), - "WRMSR 0x{msr_index:X}: expected direct #GP, got: {result:?}" - ); - } + assert!( + matches!(&result, Err(HyperlightError::GuestAborted(_, _))), + "WRMSR 0x{msr_index:X}: expected direct #GP, got: {result:?}" + ); assert!( sbox.poisoned(), "sandbox should be poisoned after a denied WRMSR to 0x{msr_index:X}" @@ -3456,54 +3414,42 @@ mod tests { } } - /// A rejected MSR restore poisons the sandbox on every backend. - /// - /// The host register set rejects the noncanonical KERNEL_GS_BASE value - /// on KVM, MSHV, and WHP alike, leaving the sandbox poisoned. #[test] - #[cfg(target_arch = "x86_64")] - fn rejected_msr_restore_poisons_sandbox() { - let mut sbox = UninitializedSandbox::new( + #[cfg(all(kvm, target_arch = "x86_64"))] + fn all_kvm_custom_msrs_are_denied() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Kvm)) { + return; + } + + const KVM_CUSTOM_MSR_START: u32 = 0x4B56_4D00; + const KVM_CUSTOM_MSR_END: u32 = 0x4B56_4DFF; + + let mut sandbox = UninitializedSandbox::new( GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), None, ) .unwrap() .evolve() .unwrap(); + let snapshot = sandbox.snapshot().unwrap(); - let baseline = sbox.snapshot().unwrap(); - sbox.snapshot = None; - let Ok(mut snap) = Arc::try_unwrap(baseline) else { - panic!("snapshot should be uniquely owned after clearing the cache"); - }; - // A noncanonical KERNEL_GS_BASE value the host set rejects. - let mut msrs = snap.msrs().unwrap().clone(); - let kernel_gs_base = msrs - .iter_mut() - .find(|entry| entry.index == 0xC000_0102) - .expect("KERNEL_GS_BASE should be in the reset set"); - kernel_gs_base.value = 0xDEAD_0000_0000_0000; - snap.set_msrs(Some(msrs)); - let snap = Arc::new(snap); - - let err = sbox - .restore(snap) - .expect_err("restore should fail on a rejected MSR set"); - assert!( - format!("{err:?}").to_lowercase().contains("msr") - || format!("{err:?}").to_lowercase().contains("restore"), - "expected an MSR restore error, got: {err:?}" - ); - assert!( - sbox.poisoned(), - "sandbox should be poisoned after failed restore" - ); + for index in KVM_CUSTOM_MSR_START..=KVM_CUSTOM_MSR_END { + let result = sandbox.call::("ReadMSR", index); + assert!( + matches!(result, Err(HyperlightError::GuestAborted(_, _))), + "RDMSR {index:#x} was not denied: {result:?}" + ); + sandbox.restore(snapshot.clone()).unwrap(); - let call = sbox.call::("Echo", "hi".to_string()); - assert!( - matches!(call, Err(HyperlightError::PoisonedSandbox)), - "poisoned sandbox should reject guest calls, got: {call:?}" - ); + let result = sandbox.call::<()>("WriteMSR", (index, 1u64)); + assert!( + matches!(result, Err(HyperlightError::GuestAborted(_, _))), + "WRMSR {index:#x} was not denied: {result:?}" + ); + sandbox.restore(snapshot.clone()).unwrap(); + } } /// Unresettable feature-class MSRs must not retain guest writes. PMU, @@ -3555,15 +3501,15 @@ mod tests { #[test] #[cfg(target_arch = "x86_64")] fn runtime_msr_table_entries_are_justified() { - use crate::hypervisor::regs::core_reset_indices; + use crate::hypervisor::regs::resettable_msr_indices; #[cfg(kvm)] - let kernel_gs_uses_instruction_side_effect = matches!( + let is_kvm = matches!( crate::hypervisor::virtual_machine::get_available_hypervisor(), Some(crate::hypervisor::virtual_machine::HypervisorType::Kvm) ); #[cfg(not(kvm))] - let kernel_gs_uses_instruction_side_effect = false; + let is_kvm = false; let mut sbox = UninitializedSandbox::new( GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), @@ -3582,18 +3528,16 @@ mod tests { .map(|entry| entry.index) .collect(); - for index in core_reset_indices() { + for index in resettable_msr_indices() { if !reset_indices.contains(&index) { assert_omitted_msr_does_not_retain(&mut sbox, index); - } else if kernel_gs_uses_instruction_side_effect && index == KERNEL_GS_BASE { + } else if is_kvm && index == KERNEL_GS_BASE { // Direct WRMSR is denied. The dedicated SWAPGS test proves // the instruction-side mutation is restored. - } else if (index == 0x10 && !kernel_gs_uses_instruction_side_effect) - || matches!(index, 0xE7 | 0xE8) - { + } else if (index == MSR_TSC && !is_kvm) || matches!(index, MSR_MPERF | MSR_APERF) { assert_guest_counter_is_writable_and_restored(&mut sbox, index); - } else if let Some(sentinel) = positive_write_sentinel(index) { - assert_guest_msr_is_writable_and_restored(&mut sbox, index, sentinel); + } else if let Some(test_value) = guest_write_test_value(index) { + assert_guest_msr_is_writable_and_restored(&mut sbox, index, test_value); } else { assert!( reset_exception_reason(index).is_some(), @@ -3613,7 +3557,7 @@ mod tests { return; } }; - let preferred = positive_write_sentinel(index).unwrap_or(original ^ 1); + let preferred = guest_write_test_value(index).unwrap_or(original ^ 1); let candidates = [preferred, original ^ 1, original ^ 2, 0, 1, 0x1000]; for candidate in candidates { @@ -3641,40 +3585,52 @@ mod tests { } } - fn positive_write_sentinel(index: u32) -> Option { + fn guest_write_test_value(index: u32) -> Option { match index { - 0x174 => Some(0x10), // SYSENTER_CS - 0x175 | 0x176 => Some(0x1000), // SYSENTER_ESP/EIP - 0x277 => Some(0x0007_0406_0007_0406), // PAT - 0xC000_0081 => Some(0x001B_0008_0000_0000), // STAR - 0xC000_0082 | 0xC000_0083 => Some(0x1000), // LSTAR/CSTAR - 0xC000_0084 => Some(0x200), // SFMASK - 0xC000_0102 => Some(0x1000), // KERNEL_GS_BASE - 0x3B => Some(0x1000), // TSC_ADJUST - 0xC000_0103 => Some(0x5), // TSC_AUX - 0x2FF => Some(0xC00), // MTRR_DEF_TYPE + SYSENTER_CS => Some(0x10), + MSR_SYSENTER_ESP | MSR_SYSENTER_EIP => Some(0x1000), + MSR_PAT => Some(0x0007_0406_0007_0406), + MSR_STAR => Some(0x001B_0008_0000_0000), + MSR_LSTAR | MSR_CSTAR => Some(0x1000), + MSR_SFMASK => Some(0x200), + KERNEL_GS_BASE => Some(0x1000), + MSR_TSC_ADJUST => Some(0x1000), + MSR_TSC_AUX => Some(0x5), + MSR_MTRR_DEF_TYPE => Some(0xC00), 0x200..=0x21F if index & 1 == 0 => Some(0x6), // MTRR_PHYSBASEn 0x200..=0x21F => Some(0x800), // MTRR_PHYSMASKn - 0x250 | 0x258 | 0x259 | 0x268..=0x26F => Some(0x0606_0606_0606_0606), + MSR_MTRR_FIX64K_00000 | 0x258 | 0x259 | 0x268..=0x26F => { + Some(0x0606_0606_0606_0606) + } _ => None, } } fn reset_exception_reason(index: u32) -> Option<&'static str> { match index { - 0x10 => Some("KVM denies direct guest TSC MSR access"), - 0x1D9 => Some("DEBUGCTL support depends on exposed debug features"), - 0x48 => Some("SPEC_CTRL writable bits depend on mitigation features"), - 0x6A0 | 0x6A2 | 0x6A4..=0x6A8 => { + MSR_TSC => Some("KVM denies direct guest TSC MSR access"), + MSR_DEBUGCTL => Some("DEBUGCTL support depends on exposed debug features"), + MSR_SPEC_CTRL => Some("SPEC_CTRL writable bits depend on mitigation features"), + MSR_U_CET + | MSR_S_CET + | MSR_PL0_SSP + | MSR_PL1_SSP + | MSR_PL2_SSP + | MSR_PL3_SSP + | MSR_INTERRUPT_SSP_TABLE_ADDR => { Some("CET writable state depends on exposed CET features") } - 0x122 => Some("TSX_CTRL writable bits depend on exposed TSX features"), - 0x1C4 | 0x1C5 => Some("XFD writable bits depend on exposed XSAVE features"), - 0xE1 => Some("UMWAIT_CONTROL writable bits depend on exposed WAITPKG features"), - 0x6E0 => Some("TSC_DEADLINE writable bits depend on exposed APIC-timer features"), - 0xD90 => Some("BNDCFGS writable bits depend on exposed MPX features"), - 0xDA0 => Some("XSS writable bits depend on exposed XSAVE features"), - 0xC001_011F => { + MSR_TSX_CTRL => Some("TSX_CTRL writable bits depend on exposed TSX features"), + MSR_XFD | MSR_XFD_ERR => Some("XFD writable bits depend on exposed XSAVE features"), + MSR_UMWAIT_CONTROL => { + Some("UMWAIT_CONTROL writable bits depend on exposed WAITPKG features") + } + MSR_TSC_DEADLINE => { + Some("TSC_DEADLINE writable bits depend on exposed APIC-timer features") + } + MSR_BNDCFGS => Some("BNDCFGS writable bits depend on exposed MPX features"), + MSR_XSS => Some("XSS writable bits depend on exposed XSAVE features"), + MSR_VIRT_SPEC_CTRL => { Some("VIRT_SPEC_CTRL writable bits depend on exposed AMD SSBD virtualization") } _ => None, @@ -3950,22 +3906,22 @@ mod tests { .evolve() .unwrap(); - let base = sbox.vm.capture_msrs_for_test(&[0x10]).unwrap()[0].value; + let base = sbox.vm.capture_msrs_for_test(&[MSR_TSC]).unwrap()[0].value; let jump = base.wrapping_add(1 << 60); - sbox.call::<()>("WriteMSR", (0x10u32, jump)).unwrap(); - let planted: u64 = sbox.call("ReadMSR", 0x10u32).unwrap(); + sbox.call::<()>("WriteMSR", (MSR_TSC, jump)).unwrap(); + let planted: u64 = sbox.call("ReadMSR", MSR_TSC).unwrap(); assert!( planted >= jump, "guest TSC write did not take (planted=0x{planted:X} jump=0x{jump:X})" ); assert!( - sbox.vm.try_set_msr_for_test(0x10, base), + sbox.vm.try_set_msr_for_test(MSR_TSC, base), "host set of HV_X64_REGISTER_TSC failed" ); - let after: u64 = sbox.call("ReadMSR", 0x10u32).unwrap(); + let after: u64 = sbox.call("ReadMSR", MSR_TSC).unwrap(); eprintln!( "mshv TSC writeback probe: base=0x{base:X} jump=0x{jump:X} planted=0x{planted:X} after=0x{after:X}" ); diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs index 4286cb993..4d918302d 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs @@ -190,17 +190,12 @@ pub(super) struct OciSnapshotConfig { /// Special registers captured from the paused vCPU, restored /// verbatim when resuming the call. pub(super) sregs: CommonSpecialRegisters, - /// MSR reset state captured from the paused vCPU. `None` uses the - /// destination sandbox's reset set. + /// Complete MSR reset state and capture-time guest access policy. + /// `msrs` contains every reset value. `allowed_msrs` is the policy subset. #[cfg(target_arch = "x86_64")] + // Legacy configs omit this field. Empty state restores the destination baseline. #[serde(default)] - pub(super) msrs: Option>, - /// Allow list of the sandbox that captured this snapshot. Present - /// exactly when `msrs` is, and checked against the destination allow - /// list on load. - #[cfg(target_arch = "x86_64")] - #[serde(default)] - pub(super) allowed_msrs: Option>, + pub(super) msr_state: OciSnapshotMsrState, pub(super) layout: MemoryLayout, /// Total size of the memory blob in bytes (including the guest /// page-table tail, if any). Equal to `self.memory.mem_size()`. @@ -215,6 +210,14 @@ pub(super) struct OciSnapshotConfig { pub(super) snapshot_generation: u64, } +#[cfg(target_arch = "x86_64")] +#[derive(Default, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub(super) struct OciSnapshotMsrState { + pub(super) msrs: Vec, + pub(super) allowed_msrs: Vec, +} + /// Sizes and permissions of the regions inside the snapshot blob, /// enough for the loader to rebuild a `SandboxMemoryLayout`. #[derive(Serialize, Deserialize)] @@ -666,30 +669,28 @@ mod tests { ])); let json = serde_json::to_vec(&original).unwrap(); let restored: OciSnapshotConfig = serde_json::from_slice(&json).unwrap(); - assert_eq!(restored.msrs, original.msrs); + assert_eq!(restored.msr_state.msrs, original.msr_state.msrs); + assert_eq!( + restored.msr_state.allowed_msrs, + original.msr_state.allowed_msrs + ); } - /// A config JSON with no MSR keys deserializes both fields to `None`. + /// A config JSON with no MSR state deserializes empty state. #[cfg(target_arch = "x86_64")] #[test] - fn config_without_msr_keys_deserializes_to_none() { + fn config_without_msr_state_deserializes_to_empty_state() { let with = gating_config_with_msrs(Some(vec![MsrEntry { index: 0x10, value: 1, }])); let mut json: serde_json::Value = serde_json::from_slice(&serde_json::to_vec(&with).unwrap()).unwrap(); - assert!(json.as_object_mut().unwrap().remove("msrs").is_some()); - assert!( - json.as_object_mut() - .unwrap() - .remove("allowed_msrs") - .is_some() - ); + assert!(json.as_object_mut().unwrap().remove("msr_state").is_some()); let restored: OciSnapshotConfig = serde_json::from_value(json).unwrap(); - assert_eq!(restored.msrs, None); - assert_eq!(restored.allowed_msrs, None); + assert!(restored.msr_state.msrs.is_empty()); + assert!(restored.msr_state.allowed_msrs.is_empty()); } /// Every `ParameterType` survives the round-trip through its serde @@ -778,9 +779,7 @@ mod tests { original_entrypoint_addr: 0, sregs: distinct_sregs(), #[cfg(target_arch = "x86_64")] - msrs: None, - #[cfg(target_arch = "x86_64")] - allowed_msrs: None, + msr_state: OciSnapshotMsrState::default(), layout: MemoryLayout { input_data_size: 0, output_data_size: 0, @@ -801,10 +800,11 @@ mod tests { /// `gating_config` with a chosen MSR set, for serde tests. #[cfg(target_arch = "x86_64")] fn gating_config_with_msrs(msrs: Option>) -> OciSnapshotConfig { - let allowed_msrs = msrs.as_ref().map(|_| Vec::new()); OciSnapshotConfig { - msrs, - allowed_msrs, + msr_state: OciSnapshotMsrState { + msrs: msrs.unwrap_or_default(), + allowed_msrs: Vec::new(), + }, ..gating_config() } } @@ -1005,20 +1005,22 @@ mod schema_pin { 11 ] }, - "msrs": [ - { - "index": 16, - "value": 42 + "msr_state": { + "msrs": [ + { + "index": 16, + "value": 42 + }, + { + "index": 3221225474, + "value": 3735928559 + } + ], + "allowed_msrs": [ + 16, + 3221225474 + ] }, - { - "index": 3221225474, - "value": 3735928559 - } - ], - "allowed_msrs": [ - 16, - 3221225474 - ], "layout": { "input_data_size": 1, "output_data_size": 2, @@ -1097,12 +1099,15 @@ mod schema_pin { ]"#; fn assert_round_trip(pinned: &str) { + let pinned_value: serde_json::Value = + serde_json::from_str(pinned).expect("pinned JSON must deserialize as a value"); let parsed: OciSnapshotConfig = - serde_json::from_str(pinned).expect("pinned JSON must deserialize"); + serde_json::from_value(pinned_value.clone()).expect("pinned JSON must deserialize"); let actual = serde_json::to_string_pretty(&parsed).expect("serialize"); + let actual_value: serde_json::Value = + serde_json::from_str(&actual).expect("serialized config must deserialize"); assert_eq!( - actual.trim(), - pinned.trim(), + actual_value, pinned_value, "Snapshot config JSON schema changed. If the change can break \ existing snapshots on disk, bump `MT_CONFIG_V1` in \ `super::media_types` and follow `docs/snapshot-versioning.md`. \ diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs index 9e03873a4..4920826cc 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs @@ -32,6 +32,8 @@ use oci_spec::image::{ ImageManifestBuilder, MediaType, SCHEMA_VERSION, }; +#[cfg(target_arch = "x86_64")] +use self::config::OciSnapshotMsrState; use self::config::{Arch, CpuVendor, HostFunction, Hypervisor, MemoryLayout, OciSnapshotConfig}; use self::digest::{Digest256, oci_digest, parse_oci_digest, verify_blob_bytes, verify_blob_file}; use self::fsutil::{put_blob, put_blob_if_absent, read_bounded, replace_file_atomic}; @@ -42,6 +44,8 @@ pub(super) use self::media_types::{ MT_CONFIG_CURRENT, MT_CONFIG_V1, MT_SNAPSHOT_CURRENT, MT_SNAPSHOT_V1, SNAPSHOT_ABI_VERSION, }; use self::reference::{OciDigest, OciReference, OciTag}; +#[cfg(target_arch = "x86_64")] +use super::SnapshotMsrState; use super::{NextAction, Snapshot}; use crate::mem::layout::SandboxMemoryLayout; use crate::mem::memory_region::MemoryRegionFlags; @@ -612,9 +616,16 @@ impl Snapshot { original_entrypoint_addr: self.original_entrypoint, sregs: *sregs, #[cfg(target_arch = "x86_64")] - msrs: self.msrs.clone(), - #[cfg(target_arch = "x86_64")] - allowed_msrs: self.allowed_msrs.clone(), + msr_state: { + let state = self + .msr_state + .as_ref() + .ok_or_else(|| crate::new_error!("snapshot has no MSR state"))?; + OciSnapshotMsrState { + msrs: state.msrs.clone(), + allowed_msrs: state.allowed_msrs.clone(), + } + }, layout: MemoryLayout { input_data_size: l.input_data_size(), output_data_size: l.output_data_size(), @@ -880,15 +891,6 @@ impl Snapshot { // 8. Build the next action + sregs back from the config. let next_action = NextAction::Call(cfg.entrypoint_addr); - // `msrs` and `allowed_msrs` travel together. A config with one but - // not the other cannot enforce the allow-list check on restore. - #[cfg(target_arch = "x86_64")] - if cfg.msrs.is_some() != cfg.allowed_msrs.is_some() { - return Err(crate::new_error!( - "snapshot config inconsistent: msrs and allowed_msrs must both be present or both absent" - )); - } - // 9. Reconstitute host_functions metadata. let snapshot_generation = cfg.snapshot_generation; let host_funcs_vec: Vec< @@ -911,9 +913,10 @@ impl Snapshot { stack_top_gva: cfg.stack_top_gva, sregs: Some(cfg.sregs), #[cfg(target_arch = "x86_64")] - msrs: cfg.msrs, - #[cfg(target_arch = "x86_64")] - allowed_msrs: cfg.allowed_msrs, + msr_state: Some(SnapshotMsrState::new( + cfg.msr_state.msrs, + cfg.msr_state.allowed_msrs, + )), next_action, original_entrypoint: cfg.original_entrypoint_addr, snapshot_generation, diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index 6babfa3e4..2152da41b 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -229,7 +229,7 @@ fn msrs_round_trip_via_disk() { assert_eq!(loaded.allowed_msrs(), Some(&[][..])); } -/// A config with no `msrs` key loads and uses the destination reset set. +/// A config with no `msr_state` key loads and uses the destination reset set. #[cfg(target_arch = "x86_64")] #[test] fn snapshot_without_msrs_key_loads_and_runs() { @@ -237,17 +237,14 @@ fn snapshot_without_msrs_key_loads_and_runs() { rewrite_config(&path, |cfg| { let obj = cfg.as_object_mut().unwrap(); assert!( - obj.remove("msrs").is_some(), - "a running x86_64 snapshot config should carry msrs to remove" - ); - assert!( - obj.remove("allowed_msrs").is_some(), - "a running x86_64 snapshot config should carry allowed_msrs to remove" + obj.remove("msr_state").is_some(), + "a running x86_64 snapshot config should carry msr_state to remove" ); }); let loaded = Snapshot::checked_load(&path, OciTag::new("latest").unwrap()).unwrap(); - assert_eq!(loaded.msrs(), None); + assert_eq!(loaded.msrs(), Some(&Vec::new())); + assert_eq!(loaded.allowed_msrs(), Some(&[][..])); let mut sbox = MultiUseSandbox::from_snapshot(Arc::new(loaded), HostFunctions::default(), None).unwrap(); @@ -376,8 +373,7 @@ fn disk_snapshot_non_superset_allow_list_rejected() { assert!(target.poisoned()); } -/// A config with `msrs` but no `allowed_msrs` cannot enforce the -/// allow-list check, so the load rejects it. +/// An MSR state without `allowed_msrs` cannot enforce the allow-list check. #[cfg(target_arch = "x86_64")] #[test] fn disk_snapshot_msrs_without_allow_list_rejected() { @@ -385,6 +381,10 @@ fn disk_snapshot_msrs_without_allow_list_rejected() { rewrite_config(&path, |cfg| { assert!( cfg.as_object_mut() + .unwrap() + .get_mut("msr_state") + .unwrap() + .as_object_mut() .unwrap() .remove("allowed_msrs") .is_some(), @@ -398,7 +398,7 @@ fn disk_snapshot_msrs_without_allow_list_rejected() { )); assert!( format!("{err:?}").contains("allowed_msrs"), - "expected an msrs/allowed_msrs consistency error, got: {err:?}" + "expected a missing allowed_msrs error, got: {err:?}" ); } diff --git a/src/hyperlight_host/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index c3cfc2c17..bff08ec71 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -67,6 +67,20 @@ pub enum NextAction { None, } +#[cfg(target_arch = "x86_64")] +#[derive(Clone)] +pub(crate) struct SnapshotMsrState { + msrs: Vec, + allowed_msrs: Vec, +} + +#[cfg(target_arch = "x86_64")] +impl SnapshotMsrState { + pub(crate) fn new(msrs: Vec, allowed_msrs: Vec) -> Self { + Self { msrs, allowed_msrs } + } +} + /// A wrapper around a `SharedMemory` reference and a snapshot /// of the memory therein pub struct Snapshot { @@ -94,17 +108,9 @@ pub struct Snapshot { /// tables are relocated during snapshot. sregs: Option, - /// MSR reset state captured from a running vCPU. - /// None for a pre-init snapshot or an old on-disk snapshot - /// predating MSR capture. - #[cfg(target_arch = "x86_64")] - msrs: Option>, - - /// The allow list of the sandbox that captured this snapshot, - /// sorted and deduplicated. Present exactly when `msrs` is present. - /// A restore requires the destination allow list to be a superset. + /// MSR reset state and capture-time allow list from a running vCPU. #[cfg(target_arch = "x86_64")] - allowed_msrs: Option>, + msr_state: Option, /// The next action that should be performed on this snapshot next_action: NextAction, @@ -407,9 +413,7 @@ impl Snapshot { stack_top_gva: exn_stack_top_gva, sregs: None, #[cfg(target_arch = "x86_64")] - msrs: None, - #[cfg(target_arch = "x86_64")] - allowed_msrs: None, + msr_state: None, next_action: NextAction::Initialise(entrypoint_gva), original_entrypoint: entrypoint_gva, snapshot_generation: 0, @@ -437,8 +441,7 @@ impl Snapshot { root_pt_gpas: &[u64], stack_top_gva: u64, sregs: CommonSpecialRegisters, - #[cfg(target_arch = "x86_64")] msrs: Option>, - #[cfg(target_arch = "x86_64")] allowed_msrs: Option>, + #[cfg(target_arch = "x86_64")] msr_state: SnapshotMsrState, next_action: NextAction, original_entrypoint: u64, snapshot_generation: u64, @@ -594,9 +597,7 @@ impl Snapshot { stack_top_gva, sregs: Some(sregs), #[cfg(target_arch = "x86_64")] - msrs, - #[cfg(target_arch = "x86_64")] - allowed_msrs, + msr_state: Some(msr_state), next_action, original_entrypoint, snapshot_generation, @@ -644,25 +645,15 @@ impl Snapshot { /// Returns the captured MSR reset state. #[cfg(target_arch = "x86_64")] pub(crate) fn msrs(&self) -> Option<&Vec> { - self.msrs.as_ref() + self.msr_state.as_ref().map(|state| &state.msrs) } /// Returns the capturing sandbox's allow list. #[cfg(target_arch = "x86_64")] pub(crate) fn allowed_msrs(&self) -> Option<&[u32]> { - self.allowed_msrs.as_deref() - } - - /// Stores captured MSR reset state. - #[cfg(all(test, target_arch = "x86_64"))] - pub(crate) fn set_msrs(&mut self, msrs: Option>) { - self.msrs = msrs; - } - - /// Stores the capturing sandbox's allow list. - #[cfg(all(test, target_arch = "x86_64"))] - pub(crate) fn set_allowed_msrs(&mut self, allowed: Option>) { - self.allowed_msrs = allowed; + self.msr_state + .as_ref() + .map(|state| state.allowed_msrs.as_slice()) } pub(crate) fn next_action(&self) -> NextAction { @@ -831,9 +822,7 @@ mod tests { 0, default_sregs(), #[cfg(target_arch = "x86_64")] - None, - #[cfg(target_arch = "x86_64")] - None, + super::SnapshotMsrState::new(Vec::new(), Vec::new()), super::NextAction::None, 0, 1, @@ -853,9 +842,7 @@ mod tests { 0, default_sregs(), #[cfg(target_arch = "x86_64")] - None, - #[cfg(target_arch = "x86_64")] - None, + super::SnapshotMsrState::new(Vec::new(), Vec::new()), super::NextAction::None, 0, 2, diff --git a/src/tests/rust_guests/simpleguest/src/main.rs b/src/tests/rust_guests/simpleguest/src/main.rs index efe6badab..536e64cf0 100644 --- a/src/tests/rust_guests/simpleguest/src/main.rs +++ b/src/tests/rust_guests/simpleguest/src/main.rs @@ -1112,6 +1112,18 @@ fn read_msr(msr: u32) -> u64 { ((read_edx as u64) << 32) | (read_eax as u64) } +#[guest_function("NestedVirtualizationCpuid")] +#[cfg(target_arch = "x86_64")] +#[allow(unused_unsafe)] +fn nested_virtualization_cpuid() -> u32 { + // SAFETY: CPUID leaves 0, 1, and 0x80000000 are always available on x86_64. + let vmx = unsafe { core::arch::x86_64::__cpuid(1) }.ecx & (1 << 5) != 0; + let max_extended = unsafe { core::arch::x86_64::__cpuid(0x8000_0000) }.eax; + let svm = max_extended >= 0x8000_0001 + && unsafe { core::arch::x86_64::__cpuid(0x8000_0001) }.ecx & (1 << 2) != 0; + u32::from(vmx) | (u32::from(svm) << 1) +} + #[guest_function("WriteKernelGsBaseViaSwapgs")] #[cfg(target_arch = "x86_64")] fn write_kernel_gs_base_via_swapgs(value: u64) { From f4044e20faba979b977841d8dbcbd343b0a6208d Mon Sep 17 00:00:00 2001 From: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:56:27 -0700 Subject: [PATCH 3/4] Reset active SSP across restore on Hyper-V Active SSP is guest-writable state exposed through the Hyper-V VP register API, not an architectural MSR. Add it to the Hyper-V reset candidates and the WHP register map so restore clears it, while keeping it out of the allow-list surface. Guest tests read SSP and TSC to confirm neither leaks across restore. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> --- docs/msr.md | 13 +- .../src/hypervisor/hyperlight_vm/x86_64.rs | 15 --- .../src/hypervisor/regs/x86_64/msrs.rs | 11 +- .../src/hypervisor/virtual_machine/whp.rs | 15 ++- .../src/sandbox/initialized_multi_use.rs | 55 ++++---- src/tests/rust_guests/simpleguest/src/main.rs | 122 ++++++++++++++++++ 6 files changed, 175 insertions(+), 56 deletions(-) diff --git a/docs/msr.md b/docs/msr.md index 636647001..19f1c745a 100644 --- a/docs/msr.md +++ b/docs/msr.md @@ -45,8 +45,8 @@ the destination baseline are shared in `MsrResetState`. Every reset entry must represent guest-writable retained state that the host can read and write. The shared candidate table is derived from the Hyper-V source and must be audited when that source, register mappings, or feature exposure -changes. VM creation probes host reads. Host write support currently holds by -table construction and round-trip tests. +changes. VM creation probes host reads. Table construction and round-trip +tests currently cover host write support. ## Snapshot validation and access policy @@ -69,6 +69,11 @@ also supports at most 16 contiguous filter ranges. Each allowed index must be resettable, host-readable, and host-writable. Write-only command MSRs such as `PRED_CMD` and `FLUSH_CMD` hold no resettable state and cannot be allowed. +Reset and allowable are not the same set. Active SSP (`0x7A0`) is reset on the +Hyper-V backends but cannot be allowed. It has no architectural `RDMSR`/`WRMSR` +and is reachable only through the VP register API, so no guest `WRMSR` sets it +and no filter range names it. + MSHV and WHP cannot enforce the allow list during guest execution. They retain it in snapshots so restore compatibility has the same meaning on every backend. @@ -117,7 +122,8 @@ poisons the sandbox. MSHV and WHP build their reset sets from: -* Retained-state candidates that the backend maps and can read. +* Retained-state candidates the backend maps and can read, including state + reachable only through the VP register API such as active SSP. * MTRRs required by the virtual CPU's `MTRRCAP`. * The validated allow list. @@ -185,6 +191,7 @@ These MSRs are reset when supported by the selected backend and host. | DEBUGCTL (`0x1D9`) | Debug control state. | | SPEC_CTRL (`0x48`), VIRT_SPEC_CTRL (`0xC001_011F`) | Speculation control state. | | CET (`0x6A0`, `0x6A2`, `0x6A4`-`0x6A8`) | CET control and shadow-stack state. | +| Active SSP (`0x7A0`) | Shadow-stack pointer. Reset on Hyper-V backends through the VP register API. Not allowable: no architectural `RDMSR`/`WRMSR`. | | XSS (`0xDA0`) | Extended supervisor state mask. | | TSC, TSC_ADJUST, TSC_AUX (`0x10`, `0x3B`, `0xC000_0103`) | Guest clock state. | | MTRRs (`0x2FF`, `0x200`-`0x21F`, `0x250`, `0x258`-`0x259`, `0x268`-`0x26F`) | Memory-type state. | diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs index 960c2fbf5..51c6a520b 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -317,21 +317,6 @@ impl HyperlightVm { Ok(()) } - /// Reads arbitrary backend MSRs for tests. - #[cfg(all(test, mshv3, target_arch = "x86_64"))] - pub(crate) fn capture_msrs_for_test( - &self, - indices: &[u32], - ) -> std::result::Result, RegisterError> { - self.vm.msrs(indices) - } - - /// Attempts one backend MSR write for tests. - #[cfg(all(test, mshv3, target_arch = "x86_64"))] - pub(crate) fn try_set_msr_for_test(&self, index: u32, value: u64) -> bool { - self.vm.set_msrs(&[MsrEntry { index, value }]).is_ok() - } - /// Dispatch a call from the host to the guest using the given pointer /// to the dispatch function _in the guest's address space_. /// diff --git a/src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs b/src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs index d930a3691..aabdf9fe4 100644 --- a/src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs +++ b/src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs @@ -146,6 +146,7 @@ pub(crate) const MSR_PL1_SSP: u32 = 0x6A5; pub(crate) const MSR_PL2_SSP: u32 = 0x6A6; pub(crate) const MSR_PL3_SSP: u32 = 0x6A7; pub(crate) const MSR_INTERRUPT_SSP_TABLE_ADDR: u32 = 0x6A8; +pub(crate) const MSR_IA32_SSP: u32 = 0x7A0; pub(crate) const MSR_TSC_DEADLINE: u32 = 0x6E0; pub(crate) const MSR_BNDCFGS: u32 = 0xD90; pub(crate) const MSR_XSS: u32 = 0xDA0; @@ -216,6 +217,10 @@ const NON_MTRR_RESETTABLE_MSRS: &[u32] = &[ MSR_BNDCFGS, ]; +// State exposed through Hyper-V's VP register API, but not through an +// architectural RDMSR/WRMSR pair suitable for allow_msrs. +const HYPERV_ONLY_RESETTABLE_MSRS: &[u32] = &[MSR_IA32_SSP]; + const MTRR_RESET_INDICES: &[u32] = &[ // Hyper-V accepts fixed-MTRR writes even when MTRRCAP.FIX is clear. MSR_MTRR_DEF_TYPE, @@ -271,13 +276,17 @@ pub(crate) fn is_resettable_msr(index: u32) -> bool { /// Returns non-MTRR candidates probed by filterless Hyper-V backends. pub(crate) fn filterless_core_reset_candidates() -> impl Iterator { - NON_MTRR_RESETTABLE_MSRS.iter().copied() + NON_MTRR_RESETTABLE_MSRS + .iter() + .chain(HYPERV_ONLY_RESETTABLE_MSRS) + .copied() } #[cfg(test)] pub(crate) fn resettable_msr_indices() -> impl Iterator { NON_MTRR_RESETTABLE_MSRS .iter() + .chain(HYPERV_ONLY_RESETTABLE_MSRS) .chain(MTRR_RESET_INDICES) .copied() } diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs index 314b8fc01..6595f14d2 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs @@ -33,13 +33,13 @@ use windows_result::HRESULT; use crate::hypervisor::gdb::{DebugError, DebuggableVm}; use crate::hypervisor::regs::{ Align16, CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, - FP_CONTROL_WORD_DEFAULT, MSR_APERF, MSR_BNDCFGS, MSR_CSTAR, MSR_INTERRUPT_SSP_TABLE_ADDR, - MSR_KERNEL_GS_BASE, MSR_LSTAR, MSR_MPERF, MSR_MTRR_CAP, MSR_PAT, MSR_PL0_SSP, MSR_PL1_SSP, - MSR_PL2_SSP, MSR_PL3_SSP, MSR_S_CET, MSR_SFMASK, MSR_SPEC_CTRL, MSR_STAR, MSR_SYSENTER_CS, - MSR_SYSENTER_EIP, MSR_SYSENTER_ESP, MSR_TSC_DEADLINE, MSR_TSX_CTRL, MSR_U_CET, - MSR_UMWAIT_CONTROL, MSR_XFD, MSR_XFD_ERR, MXCSR_DEFAULT, MsrEntry, WHP_DEBUG_REGS_NAMES, - WHP_DEBUG_REGS_NAMES_LEN, WHP_FPU_NAMES, WHP_FPU_NAMES_LEN, WHP_REGS_NAMES, WHP_REGS_NAMES_LEN, - WHP_SREGS_NAMES, WHP_SREGS_NAMES_LEN, + FP_CONTROL_WORD_DEFAULT, MSR_APERF, MSR_BNDCFGS, MSR_CSTAR, MSR_IA32_SSP, + MSR_INTERRUPT_SSP_TABLE_ADDR, MSR_KERNEL_GS_BASE, MSR_LSTAR, MSR_MPERF, MSR_MTRR_CAP, MSR_PAT, + MSR_PL0_SSP, MSR_PL1_SSP, MSR_PL2_SSP, MSR_PL3_SSP, MSR_S_CET, MSR_SFMASK, MSR_SPEC_CTRL, + MSR_STAR, MSR_SYSENTER_CS, MSR_SYSENTER_EIP, MSR_SYSENTER_ESP, MSR_TSC_DEADLINE, MSR_TSX_CTRL, + MSR_U_CET, MSR_UMWAIT_CONTROL, MSR_XFD, MSR_XFD_ERR, MXCSR_DEFAULT, MsrEntry, + WHP_DEBUG_REGS_NAMES, WHP_DEBUG_REGS_NAMES_LEN, WHP_FPU_NAMES, WHP_FPU_NAMES_LEN, + WHP_REGS_NAMES, WHP_REGS_NAMES_LEN, WHP_SREGS_NAMES, WHP_SREGS_NAMES_LEN, }; use crate::hypervisor::surrogate_process::SurrogateProcess; use crate::hypervisor::surrogate_process_manager::{ @@ -98,6 +98,7 @@ fn msr_to_whv_register_name(index: u32) -> Option { MSR_PL2_SSP => WHvX64RegisterPl2Ssp, MSR_PL3_SSP => WHvX64RegisterPl3Ssp, MSR_INTERRUPT_SSP_TABLE_ADDR => WHvX64RegisterInterruptSspTableAddr, + MSR_IA32_SSP => WHvX64RegisterSsp, // TSC and TSC offset/aux. 0x10 => WHvX64RegisterTsc, 0x3B => WHvX64RegisterTscAdjust, diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index f6bd194b2..cd4fb3ef0 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -2775,7 +2775,7 @@ mod tests { use crate::HostFunctions; use crate::hypervisor::hyperlight_vm::{CreateHyperlightVmError, HyperlightVmError}; use crate::hypervisor::regs::{ - APIC_BASE_X2APIC_ENABLE, MSR_APERF, MSR_BNDCFGS, MSR_CSTAR, MSR_DEBUGCTL, + MSR_APERF, MSR_BNDCFGS, MSR_CSTAR, MSR_DEBUGCTL, MSR_IA32_SSP, MSR_INTERRUPT_SSP_TABLE_ADDR, MSR_KERNEL_GS_BASE as KERNEL_GS_BASE, MSR_LSTAR, MSR_MPERF, MSR_MTRR_DEF_TYPE, MSR_MTRR_FIX64K_00000, MSR_PAT, MSR_PL0_SSP, MSR_PL1_SSP, MSR_PL2_SSP, MSR_PL3_SSP, MSR_S_CET, MSR_SFMASK, MSR_SPEC_CTRL, MSR_STAR, @@ -3133,6 +3133,7 @@ mod tests { #[test] #[cfg(kvm)] fn guest_cannot_enable_x2apic_through_apic_base() { + use crate::hypervisor::regs::APIC_BASE_X2APIC_ENABLE; use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; if !matches!(get_available_hypervisor(), Some(HypervisorType::Kvm)) { @@ -3609,6 +3610,9 @@ mod tests { fn reset_exception_reason(index: u32) -> Option<&'static str> { match index { MSR_TSC => Some("KVM denies direct guest TSC MSR access"), + MSR_IA32_SSP => Some( + "active SSP has no architectural RDMSR/WRMSR; covered by active_ssp_does_not_leak_across_restore", + ), MSR_DEBUGCTL => Some("DEBUGCTL support depends on exposed debug features"), MSR_SPEC_CTRL => Some("SPEC_CTRL writable bits depend on mitigation features"), MSR_U_CET @@ -3888,16 +3892,12 @@ mod tests { ); } - /// A host TSC write must reset the guest-visible MSHV counter. + /// Active SSP is guest-writable state the Hyper-V backends reset + /// across restore. Skips where the guest cannot use CET shadow + /// stacks, which includes every KVM host. #[test] - #[cfg(all(mshv3, target_arch = "x86_64"))] - fn mshv_host_tsc_writeback_resets_guest_tsc() { - use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; - - if !matches!(get_available_hypervisor(), Some(HypervisorType::Mshv)) { - return; - } - + #[cfg(all(any(mshv3, target_os = "windows"), target_arch = "x86_64"))] + fn active_ssp_does_not_leak_across_restore() { let mut sbox = UninitializedSandbox::new( GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), None, @@ -3906,29 +3906,24 @@ mod tests { .evolve() .unwrap(); - let base = sbox.vm.capture_msrs_for_test(&[MSR_TSC]).unwrap()[0].value; + if !sbox.call::("CetShadowStackSupported", ()).unwrap() { + return; + } - let jump = base.wrapping_add(1 << 60); - sbox.call::<()>("WriteMSR", (MSR_TSC, jump)).unwrap(); - let planted: u64 = sbox.call("ReadMSR", MSR_TSC).unwrap(); - assert!( - planted >= jump, - "guest TSC write did not take (planted=0x{planted:X} jump=0x{jump:X})" - ); + let baseline = sbox.snapshot().unwrap(); + let original: u64 = sbox.call("ReadActiveSsp", ()).unwrap(); - assert!( - sbox.vm.try_set_msr_for_test(MSR_TSC, base), - "host set of HV_X64_REGISTER_TSC failed" - ); + let mutated: u64 = sbox.call("IncrementActiveSsp", ()).unwrap(); + assert_ne!(mutated, original, "guest did not change active SSP"); + let seen: u64 = sbox.call("ReadActiveSsp", ()).unwrap(); + assert_eq!(seen, mutated, "guest did not observe its own SSP mutation"); - let after: u64 = sbox.call("ReadMSR", MSR_TSC).unwrap(); - eprintln!( - "mshv TSC writeback probe: base=0x{base:X} jump=0x{jump:X} planted=0x{planted:X} after=0x{after:X}" - ); - assert!( - after < jump / 2, - "host TSC write-back did NOT reset the guest TSC (after=0x{after:X} still near \ - jump=0x{jump:X}); the reset approach is not viable on this host" + sbox.restore(baseline).unwrap(); + + let after: u64 = sbox.call("ReadActiveSsp", ()).unwrap(); + assert_eq!( + after, original, + "active SSP leaked across restore (original=0x{original:X}, mutated=0x{mutated:X}, after=0x{after:X})" ); } } diff --git a/src/tests/rust_guests/simpleguest/src/main.rs b/src/tests/rust_guests/simpleguest/src/main.rs index 536e64cf0..677286d91 100644 --- a/src/tests/rust_guests/simpleguest/src/main.rs +++ b/src/tests/rust_guests/simpleguest/src/main.rs @@ -1112,6 +1112,128 @@ fn read_msr(msr: u32) -> u64 { ((read_edx as u64) << 32) | (read_eax as u64) } +#[guest_function("IncrementActiveSsp")] +#[cfg(target_arch = "x86_64")] +fn increment_active_ssp() -> u64 { + const MSR_IA32_S_CET: u32 = 0x6A2; + const CR4_CET: u64 = 1 << 23; + + let shadow_stack_gpa = unsafe { hyperlight_guest::prim_alloc::alloc_phys_pages(1) }; + let shadow_stack = hyperlight_guest_bin::paging::phys_to_virt(shadow_stack_gpa) + .expect("allocated shadow stack is outside scratch"); + let restore_token = (shadow_stack as u64 + 8) | 1; + unsafe { + core::ptr::write_volatile(shadow_stack.cast::(), restore_token); + hyperlight_guest_bin::paging::map_region( + shadow_stack_gpa, + shadow_stack, + hyperlight_common::vmem::PAGE_SIZE as u64, + MappingKind::Basic(BasicMapping { + readable: true, + writable: false, + executable: false, + }), + ); + core::arch::asm!("invlpg [{shadow_stack}]", shadow_stack = in(reg) shadow_stack); + } + + let original_s_cet = read_msr(MSR_IA32_S_CET); + let original_s_cet_low = original_s_cet as u32; + let original_s_cet_high = (original_s_cet >> 32) as u32; + let after: u64; + + // SAFETY: The test guest runs at CPL0. The restore token is on a read-only, dirty page, + // and CET is disabled before the function returns. + unsafe { + core::arch::asm!( + "mov {original_cr4}, cr4", + "mov {cet_cr4}, {original_cr4}", + "or {cet_cr4}, {cr4_cet}", + "mov cr4, {cet_cr4}", + "mov ecx, {s_cet}", + "mov eax, 1", + "xor edx, edx", + "wrmsr", + "rstorssp [{shadow_stack}]", + "incsspq {increment}", + "rdsspq {after}", + "mov ecx, {s_cet}", + "mov eax, {original_s_cet_low:e}", + "mov edx, {original_s_cet_high:e}", + "wrmsr", + "mov cr4, {original_cr4}", + original_cr4 = out(reg) _, + cet_cr4 = out(reg) _, + after = out(reg) after, + shadow_stack = in(reg) shadow_stack, + increment = in(reg) 1_u64, + original_s_cet_low = in(reg) original_s_cet_low, + original_s_cet_high = in(reg) original_s_cet_high, + s_cet = const MSR_IA32_S_CET, + cr4_cet = const CR4_CET, + out("eax") _, + out("ecx") _, + out("edx") _, + options(nostack) + ); + } + + after +} + +#[guest_function("CetShadowStackSupported")] +#[cfg(target_arch = "x86_64")] +fn cet_shadow_stack_supported() -> bool { + core::arch::x86_64::__cpuid(0).eax >= 7 + && core::arch::x86_64::__cpuid_count(7, 0).ecx & (1 << 7) != 0 +} + +#[guest_function("ReadActiveSsp")] +#[cfg(target_arch = "x86_64")] +fn read_active_ssp() -> u64 { + const MSR_IA32_S_CET: u32 = 0x6A2; + const CR4_CET: u64 = 1 << 23; + + let original_s_cet = read_msr(MSR_IA32_S_CET); + let original_s_cet_low = original_s_cet as u32; + let original_s_cet_high = (original_s_cet >> 32) as u32; + let ssp: u64; + + // SAFETY: The test guest runs at CPL0. RDSSPQ only reads the SSP register, + // and CET is disabled before the function returns. + unsafe { + core::arch::asm!( + "mov {original_cr4}, cr4", + "mov {cet_cr4}, {original_cr4}", + "or {cet_cr4}, {cr4_cet}", + "mov cr4, {cet_cr4}", + "mov ecx, {s_cet}", + "mov eax, 1", + "xor edx, edx", + "wrmsr", + "rdsspq {ssp}", + "mov ecx, {s_cet}", + "mov eax, {original_s_cet_low:e}", + "mov edx, {original_s_cet_high:e}", + "wrmsr", + "mov cr4, {original_cr4}", + original_cr4 = out(reg) _, + cet_cr4 = out(reg) _, + ssp = out(reg) ssp, + original_s_cet_low = in(reg) original_s_cet_low, + original_s_cet_high = in(reg) original_s_cet_high, + s_cet = const MSR_IA32_S_CET, + cr4_cet = const CR4_CET, + out("eax") _, + out("ecx") _, + out("edx") _, + options(nostack) + ); + } + + ssp +} + #[guest_function("NestedVirtualizationCpuid")] #[cfg(target_arch = "x86_64")] #[allow(unused_unsafe)] From 482e3a043e43f6b4fb523d02af2445461cfdaaf9 Mon Sep 17 00:00:00 2001 From: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:33:52 -0700 Subject: [PATCH 4/4] Hide CET from KVM guests KVM cannot reset active SSP across restore. Removing CET (CPUID leaf 7 SHSTK/IBT) from the guest stops it enabling shadow stacks, so active SSP never changes and the gap is unreachable. IA32_S_CET is then unreadable host-side and rejected from allow_msrs at creation. MSHV and WHP expose CET and reset active SSP instead. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> --- docs/msr.md | 4 ++ .../hypervisor/virtual_machine/kvm/x86_64.rs | 13 +++++++ .../src/sandbox/initialized_multi_use.rs | 39 +++++++++++++++++++ 3 files changed, 56 insertions(+) diff --git a/docs/msr.md b/docs/msr.md index 19f1c745a..8252206c6 100644 --- a/docs/msr.md +++ b/docs/msr.md @@ -110,6 +110,10 @@ paths separately: * VMX and SVM are removed from guest CPUID, so nested VMCS and VMCB state is unreachable. Their setup MSRs remain denied. +* CET is removed from guest CPUID, so the guest cannot enable shadow stacks and + cannot move active SSP. Active SSP has no architectural MSR, so it is absent + from the KVM reset set and the backend never restores it. The CET MSRs stay + denied and unallowable. * Hyperlight keeps the APIC in xAPIC mode. Guest writes to `APIC_BASE` and the x2APIC range `0x800..=0x8FF` are denied. Snapshot restore also rejects an `APIC_BASE` value with the x2APIC enable bit set. diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs index 96238f806..dbee6e450 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs @@ -69,9 +69,14 @@ use crate::sandbox::trace::TraceContext as SandboxTraceContext; const CPUID_FUNCTION_PROCESSOR_CAPACITY_PARAMETERS_AND_EXTENDED_FEATURE_IDENTIFICATION: u32 = 0x8000_0008; const CPUID_FUNCTION_FEATURE_INFORMATION: u32 = 0x1; +const CPUID_FUNCTION_STRUCTURED_EXTENDED_FEATURES: u32 = 0x7; const CPUID_FUNCTION_EXTENDED_FEATURE_INFORMATION: u32 = 0x8000_0001; const CPUID_FEATURE_VMX: u32 = 1 << 5; const CPUID_FEATURE_SVM: u32 = 1 << 2; +/// CPUID.(EAX=7,ECX=0):ECX[7], shadow-stack support. +const CPUID_FEATURE_SHSTK: u32 = 1 << 7; +/// CPUID.(EAX=7,ECX=0):EDX[20], indirect-branch-tracking support. +const CPUID_FEATURE_IBT: u32 = 1 << 20; /// Return `true` if the KVM API is available, version 12, and has UserMemory capability, or `false` otherwise #[instrument(skip_all, parent = Span::current(), level = "Trace")] @@ -200,6 +205,14 @@ impl KvmVm { CPUID_FUNCTION_EXTENDED_FEATURE_INFORMATION => { entry.ecx &= !CPUID_FEATURE_SVM; } + // Hyperlight does not expose CET. Shadow stacks let the guest + // move active SSP. It has no architectural MSR, so it is not in + // the KVM reset set, and the backend does not make the separate + // KVM register access needed to restore it. + CPUID_FUNCTION_STRUCTURED_EXTENDED_FEATURES if entry.index == 0 => { + entry.ecx &= !CPUID_FEATURE_SHSTK; + entry.edx &= !CPUID_FEATURE_IBT; + } // KVM allows MaxPhysAddr to be overridden and defaults it too low for // Hyperlight's memory layout. MSHV passes it through from hardware // unless an intercept is installed. diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index cd4fb3ef0..8ee924d32 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -3926,6 +3926,45 @@ mod tests { "active SSP leaked across restore (original=0x{original:X}, mutated=0x{mutated:X}, after=0x{after:X})" ); } + + /// Hyperlight hides CET from KVM guests, so shadow stacks cannot be + /// enabled and active SSP cannot be moved. Active SSP has no + /// architectural MSR, so it is absent from the KVM reset set and the + /// backend never restores it. Hiding CET keeps that gap unreachable. + #[test] + #[cfg(all(kvm, target_arch = "x86_64"))] + fn kvm_does_not_expose_cet_to_guest() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Kvm)) { + return; + } + + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + assert!( + !sbox.call::("CetShadowStackSupported", ()).unwrap(), + "KVM guest CPUID exposes CET shadow stacks" + ); + + // With CET hidden the host cannot read or write IA32_S_CET, so + // allowing it is rejected at VM creation. + let mut cfg = SandboxConfiguration::default(); + cfg.allow_msrs(&[MSR_S_CET]).unwrap(); + let err = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(cfg), + ) + .unwrap() + .evolve() + .expect_err("allowing IA32_S_CET must be rejected when CET is hidden"); + assert_msr_not_allowable(&err, MSR_S_CET); + } } /// Tests for [`MultiUseSandbox::from_snapshot`] in-memory.