diff --git a/phase1-kvm-handoff.md b/phase1-kvm-handoff.md new file mode 100644 index 000000000..1813ee72c --- /dev/null +++ b/phase1-kvm-handoff.md @@ -0,0 +1,172 @@ +# Phase 1 handoff: MSR reset on KVM + +This brief is for an agent working on a Linux/KVM machine. It implements Phase 1 +of `plan.md` (KVM only). Read `plan.md` first, then this. This brief pins the +starting point, the exact files, the task order, and the done criteria. Every +factual claim here was verified against the source or the pinned crates. + +## Scope + +Phase 1 only. KVM backend. Split reset from access control: +* Add MSR reset (capture in snapshot, write back in `reset_vcpu`). +* Replace the `allow_msr` bool with a validated per-MSR allow list. +* Keep the KVM default-deny filter, add allow ranges so allowed MSRs pass through. +* Tests. + +Out of scope for Phase 1: mshv, WHP, the `0a` CPUID masking work, LBR/MTRR +decisions beyond CPUID gating, persistence schema changes. Do not start those. + +## Why this change (one paragraph) + +`reset2` denies MSR access but never resets MSR state, and its `allow_msr` bool is +an all-or-nothing `unsafe` switch that disables the filter entirely and leaks +state across `restore()`. Reset (save in snapshot, restore on `restore()`) is the +isolation guarantee. Access control (deny-by-default plus a per-MSR allow list) +bounds what the guest can write so the reset set is complete by construction. See +`plan.md` §2 for the full rationale. + +## Starting point + +* Branch: detached at `origin/reset2` @ `68ad5265` "Disallow guest from + read/write MSRs by default". Create a working branch from it. +* Environment check before anything else. Confirm `/dev/kvm` exists and is + read/write for the current user. If a sandbox fails with "No Hypervisor was + found", print `ls -l /dev/kvm` and group membership and stop. + +## What `reset2` already ships (verified anchors) + +All KVM-only, under `src/hyperlight_host/src/`. + +* KVM deny-all filter: `enable_msr_filter()` in + `hypervisor/virtual_machine/kvm/x86_64.rs` (~line 357). Sets + `KVM_MSR_FILTER_DEFAULT_DENY` with one dummy all-zero-bit range plus + `enable_cap(X86UserSpaceMsr, Filter)`. +* Filter wired at VM creation: `hypervisor/hyperlight_vm/x86_64.rs` (~line 99), + calls `enable_msr_filter()` unless `config.get_allow_msr()`. +* Filter exits: `kvm/x86_64.rs` handles `X86Rdmsr`/`X86Wrmsr` (~lines 236-252 and + 313-333) with the `error=1` plus `immediate_exit` handshake, injecting `#GP`, + returning `VmExit::MsrRead`/`MsrWrite`. +* `VmExit` variants: `hypervisor/virtual_machine/mod.rs` (~lines 144, 147). +* Violation mapping: `hypervisor/hyperlight_vm/mod.rs` maps `MsrRead`/`MsrWrite` + (~lines 752-757) to `RunVmError::MsrReadViolation`/`MsrWriteViolation` + (~lines 228, 231), then to the matching `HyperlightError` (~lines 167-173). +* Config: `sandbox/config.rs` has `allow_msr: bool` (~lines 79, 125) and + `unsafe fn set_allow_msr(bool)` / `get_allow_msr()` (~lines 177, 184). +* Config use: `sandbox/initialized_multi_use.rs` calls `set_allow_msr(true)` + (~line 2777). Update this call site when the API changes. +* Reset today: `reset_vcpu` in `hypervisor/hyperlight_vm/x86_64.rs` (~line 348) + resets only the `sregs` MSRs. `apply_sregs` (~lines 262-269) calls `set_sregs`. + `get_snapshot_sregs` (~line 274). +* Snapshot: `sandbox/snapshot/mod.rs` `Snapshot` struct (~line 70). `snapshot()` + in `sandbox/initialized_multi_use.rs` (~line 356). +* Registers: `hypervisor/regs/x86_64/special_regs.rs`. `CommonSegmentRegister` + carries `base` (~line 379), so `fs.base`/`gs.base` restore through `set_sregs`. + +## Verified facts that shape the code + +* `FS_BASE`/`GS_BASE` reset through `sregs` on every restore. Do NOT put them in + `CommonMsrs`. Only `KERNEL_GS_BASE` needs MSR reset. +* Guest CPUID is host pass-through on KVM. `kvm/x86_64.rs` (~lines 155-165) starts + from `get_supported_cpuid()` and modifies only leaf `0x8000_0008`. So the guest + sees host MTRR/CET/FRED/LBR/PMU. `KVM_GET_MSR_INDEX_LIST` tracks this, so Phase 1 + needs no CPUID masking. +* In-kernel LAPIC exists under the `hw-interrupts` feature (`create_irq_chip`, + `kvm/x86_64.rs` ~line 142). It is xAPIC, not x2APIC. LAPIC reset (§7) is part of + Phase 3, but keep the ordering hook in mind. +* No TSC setup anywhere. Bare TSC is monotonic across VP reuse. Reset `TSC_ADJUST` + to baseline, never write raw `TSC`. Mark `TSC`/`TSC_AUX` `host_specific`. +* `kvm-ioctls` `get_msrs`/`set_msrs` are batched. `KVM_SET_MSRS` returns a count + and stops at the first bad index. A short count means fail closed and poison. + +## API decision to make first + +Replace `unsafe fn set_allow_msr(bool)` on `SandboxConfiguration` with a safe, +fallible allow list. Suggested shape, adjust to house style: + +```rust +// Add each MSR the guest may read and write. Validated at init (see plan §10). +pub fn allow_msr(&mut self, index: u32) -> &mut Self; +pub fn allow_msrs(&mut self, indices: &[u32]) -> &mut Self; +``` + +Default allow list is empty (`plan.md` §0). Drop the `unsafe` bypass. Allowing an +MSR must never disable reset. Un-gate the field from `#[cfg(kvm)]` so later phases +reuse it (the KVM filter wiring stays `#[cfg(kvm)]`). + +## Task order (small, focused commits) + +1. Data model. Add `CommonMsrs = Vec` with `MsrEntry { index, value, + class, flags }` and the `MsrClass`/`MsrFlags` enums (`plan.md` §5). Put it near + `regs/x86_64/`. `host_specific` marks `TSC`/`TSC_AUX`. +2. Static table plus derivation (§3, §4). Author the per-index table (class and + gating CPUID). Derive the resolved reset set from `KVM_GET_MSR_INDEX_LIST` minus + the `HostOwned` class, expanded with the table. Add MTRRs from CPUID. An + enumerated writable index the table cannot classify fails closed (probe get/set, + add if stateful, else refuse VM creation). Cache per process with `LazyLock`. +3. Backend get/set. KVM `get_msrs`/`set_msrs` over `CommonMsrs`. Short count + poisons and fails closed. +4. Snapshot wiring (§6). Add `Snapshot::msrs: Option`. Capture in + `snapshot()` after `get_snapshot_sregs()`, `Stateful` entries only. In + `reset_vcpu`: `apply_sregs` (EFER first), then batched `SET_MSRS` of the + `Stateful` set (no `host_specific` entry in the batch), then `TSC_ADJUST`. + Init baseline for `msrs == None`. +5. Access control (§9). Replace the `allow_msr` bool with the allow list. Build + `KVM_X86_SET_MSR_FILTER` DEFAULT_DENY plus allow ranges for opted-in MSRs. + Allowed MSRs pass through, denied MSRs keep the existing `#GP`/poison path. +6. Validation (§10). At init, for each allowed MSR: confirm host has it (index + list), confirm it is `Stateful` and settable back to baseline, else hard error. + Add it to the reset set and the allow ranges. +7. Tests (below). + +## Tests (Phase 1 subset of §12) + +* Fixed-list differential. For each reset-set MSR, guest writes a sentinel, + `restore()`, host reads back, assert baseline. +* KVM index-list sweep. Sweep `KVM_GET_MSR_INDEX_LIST`, write sentinel, restore, + assert baseline. +* Opt-out. Allow `KERNEL_GS_BASE`. Assert the guest reads and writes it, assert + `restore()` returns it to baseline, assert a non-resettable allow request errors. +* Fail closed. Default-deny installs. Non-get/settable allow errors. Simulated + partial `SET_MSRS` poisons. An unclassifiable enumerated writable MSR fails at + VM creation, not silently dropped. +* Negative leak. Write a sentinel, `restore()`, assert it is gone. + +## Build and test loop + +```bash +just guests # REQUIRED before tests, builds the guest binaries +just test # debug +just test release # release +``` + +Before pushing: + +```bash +just fmt-apply +just clippy debug +just clippy release +just test-like-ci +just test-like-ci release +``` + +Commits must be signed and DCO signed off: `git commit -S --signoff`. Keep commits +small and logically ordered. Rebase on `main`, no merge commits. Label the PR per +`docs/github-labels.md`. + +## Done criteria + +* A guest MSR write to any reset-set MSR does not survive `restore()`. +* An allowed MSR is readable and writable by the guest and still resets. +* A non-resettable allow request is a hard init error, no `unsafe` bypass remains. +* The default-deny filter installs, denied MSRs `#GP` and poison. +* `just test-like-ci` and `just test-like-ci release` pass on Intel and, if + available, AMD. + +## Pitfalls + +* Do not write raw `TSC`. It rewinds guest time. Reset `TSC_ADJUST` only. +* Do not add `FS_BASE`/`GS_BASE` to `CommonMsrs`. They reset via `sregs`. +* Do not defer an unclassified writable MSR to a nightly sweep. Fail closed. +* Do not reintroduce an `unsafe` allow-all. Allowing must never disable reset. +* `KVM_MSR_FILTER_DEFAULT_DENY` needs at least one range. Keep the dummy all-zero + range and add real allow ranges alongside it. diff --git a/phase2-mshv-handoff.md b/phase2-mshv-handoff.md new file mode 100644 index 000000000..5759b568f --- /dev/null +++ b/phase2-mshv-handoff.md @@ -0,0 +1,244 @@ +# Phase 2 handoff: MSR reset on mshv + +This brief is for an agent working on a Windows/mshv (Hyper-V, Linux dom0) machine +that can build and run the mshv backend. It implements Phase 2 of `plan.md`: bring +MSR reset to mshv. Read `plan.md` (especially §0a, §4, §5, §7, §9, §13) and +`phase1-kvm-handoff.md` first. Phase 1 (KVM) is the reference implementation, +already merged on this branch. Every factual claim here was verified against the +pinned crates or the source. + +## Scope + +Phase 2, mshv backend only: +* Widen the MSR reset machinery from KVM-only to cover mshv. +* Implement `capture_msrs`/`apply_msrs` on the mshv VM. +* Resolve the completeness gap (mshv has no per-MSR deny filter, unlike KVM). +* Handle the `msr_to_hv_reg_name` mapping gaps. +* Tests on real Hyper-V hardware. + +Out of scope: WHP, the full §0a CPUID masking as a finished feature (scope it, and +do the minimum masking the completeness argument needs, see below), persistence +schema changes beyond what Phase 1 already did. + +## The crux: mshv has no per-MSR deny filter + +This is the central difference from KVM and the thing to get right. + +On KVM, `KVM_X86_SET_MSR_FILTER` denies every non-allowed MSR, so the reset set +`core ∪ allow` is complete: the guest can only write what it is allowed, and every +allowed MSR is in the reset set. + +On mshv there is no equivalent per-MSR filter. `install_intercept` is coarse, and +the underlying Hyper-V hypervisor **passes through a set of MSRs to the guest +without any intercept** (verified in the Hyper-V source: the SVM/VMX pass-through +set, ~31 MSRs including `KERNEL_GS_BASE`, `SPEC_CTRL`, CET, FRED, and others). A +guest can write those directly, and the coarse intercept never sees them. So on +mshv the `core ∪ allow` completeness argument does **not** hold as written. + +The approach for Phase 2, combining reset and masking. Split the pass-through set +into "resettable" and "not resettable" and handle each: + +* **Resettable pass-through MSRs: add them to the mshv reset set.** These are the + stateful ones `msr_to_hv_reg_name()` can map and the guest legitimately uses: + `KERNEL_GS_BASE`, `SPEC_CTRL`, and CET (`U_CET`/`S_CET`/`*_SSP`/`INT_SSP_TAB`) + when CET CPUID is exposed. Capture and restore them like any core MSR, so even + though the intercept does not deny the guest writing them, reset rolls them back. + (`FS_BASE`/`GS_BASE` are pass-through too but already reset via `sregs`, so they + need nothing here.) +* **Non-resettable classes: mask them in guest CPUID** (the §0a work). These are the + classes whose MSRs you cannot reliably capture/restore: LBR, FRED, UMWAIT, and + PMU. Clear their CPUID bits so the guest cannot use the feature and cannot reach + those MSRs. `PRED_CMD`/`FLUSH_CMD` are write-only commands with no state, so they + need neither reset nor masking. + +So: reset the stateful pass-through MSRs, mask the classes you cannot reset. After +this, every MSR a guest can actually write on mshv is either in the reset set or +belongs to a masked (unusable) class. Write the mshv equivalent of Phase 1's "Why +the reset set is complete" comment, listing explicitly which pass-through MSRs are +reset and which classes are masked, so the completeness argument is auditable. + +## From the Hyper-V + mshv source (you cannot see these) + +These come from the Microsoft Hyper-V hypervisor source and the Linux mshv driver, +which are not on your machine. They are the load-bearing facts for the completeness +gap above. Treat them as authoritative but re-confirm behavior with the tests, since +the exact set is CPU-vendor and feature gated. + +**`install_intercept` does not catch pass-through MSRs.** The hypervisor keeps a +per-VP MSR bitmap that defaults to intercept, then *clears* (passes through) a fixed +set of architectural MSRs. A pass-through MSR generates no VMEXIT, so +`HV_INTERCEPT_TYPE_X64_MSR` (what `install_intercept` installs) never fires for it. +The catch-all intercept reflects only MSRs the hypervisor treats as unimplemented or +access-denied; architectural MSRs it emulates or passes through never reach your +intercept handler. This is exactly why the KVM deny-by-default completeness argument +does not carry to mshv. The mshv driver (`drivers/hv/mshv_root_main.c`) exposes no +per-MSR filter ioctl either, only register get/set (`MSHV_GET/SET_VP_REGISTERS`) and +`install_intercept`. + +**The pass-through set (AMD SVM, from `valsvmp.h` `SVMP_PASSTHROUGH_MSR`).** For a +normal, non-SNP guest (Hyperlight is not SNP) the guest can write these directly with +no intercept: + +* `FS_BASE`, `GS_BASE` — reset via `sregs`, no action. +* `KERNEL_GS_BASE`, `SPEC_CTRL` — stateful and mappable. **Reset these.** +* `U_CET`, `PL0_SSP`, `PL1_SSP`, `PL2_SSP`, `PL3_SSP` (CET) — mappable. **Reset when + CET CPUID is exposed, otherwise mask CET.** +* `PRED_CMD` — write-only command, no state. Skip. +* `XFD`, `XFD_ERR` (AMX) — not mappable by `msr_to_hv_reg_name`. **Mask AMX/XFD in + CPUID.** +* `PERF_CTR*` (PMCs, `Ctr0..Ctr3`, `Ctr0_2..Ctr5_2`) — need vPMU. **Mask PMU.** +* FRED MSRs (`FRED_CONFIG`, `FRED_RSP1..3`, `FRED_STKLVLS`, `FRED_SSP1..3`) — not + mappable. **Mask FRED.** + +So the "reset the stateful ones, mask the rest" plan above maps one-to-one onto this +list: reset `KERNEL_GS_BASE`, `SPEC_CTRL`, CET; mask AMX, PMU, FRED. + +**Non-SNP caveat.** `STAR`, `LSTAR`, `CSTAR`, `SFMASK`, sysenter, `PAT`, `EFER`, +`S_CET`, `XSS`, `TSC_AUX` are pass-through **only under SNP**. On a normal non-SNP +guest the hypervisor emulates them, so they are not in the always-writable set. +Keeping them in the reset set (as Phase 1 does) is harmless defense-in-depth. + +**Intel (VMX) differs.** The VMX MSR bitmap is built dynamically +(`VmxpSetMsrPassthrough`), and the perfmon/Intel-PT (RTIT) MSRs are pass-through when +those features are on. The architectural core (FS/GS base, `KERNEL_GS_BASE`, +`SPEC_CTRL`, CET) matches SVM. Do not hard-code an Intel list; gate on the CPUID you +expose and mask PMU, Intel PT, AMX, and FRED. The completeness test (guest write to a +reset-set pass-through MSR does not survive restore) is what actually validates the +set on the running host, so lean on it rather than on a hard-coded table. + +## Verified crate and source facts + +* `mshv-ioctls` 0.6.9 `VcpuFd::get_msrs`/`set_msrs` exist and batch: each index is + translated by `msr_to_hv_reg_name()` and applied in one `set_reg` + (`HVCALL_SET_VP_REGISTERS` rep hypercall). Use these, mirroring KVM. +* `set_msrs` returns `Ok(0)`, not a written count. Do not rely on a count. Detect + failure from `Err`. +* `set_reg` partial apply: `hvcall_set_reg` issues the rep hypercall then checks + `args.reps != len`, returning `EINTR` on a short count (mshv-ioctls 0.6.9 + `vcpu.rs`). An unmappable index fails in the pre-loop (nothing applied); a + hypervisor-rejected register mid-rep leaves earlier registers committed, then + `EINTR`. So it is **not atomic**. Rely on poison-on-`Err`, not atomicity. +* `msr_to_hv_reg_name()` (mshv-bindings 0.6.9 `src/x86_64/regs.rs`) maps: syscall + (`STAR`/`LSTAR`/`CSTAR`/`SFMASK`), sysenter, `KERNEL_GS_BASE`, `PAT`, `DEBUG_CTL`, + `SPEC_CTRL`, `MISC_ENABLE`, `BNDCFGS`, `TSC`/`TSC_ADJUST`/`TSC_AUX`, CET + (`U_CET`/`S_CET`/`*_SSP`/`INT_SSP_TAB`), `U_XSS`, the fixed MTRR set, and the + Hyper-V synthetic MSRs. It does **not** map `FS_BASE`, `GS_BASE`, + `UMWAIT_CONTROL 0xE1`, FRED, or the LBR stack. An unmapped index returns `EINVAL`. +* `FS_BASE`/`GS_BASE` reset via `sregs` on mshv too. Keep them out of `CommonMsrs`. + Phase 1 already excludes them from the core table. +* mshv creates an in-kernel xAPIC LAPIC (`mshv/x86_64.rs`, `MSHV_PT_BIT_LAPIC` plus + `set_lapic`, ~line 147), gated by `hw-interrupts`. x2APIC is not exposed. LAPIC + reset is Phase 3, not Phase 2. +* mshv sets no guest CPUID today: the guest sees the hypervisor default. Establishing + the §0a contract (masking) is new work and is what the completeness argument needs. + +## What Phase 1 built (your starting point) + +All under `src/hyperlight_host/src/`. These are currently gated +`#[cfg(all(kvm, target_arch = "x86_64"))]`. Phase 2's first job is to widen those +gates to include mshv. + +* Data model: `hypervisor/regs/x86_64/msrs.rs` (`MsrEntry`, `MsrClass`, `MsrFlags`, + `CommonMsrs`, `MsrResetState`, the static `MSR_TABLE`, `classify_msr`, + `core_reset_indices`). Backend-agnostic already, only the re-exports are gated. +* Trait: `hypervisor/virtual_machine/mod.rs` has `capture_msrs`/`apply_msrs` on the + `VirtualMachine` trait with default `Err(MsrsUnsupported)`, gated `all(kvm, ...)`. + Widen the gate and implement both on `MshvVm`. +* KVM impl: `hypervisor/virtual_machine/kvm/x86_64.rs` `capture_msrs`/`apply_msrs` + are the reference. Mirror them with `get_msrs`/`set_msrs` on mshv. +* Wiring: `hypervisor/hyperlight_vm/x86_64.rs` captures the baseline at init + (`msr_reset: Option`), `get_snapshot_msrs`/`restore_msrs`. Currently + gated to KVM and re-checks `get_available_hypervisor()`. Widen to also run on mshv. +* Snapshot: `sandbox/snapshot/mod.rs` `Snapshot::msrs`, `msrs()`, `set_msrs()`. + Widen the gate. +* Config: `sandbox/config.rs` `allowed_msrs`, `allow_msr`/`allow_msrs`, + `get_allowed_msrs`, `MAX_ALLOWED_MSRS`. Widen from KVM-only so mshv honors the + allow list. +* Errors: `hypervisor/virtual_machine/mod.rs` `RegisterError::{MsrBuild, GetMsrs, + SetMsrs, MsrShortCount, MsrsUnsupported}` and `CreateVmError::{MsrNotAllowable, + TooManyMsrRanges}`. Reuse; add mshv-specific variants only if needed. + +## Task order (small, focused commits) + +1. Widen cfg gates. Change `#[cfg(all(kvm, target_arch = "x86_64"))]` to a gate that + also covers mshv (e.g. `#[cfg(all(any(kvm, mshv3), target_arch = "x86_64"))]`) + across the trait, `Snapshot`, config, `hyperlight_vm` wiring, and error variants. + Build KVM to confirm no regression, then build mshv. +2. Implement `capture_msrs`/`apply_msrs` on `MshvVm` + (`hypervisor/virtual_machine/mshv/x86_64.rs`) using `get_msrs`/`set_msrs`. Detect + failure from `Err` (no count). Poison on `Err`, including the `EINTR` partial + apply. Mirror the KVM classification-on-capture logic. +3. Resolve mapping gaps (§5). For every reset-set index, either it maps through + `msr_to_hv_reg_name()` or it has a documented path: `FS_BASE`/`GS_BASE` via + `sregs` (already excluded), and `UMWAIT_CONTROL`/FRED/LBR masked via CPUID (§0a) + so they cannot be reached. A reset-set index that neither maps nor has a path is + a backend-selection failure. +4. Close the completeness gap. Add the stateful pass-through MSRs + (`KERNEL_GS_BASE`, `SPEC_CTRL`, CET when exposed) to the mshv reset set, mask the + classes you cannot reset (step 5), and write the completeness comment (mshv + equivalent of Phase 1's), listing which pass-through MSRs are reset and which + classes are masked. Install `install_intercept` as the fail-safe detector, + injecting `#GP` and poisoning on an unexpected interceptable access. +5. Minimal §0a masking. Mask the CPUID bits for the classes the reset set does not + cover (LBR, FRED, UMWAIT, PMU) so the completeness argument holds. This is the + mshv precondition, keep it minimal and testable. +6. Tests (below). + +## Tests (mshv, on Hyper-V hardware) + +Mirror the Phase 1 tests, adapted: +* Fixed-list differential. Guest writes a sentinel to each reset-set MSR, restore, + assert baseline. +* Host sweep. Sweep the mshv index list (or the reset set), plant sentinel via the + host-side helper, apply baseline, assert restored. +* Opt-out. Allow `KERNEL_GS_BASE`, assert read/write works and resets across + restore. +* Negative leak. Sentinel to an allowed MSR, restore baseline, assert gone. +* Partial apply. Force a mid-rep `EINTR` (a non-canonical value KVM/Hyper-V rejects) + and assert the sandbox poisons and refuses further calls. The KVM short-count test + does not cover the mshv `EINTR` path. +* Masked-class fault. Assert a guest `WRMSR` to a masked class (LBR via `DEBUGCTL`, + FRED, UMWAIT) faults after masking. +* Pass-through coverage. Assert a guest write to a pass-through MSR that is in the + reset set (e.g. `KERNEL_GS_BASE`, `SPEC_CTRL`) does not survive a restore. This is + the mshv-specific completeness test, since the intercept does not deny it. + +## Build and test loop + +```bash +just guests +just fmt-apply +just clippy debug +just clippy release +just test-like-ci +just test-like-ci release +``` + +Diagnostics: if a sandbox fails with "No Hypervisor was found", check for `/dev/mshv` +and read/write access and print the result. Commits signed and DCO signed off: +`git commit -S --signoff`. Keep commits small and logically ordered. Label the PR +per `docs/github-labels.md`. + +## Done criteria + +* A guest MSR write to any reset-set MSR (including the stateful pass-through MSRs + the intercept does not deny) does not survive `restore()` on mshv. +* An allowed MSR is guest-accessible on mshv and still resets. +* A masked class faults; a reset-set index with no mapping and no documented path is + a hard backend-selection error. +* A partial `set_msrs` (`EINTR`) poisons the sandbox and it refuses further calls. +* The completeness comment documents which pass-through MSRs are reset and which + classes are masked. +* `just test-like-ci` and `just test-like-ci release` pass on Hyper-V, Intel and AMD. + +## Pitfalls + +* Do not assume the KVM deny-by-default completeness argument carries over. It does + not. mshv has no per-MSR filter, so the reset set must cover the pass-through MSRs + or the classes must be masked. +* Do not rely on `set_msrs` returning a count. It returns `Ok(0)`. Detect `Err`. +* Do not treat mshv `set_msrs` as atomic. A mid-rep rejection partially applies then + `EINTR`. Rely on poison-on-`Err`. +* Do not add `FS_BASE`/`GS_BASE` to `CommonMsrs`. They reset via `sregs`. +* Do not write raw `TSC`. Mark it `host_specific`. Reset `TSC_ADJUST` only, and only + if it is in the reset set. diff --git a/phase3-whp-handoff.md b/phase3-whp-handoff.md new file mode 100644 index 000000000..5c9fbd7e9 --- /dev/null +++ b/phase3-whp-handoff.md @@ -0,0 +1,242 @@ +# Phase 3 handoff: MSR reset on WHP (Windows Hypervisor Platform) + +This brief is for an agent continuing the MSR-reset work on the WHP (Windows +Hypervisor Platform) backend. It implements the WHP portion of `plan.md`. Read +`plan.md`, `phase1-kvm-handoff.md`, and `phase2-mshv-handoff.md` first — KVM is the +reference implementation, mshv is the closest analogue (WHP and mshv both run on the +Hyper-V hypervisor and both lack a usable per-MSR deny filter). Every factual claim +here was verified on a real WHP host (latest Windows 11) and/or against the +Hyper-V/WHP source tree. + +The work in this phase is **implemented, tested green on real WHP hardware, and left +uncommitted** on the working branch. This document is both a record of what was done +and a to-do list for what remains (chiefly: Linux validation of the shared-table +change on KVM/mshv, and committing). + +## Scope + +WHP backend, plus one shared-table change that also benefits mshv: +* Implement `capture_msrs`/`apply_msrs` on `WhpVm` (WHP models MSRs as *named + registers*, not an index list). +* Resolve the WHP reset set and wire snapshot/restore. +* Validate the allow list at sandbox creation (`validate_allowed_msrs`). +* **Determine whether WHP can do a per-MSR deny filter** (it cannot — see the crux). +* **Audit every WHP-exposable MSR for cross-restore leaks** and close the ones found + (MTRRs, `TSC_ADJUST`, `TSC_AUX`). +* Gate the guest `ReadMSR`/`WriteMSR` functions and remove redundant `cfg` gates. + +Out of scope: any change to the KVM deny filter; CPUID masking (WHP's minimal +partition already faults the unmodeled classes — see below). + +## Status: what is done and passing + +On a real WHP host, these tests are green (`cargo test -p hyperlight-host --lib +--profile=dev test_whp_`): +* `test_whp_msr_reset_across_restore` — core reset MSR (`KERNEL_GS_BASE`) rolls back. +* `test_whp_allowed_msr_resets_across_restore` — allow-list path + named-register + capture/apply. +* `test_whp_allow_non_resettable_msr_fails_creation` — allowing a write-only command + MSR (`PRED_CMD`) is a hard error at `evolve()`. +* `test_whp_mtrr_and_tsc_msrs_reset_across_restore` — the leak fix: MTRR def-type, + variable base/mask pair 0, fixed range, `TSC_ADJUST`, `TSC_AUX` all roll back. +* `test_whp_unresettable_msr_classes_do_not_retain` — the classes the completeness + argument relies on (PMU, AMX, FRED, LBR, and masked `DEBUGCTL`) never *retain* a + guest write (each write faults or reads back unchanged). Guards the non-exposure + assumption: if a class starts retaining, this fails. +* `test_whp_no_msr_leaks_across_restore_broad_sweep` — a broad sweep over ~60 + architectural MSRs (reset set + every touched class). For each, whatever the host + does (fault / mask / retain-and-reset), no guest-written value survives a restore. + This is the closest feasible thing to "no MSR can leak" on a filterless backend. + +Also verified: host builds clean, guests build clean (debug + release), clippy +`--all-features` clean, `cargo fmt --check` clean. + +A matching `test_mshv_no_msr_leaks_across_restore_broad_sweep` (same sweep, gated +`mshv3`) was added for the Linux agent to run — see "What remains". mshv already has +the class-fault invariant (`test_mshv_unresettable_msr_classes_fault`). + +## The crux #1: WHP cannot deny access to an *implemented* MSR + +This is the load-bearing finding. The plan's completeness argument is "deny-by- +default access control bounds what the guest can write, so the reset set is complete +by construction." On KVM that holds (`KVM_X86_SET_MSR_FILTER`). **On WHP it cannot be +made to hold**, and the reason is a deliberate Hyper-V hypervisor constraint, not a +Windows version or an API-usage mistake. We chased this all the way through the +WHP/Hyper-V source (`d:\os.2020`, not on a normal machine): + +WHP exposes three MSR-exit mechanisms. On the tested host: +* `WHvPartitionPropertyCodeUnimplementedMsrAction = WHvMsrActionExit` — accepted by + `WHvSetPartitionProperty` but **rejected by `WHvSetupPartition` with `0x80070057` + (E_INVALIDARG)**. +* `WHvPartitionPropertyCodeMsrActionList` (per-MSR `Exit`) — accepted by + `WHvSetPartitionProperty` but **rejected by `WHvSetupPartition` with `0xC0350006` + (`HV_STATUS_ACCESS_DENIED`)**, regardless of whether `ExtendedVmExits.X64MsrExit` + is also set. +* `WHvPartitionPropertyCodeX64MsrExitBitmap` with the `UnhandledMsrs` bit — **works** + (setup succeeds), **but only if `ExtendedVmExits.X64MsrExit` (bit 1) is also set** + (the bitmap is silently ignored without the toggle; this is confirmed in + `Amd64Partition.cpp::ConfigureMsrIntercepts`). It only traps MSRs the hypervisor + treats as *unimplemented* — which already `#GP` without it — so it adds no + isolation. + +The root cause of the per-MSR `ACCESS_DENIED` is in the hypervisor core +(`hvx/im/common/amd64/ImMsr.c`, `ImInstallMsrIndexIntercept`): + +```c +// Ensure that the MSR is unimplemented for the intercepts requested. +if (((AccessMask & HV_INTERCEPT_ACCESS_MASK_READ) && descriptor.ChildReadAccess != MsrAccessUnimplemented) || + ((AccessMask & HV_INTERCEPT_ACCESS_MASK_WRITE) && descriptor.ChildWriteAccess != MsrAccessUnimplemented)) +{ + return HV_STATUS_ACCESS_DENIED; +} +``` + +**The hypervisor refuses to install an MSR intercept on any MSR it implements.** So +you cannot trap a guest write to an implemented MSR like `DEBUGCTL` — both the +per-MSR list and the `UnhandledMsrs` bitmap are structurally incapable of it. This is +shared by all WHP/Hyper-V partitions; it is not fixable from the WHP API. + +**Consequence for the code:** there is no deny filter on WHP. `WhpVm::new()` does +*not* set any MSR-exit property (a `NOTE` comment there records all of the above). +The `WHvRunVpExitReasonX64MsrAccess` arm in `run_vcpu` is kept, dormant and correct, +in case a future WHP exposes an implemented-MSR intercept. Because there is no filter, +**the WHP reset set must cover every guest-writable, retained MSR by construction** — +exactly the mshv situation. That is what the leak audit (crux #2) is about. + +## The crux #2: audit found real cross-restore leaks; they are now fixed + +Since WHP cannot deny, a guest can freely write any MSR the hypervisor implements, +and its value survives a restore unless we reset it. We empirically swept every +WHP-exposable MSR on the running host (write sentinel → read back → restore → read +back) and classified each: + +* **Faults (`#GP`) — safe, nothing to do:** LBR (`LBR_SELECT`, `LER_*`), PMU + (`PERF_GLOBAL_CTRL`, `PERFEVTSEL0`), AMX (`XFD`). The minimal partition does not + expose these, so a guest `RDMSR`/`WRMSR` faults and no state persists. (Same + non-exposure argument as mshv.) +* **Modeled but masked — safe:** `DEBUGCTL` (0x1D9) and `MISC_ENABLE` (0x1A0). The + write returns `Ok(())` but the hypervisor validates and drops the unsupported bits, + so it reads back unchanged. No retained state, so nothing to reset. (This is why + `DEBUGCTL`/LBR is *not* a leak on WHP even though it is un-interceptable.) +* **Retained and NOT reset — REAL LEAKS (now fixed):** `TSC_ADJUST` (0x3B), `TSC_AUX` + (0xC0000103), and the entire MTRR family (`MTRR_DEF_TYPE` 0x2FF, variable + `PHYSBASE`/`PHYSMASK` pairs 0x200..0x213, fixed ranges 0x250/0x258/0x259/0x268.. + 0x26F). These are hypervisor-implemented, guest-writable with retention, exposed as + WHP named registers — and were missing from the reset set, so guest writes survived + a restore. +* **Correctly reset (proves the mechanism):** `PAT` (0x277) rolled back to baseline + because it is already in the core reset set. +* **Deliberately not reset:** raw `TSC` (0x10). Writing a stale captured value back + would move the guest clock backwards. Instead, `TSC_ADJUST` is reset — and the CPU + updates `TSC_ADJUST` on a direct `WRMSR(TSC)`, so resetting `TSC_ADJUST` + neutralizes a direct `TSC` write too. **Verified empirically:** guest wrote + `TSC = 0x100000000000`, and after restore `TSC` was back to a normal ~few-million + value, not the guest's value. + +The fix adds those MSRs to the shared reset table so both WHP and mshv (both +filterless) reset them. See the files list for exactly where. + +## The design decision: shared table vs. WHP-only + +The leaking MSRs went into the **shared** `MSR_TABLE` / `core_reset_indices()` +(`regs/x86_64/msrs.rs`), not a WHP-only list, because the leak exists on *any* +backend without a deny filter — and mshv is filterless too (see +`phase2-mshv-handoff.md`). This is safe across backends because the per-backend reset +resolution pre-filters: +* **KVM** filters `core_reset_indices()` by `host_msr_indices()` + (`KVM_GET_MSR_INDEX_LIST`), then captures. Guest writes to these are already denied + by the KVM filter, so capture+restore is a harmless no-op. The KVM deny-filter allow + list is built from the *user allow list only* (`configure_msr_access(allowed)`), + **not** from `core_reset_indices()`, so widening the core table does **not** widen + what the guest may write on KVM. +* **mshv / WHP** filter by `host_supports_msr()` (a successful register-name mapping) + **and** a `capture_msrs` probe, so an index the host does not model is silently + skipped rather than failing init. + +Net: on KVM the change is a no-op; on mshv and WHP it closes the leak. An MSR that a +given host cannot get/set is dropped by the filters, so init cannot break. + +## Verified WHP facts (windows crate 0.62.2 + WHP source) + +* WHP has **no MSR index enumeration**. MSRs are named `WHV_REGISTER_NAME` values + read/written via `WHvGet/SetVirtualProcessorRegisters`. A successful index→name + mapping is the availability oracle (`msr_to_whv_register_name` in `whp.rs`). +* Register-value pattern: `Align16(WHV_REGISTER_VALUE { Reg64: value })`; read via + `unsafe { out[i].0.Reg64 }`. +* Named registers used by the reset set (all present in the crate): `WHvX64Register` + `SysenterCs/Esp/Eip`, `Pat`, `Star/Lstar/Cstar/Sfmask`, `KernelGsBase`, `SpecCtrl`, + `UCet/SCet/Pl0Ssp..Pl3Ssp/InterruptSspTableAddr`, `TscAdjust`, `TscAux`, + `MsrMtrrDefType`, `MsrMtrrPhysBase0..F`, `MsrMtrrPhysMask0..F`, + `MsrMtrrFix64k00000/Fix16k80000/Fix16kA0000/Fix4kC0000..Fix4kF8000`. +* `DEBUGCTL` has **no** WHP register (and would be un-resettable) — but its writes are + masked, so this is not a gap. `EFER`/`APIC_BASE` are carried in `sregs`. +* `WHV_EXTENDED_VM_EXITS` bit layout (from `WinHvPlatformDefs.h`): bit 0 = + `X64CpuidExit`, bit 1 = `X64MsrExit`, bit 2 = `ExceptionExit`. (Matches the existing + gdb `set_debug` code that uses `1 << 2` for exception exits.) +* `WHV_X64_MSR_EXIT_BITMAP` bit 0 = `UnhandledMsrs`. Capability query + `WHvCapabilityCodeX64MsrExitBitmap` returned `0x3f` (all six bits supported) on the + test host — yet the per-MSR path is still denied at setup, per crux #1. + +## Files touched (all under `src/hyperlight_host/src/` unless noted) + +* `hypervisor/regs/x86_64/msrs.rs` — added `MSR_TSC_ADJUST`, `MSR_TSC_AUX`, + `MSR_MTRR_DEF_TYPE`, `MSR_MTRR_FIX64K_00000` consts and the full MTRR/TSC block to + `MSR_TABLE` (all `MsrClass::Stateful`). Updated the module doc (raw `TSC` not reset; + guest TSC manipulation handled via `TSC_ADJUST`). +* `hypervisor/virtual_machine/whp.rs` — `msr_to_whv_register_name` (now includes MTRR + + TSC_ADJUST/TSC_AUX), `host_supports_msr`, `capture_msrs`/`apply_msrs`, + `validate_allowed_msrs`; the `WhpVm::new()` NOTE explaining why no deny filter; + dormant `WHvRunVpExitReasonX64MsrAccess` arm in `run_vcpu`. +* `hypervisor/hyperlight_vm/x86_64.rs` — WHP arm in reset-set resolution + (`core_reset_indices().filter(host_supports_msr).filter(capture ok)`) and + `validate_allowed_msrs` at creation. **Removed 7 redundant `cfg(target_arch = + "x86_64")` gates** (the whole file is already arch-gated at `mod x86_64;` in + `hyperlight_vm/mod.rs`). +* `sandbox/initialized_multi_use.rs` — the six `test_whp_*` tests (reset, allow, + non-resettable, MTRR/TSC, unresettable-classes-do-not-retain, broad no-leak sweep) + plus a matching `mshv3`-gated `test_mshv_no_msr_leaks_across_restore_broad_sweep` + for the Linux agent to run. +* `src/tests/rust_guests/simpleguest/src/main.rs` — gated `ReadMSR`/`WriteMSR` with + `#[cfg(target_arch = "x86_64")]` (they use `rdmsr`/`wrmsr`; won't compile on + aarch64). Ordering matches the existing `install_handler`/`trigger_int3` pattern + (`#[guest_function]` then `#[cfg]`), which is proven to build on aarch64. + +## What remains (to-do) + +1. **Run the mshv broad no-leak sweep on Linux (highest priority).** + `test_mshv_no_msr_leaks_across_restore_broad_sweep` was added but only *compiled* + reasoning applies — it is `mshv3`-gated and has never been built or run (Windows + cannot compile it). Run it on a real mshv host. It mirrors the validated WHP sweep + exactly; the 3-outcome logic (fault / mask / retain-and-reset) tolerates whatever + the host does, but if a volatile MSR not on the exclusion list turns out to be + guest-writable-and-advancing on mshv it may flake on the exact-match assertion — + add it to the excluded set if so. A genuine failure means a real mshv MSR leak. + + (The shared `MSR_TABLE` change itself was already validated on Linux by commit + `ef508a71`, which also fixed an over-broad `cfg` — the MSR access-violation exit + machinery is now `any(kvm, target_os = "windows")`, since mshv is filterless and + never emits those exits — and added the narrow mshv MTRR/TSC leak sweep.) +2. **Consider whether mshv should also reset MTRR/TSC.** mshv is filterless, so it + likely has the same leak. If `msr_to_hv_reg_name` maps these registers, mshv now + fixes it automatically via the shared table; if not, they are skipped (still + leaking). Worth an explicit mshv sweep like the WHP audit. +3. **Commit.** The whole phase is uncommitted. Keep commits small and signed + (`-S --signoff`), matching the repo's DCO/gpgsign requirement. +4. **Optional: normalize guest `cfg` ordering.** The file mixes `#[cfg]`-first (e.g. + `test_timer_interrupts`) and `#[guest_function]`-first (e.g. `install_handler`, + and the new `ReadMSR`/`WriteMSR`). Both compile on aarch64 here. `#[cfg]`-first is + the order-independent-safe convention if uniformity is wanted. + +## How to validate on a WHP host + +```powershell +just guests # build the guest binaries (ReadMSR/WriteMSR live here) +cargo test -p hyperlight-host --lib --profile=dev test_whp_ +cargo clippy -p hyperlight-host --all-targets --all-features --profile=dev -- -D warnings +cargo fmt -p hyperlight-host -- --check +``` + +A quick way to re-audit for new leaks: create a sandbox, for each candidate MSR +snapshot → guest `WriteMSR(sentinel)` → guest `ReadMSR` (confirms retention) → +`restore` → guest `ReadMSR`. If the last read differs from baseline, that MSR leaks +and must be added to the reset set (and needs a WHP named register to be resettable). diff --git a/plan.md b/plan.md new file mode 100644 index 000000000..233f434e4 --- /dev/null +++ b/plan.md @@ -0,0 +1,531 @@ +# Plan: MSR reset across `MultiUseSandbox::restore()` with per-MSR opt-out + +## 0. Starting point: branch `reset2` + +The work builds on `reset2`, not `main`. The checkout is detached at +`origin/reset2` @ `68ad5265` "Disallow guest from read/write MSRs by default". +Read this section first. §1 onward describes the target design. This section +maps the target onto what `reset2` already ships. + +`reset2` takes a prevention approach. It denies the guest all MSR access and +returns an error. It adds no reset. The target design inverts this. Reset is the +isolation guarantee, the guest may use MSRs, and access control is a separate +opt-in allow list. + +What `reset2` already contains: +* A KVM deny-all filter. `enable_msr_filter()` in + `hypervisor/virtual_machine/kvm/x86_64.rs` sets `KVM_MSR_FILTER_DEFAULT_DENY` + with one dummy all-zero-bit range plus `enable_cap(X86UserSpaceMsr, Filter)`. + Every `RDMSR`/`WRMSR` traps. +* Wired at VM creation. `hypervisor/hyperlight_vm/x86_64.rs` calls + `enable_msr_filter()` unless `config.get_allow_msr()` is set. +* The filter exits. `kvm/x86_64.rs` handles `X86Rdmsr`/`X86Wrmsr` with the + `error=1` plus `immediate_exit` handshake, injecting `#GP` into the guest, and + returns `VmExit::MsrRead`/`MsrWrite`. `virtual_machine/mod.rs` defines these + variants. +* A hard host error on any access. `hyperlight_vm/mod.rs` maps `MsrRead`/ + `MsrWrite` to `RunVmError::MsrReadViolation`/`MsrWriteViolation`, then to + `HyperlightError::MsrReadViolation`/`MsrWriteViolation`. A guest MSR access both + faults the guest and fails the host call. +* An all-or-nothing config. `sandbox/config.rs` has `allow_msr: bool` (default + `false`) and `unsafe fn set_allow_msr(bool)`. Setting it `true` skips + `enable_msr_filter()` entirely, so the guest reads and writes every MSR with no + reset. Its own doc comment states this leaks state across restores. +* KVM only. The filter and the `allow_msr` field are + `#[cfg(all(kvm, target_arch = "x86_64"))]`. mshv and WHP get nothing. +* No reset. `reset2` never captures or restores an MSR baseline. `reset_vcpu` + still resets only the `sregs` MSRs (`EFER`, `APIC_BASE`). + +What to change or remove: +* Add reset. The whole of §5, §6, and §7 is new. `CommonMsrs`, snapshot capture, + and the `reset_vcpu` write-back do not exist yet. +* Replace `set_allow_msr(bool)` with an allow list (§9, §10). Drop the `unsafe` + bypass. Allowing an MSR must never disable reset. Update the + `set_allow_msr(true)` call site in + `sandbox/initialized_multi_use.rs` (around line 2777). +* Keep `KVM_MSR_FILTER_DEFAULT_DENY` and the dummy range. Add real allow ranges + for opted-in MSRs so allowed MSRs pass through instead of trapping (§9). +* Reframe the violation path. Under the target design a denied MSR faults with + `#GP` and poisons, which the existing `MsrReadViolation`/`MsrWriteViolation` + path already does. It stays for denied MSRs. Allowed MSRs must not hit it. +* Un-gate the config. Move the allow list off `#[cfg(kvm)]` so mshv and WHP honor + it. Reset applies on all backends. Access control is per-backend (§9). + +Resolved: default allow list is empty. +* Hyperlight guests do not write MSRs in normal operation, so deny-all plus reset + is the default and standard guests run unaffected. `reset2`'s deny-all does not + break them. +* Some guests need MSR writes. hyperlight-unikraft is a unikernel that sets up + syscall and base MSRs (`STAR`, `LSTAR`, `CSTAR`, `SFMASK`, `FS_BASE`, `GS_BASE`, + `KERNEL_GS_BASE`, sysenter, `PAT`). Those guests supply an explicit allow list + (§9, §10). Allowed MSRs still reset across `restore()`. +* This confirms the two-mechanism design: deny by default, opt in per MSR, reset + always. Follow-up: audit the exact set hyperlight-unikraft writes and ship it as + a documented allow list (or a named bundle) for that use case. The write set + lives in the Unikraft guest kernel (`unikraft/unikraft` `plat-hyperlight` + branch), not the host repo. Expect the syscall and base bundle plus `EFER` and + `PAT` from x86-64 long-mode and syscall setup. + +## 0a. Guest-CPUID contract (Phase-2 precondition) + +The reset-set completeness guarantee reduces to one input: the guest CPUID +Hyperlight exposes. Reset covers "every MSR a guest can persist", and CPUID +decides which MSR classes exist for the guest. Until this is pinned the guarantee +is bounded, not proven. This is a hard precondition for the mshv and WHP phases +(§13), not an open question. + +Define, per backend, the exposed CPUID feature set that gates an MSR class: MTRR, +x2APIC, TSC-deadline, CET, FRED, LBR, PMU, and synthetic (`HV_X64_MSR_*`). + +Current state, verified. Hyperlight does not own a pinned CPUID today. KVM starts +from `get_supported_cpuid()` and modifies only leaf `0x8000_0008` +(`kvm/x86_64.rs`), so the guest sees the host's MTRR, CET, FRED, LBR, and PMU +bits. mshv and WHP set no guest CPUID, they take the hypervisor default. So the +exposed MSR classes are host-varying, not Hyperlight-owned. Establishing the +contract is new work: Hyperlight masks the gating CPUID bits it does not support. +That masking is the mshv/WHP Phase-2 precondition. KVM Phase 1 does not need it, +`KVM_GET_MSR_INDEX_LIST` already tracks host variance. + +Rules, the target after the masking work: +* The exposed set is explicit and version-pinned per backend. Hyperlight owns it + by masking CPUID. Today it is host pass-through, which this work changes. +* Every exposed MSR class maps to reset-set coverage. A class the reset set does + not cover is a hypervisor-selection failure, not a silent gap. +* The static table (§4) and the exposed CPUID set agree. A test asserts the two + match (§12). +* Masked classes fault. A guest `WRMSR`/`RDMSR` to a masked class faults, and a + test asserts it on all backends (§12). + +Deliver this before Phase 2. The WHP and mshv guarantees stay provisional until +it lands. + +## 1. Goal + +`MultiUseSandbox::restore()` rolls a sandbox back to a snapshot. This plan covers +MSR and LAPIC state. Its guarantee is that no MSR or LAPIC state leaks across a +`restore()`. For MSRs this means resetting every MSR a guest could have modified +to a known baseline. A user can allow specific MSRs for guest read and write. +Reset covers those too. + +This plan does not claim full restore isolation. Adjacent VP state (pending events +and exceptions, interrupt shadow, XCR0/XSS) needs its own audit and test gate +before `restore()` can claim to leak no state at all. See §14. + +Constraints: +* Works on KVM, mshv (Linux dom0), and WHP (Windows). +* Fast on the `restore()` hot path. + +## 2. Design principle + +The design has two independent mechanisms: reset and access control. + +Reset runs on every `restore()`. Hyperlight writes a saved baseline value back +to every MSR in the reset set. It does this for every MSR in the set, whether or +not the guest was allowed to touch that MSR. + +Access control runs while the guest executes. It decides whether the guest may +read or write each MSR. A denied MSR faults the guest with `#GP`. An allowed MSR +goes through. The allow list configures which MSRs are allowed. + +The two mechanisms are independent. Whether an MSR is allowed or denied does not +change whether reset covers it. Reset always writes the full reset set. + +Consequences: +* Access control is defense in depth. Reset is the isolation guarantee. +* Allowing an MSR lets the guest use it. Hyperlight still captures and resets + that MSR like every other one in the reset set. +* Hyperlight adds an MSR to the reset set only if it can read and write that MSR + as ordinary state. A request to allow an MSR that cannot be reset fails at + init. See §10. + +## 3. Reset set + +The reset set holds every MSR a guest can persist that Hyperlight can get and +set as stateful state, plus user-allowed MSRs. Derive it by expansion. See §4. + +Core stateful MSRs. Numbers match on Intel and AMD. `EFER` and `APIC_BASE` live +in `sregs`. Use canonical constant names (`MSR_LSTAR` and so on) in code and +tests. The numbers here are for reference. +* syscall: `STAR 0xC0000081`, `LSTAR 0xC0000082`, `CSTAR 0xC0000083`, + `SFMASK 0xC0000084` +* bases: `FS_BASE 0xC0000100`, `GS_BASE 0xC0000101`, `KERNEL_GS_BASE 0xC0000102`. + Verified: `FS_BASE` and `GS_BASE` are segment-descriptor bases in `sregs` + (`fs.base`, `gs.base`) and `apply_sregs`/`set_sregs` resets them on every restore + on all backends. Keep them out of `CommonMsrs` entirely. Only `KERNEL_GS_BASE` + needs MSR reset. +* sysenter: `SYSENTER_CS 0x174`, `SYSENTER_ESP 0x175`, `SYSENTER_EIP 0x176` +* misc: `PAT 0x277`, `DEBUGCTL 0x1D9` (holds LBR enable bits, see §8) +* CPUID-gated: `SPEC_CTRL 0x48`, CET (`U_CET/S_CET/PL{0-3}_SSP/INT_SSP_TAB`), + FRED, `MISC_ENABLE 0x1A0`, `UMWAIT_CONTROL 0xE1`, `BNDCFGS 0xD90`, + `XSS 0xDA0`, MTRRs (see §8) + +Special handling: +* `TSC 0x10` and `TSC_ADJUST 0x3B`. Verified: Hyperlight does no TSC setup (no + `KVM_SET_TSC_KHZ`, no offset, no `IA32_TSC`/`TSC_ADJUST` writes). The bare TSC + is hypervisor-driven and runs monotonically across VP reuse, it is never + rewound. Reset `TSC_ADJUST` to its captured baseline (0 unless the guest wrote + it) and never write raw `TSC`. Resetting `TSC_ADJUST` alone keeps guest time + monotonic, no host-offset re-derivation. Mark `TSC` and `TSC_AUX` `host_specific` + in the data model (§5) so they never enter the batched `Stateful` restore. Test + for `TSC_ADJUST` leakage. +* `TSC_DEADLINE 0x6E0`. LAPIC timer state. Reset it in the APIC unit, see §7. +* Write-only command MSRs `PRED_CMD 0x49` and `FLUSH_CMD 0x10B`. `RDMSR` faults + and they hold no state. Class `WriteOnlyCmd`. Skip capture. + +Pruning depends on Hyperlight's minimal partition. No SynIC and no synthetic +features make `HV_X64_MSR_*` fault. No vPMU makes Perfmon MSRs fault. Guest +CPUID enforces this with a fail-fast check. See §4. + +## 4. Deriving the list + +Grow the set from enumeration. Exclude only the `HostOwned` class (§5). +Expansion keeps enumerated guest-writable MSRs like `MISC_ENABLE`, +`UMWAIT_CONTROL`, `BNDCFGS`, `XSS` in the set. + +Classification comes from a static table keyed by index. Each entry holds its +class and its gating CPUID feature. One CPUID read per process resolves which +entries apply. The batched baseline capture at init confirms each entry reads +back. A blind per-MSR read at init is unnecessary and risks read side effects. + +* KVM. Enumerate through `KVM_GET_MSR_INDEX_LIST`, a host oracle that tracks + vendor, generation, and microcode. Reset set = enumerated writable MSRs, minus + the `HostOwned` class (§5), keeping `Stateful` get/settable entries per the + table. The core list in §3 is a lower bound. A missing core MSR fails fast. +* mshv. Enumerate through `mshv-ioctls` `get_msr_index_list`. This returns a + static array baked into the crate, not a probed host list. Treat it as a + starting set and cross-check it against the §0a CPUID contract, not as an + authority on the running host. +* WHP. No enumeration API. WHP models MSRs as named `WHV_REGISTER_NAME` values. + Bound the set from the §0a CPUID contract and the static table. Fail closed at + hypervisor selection when the host exposes an architectural MSR class outside + the covered set. The guarantee reads "complete for the guest CPUID we expose", + which §0a pins. The nightly sweep in §12 is the regression oracle. +* All backends. `KVM_GET_MSR_INDEX_LIST` and the mshv static list both omit + MTRRs, so add the exposed MTRR MSRs from CPUID (§8). Any enumerated or + CPUID-derived writable index the static table cannot classify fails closed. + Probe it once with a get/set round-trip. Add it to the reset set if it + round-trips as stateful. Refuse VM creation if it does not. Never drop an + unclassified writable MSR and defer it to the nightly sweep, that leaks until + the sweep runs and contradicts §2. +* Cache the resolved set once per process with `LazyLock`. It depends on host CPU + and hypervisor, both fixed for the process. Per-sandbox init captures values. + +Probe a single index only to classify a user-allowed or unclassified MSR the +table lacks. The static table marks indices with read side effects, and the probe +skips a blind read on those. + +## 5. Data model and APIs + +`CommonMsrs` is a `Vec`. Iterating dozens of entries costs little +against the syscall. Each entry carries metadata: + +```rust +struct MsrEntry { + index: u32, + value: u64, + class: MsrClass, // Stateful, WriteOnlyCmd, ReadOnly, HostOwned, LapicOwned + flags: MsrFlags, // host_specific (e.g. TSC), ordering_sensitive +} +``` + +Capture and restore cover `Stateful` get/settable entries. The class field keeps +`WriteOnlyCmd` (`PRED_CMD`, `FLUSH_CMD`), `ReadOnly`, `HostOwned`, and +`LapicOwned` (§7) out of the baseline. The `HostOwned` class is the single source +of truth for host-owned exclusion. §4 has no separate denylist. The +`host_specific` flag marks entries the batched `Stateful` restore skips (raw +`TSC`, `TSC_AUX`), and the batch asserts none is present. This metadata covers the +write-only, read-only, TSC-ordering, and LAPIC cases. + +`Snapshot` gains `msrs: Option` beside `sregs`. `snapshot()` captures +it. `reset_vcpu` applies it. + +Backend get/set sits behind a small trait over `CommonMsrs`. All calls batch into +one syscall or hypercall. +* KVM: `KVM_GET_MSRS` and `KVM_SET_MSRS` (`kvm-ioctls` `get_msrs`/`set_msrs`). + `KVM_SET_MSRS` returns a count and stops at the first bad index. A short count + poisons the sandbox and fails closed. A partially reset VP stays out of guest + execution. +* mshv: `get_msrs`/`set_msrs` (`mshv-ioctls` 0.6.9, `VcpuFd`). They take a `Msrs` + array of index plus data like KVM. Internally they translate each index to an + `hv_register_name` with the crate's `msr_to_hv_reg_name()` and issue one batched + `set_reg` (a single `HVCALL_SET_VP_REGISTERS` rep hypercall). Two failure modes, + both fail-closed through poison-on-`Err`: + * An index `msr_to_hv_reg_name()` cannot map returns `EINVAL` in the pre-loop. + Nothing is applied. + * A register the hypervisor rejects mid-rep leaves earlier registers committed, + then `hvcall_set_reg` returns `EINTR` (it checks `args.reps != len`). This is + a partial apply, not all-or-nothing. Do not rely on atomicity. Rely on + poison-on-`Err`: a poisoned VP never runs the guest again. + `set_msrs` returns `Ok(0)`, not a written count, so KVM's short-count detection + does not apply. Detect failure from the `Err`, not a count. +* WHP: `WHvGetVirtualProcessorRegisters` and `WHvSetVirtualProcessorRegisters`. + Fail closed on error. + +mshv MSR mapping coverage. `msr_to_hv_reg_name()` (mshv-bindings 0.6.9) maps +syscall, sysenter, `KERNEL_GS_BASE`, `PAT`, `DEBUG_CTL`, `SPEC_CTRL`, +`MISC_ENABLE`, `BNDCFGS`, `TSC`/`TSC_ADJUST`/`TSC_AUX`, CET, `U_XSS`, and the fixed +MTRR set. It does not map `FS_BASE`, `GS_BASE`, `UMWAIT_CONTROL 0xE1`, FRED, or the +LBR stack. Handle each unmapped index by an explicit path, never by silent +omission: +* `FS_BASE`, `GS_BASE`: reset through `sregs` segment bases on all backends, not + `CommonMsrs` (§3, verified). Drop them from `CommonMsrs` everywhere, not just + mshv. +* `UMWAIT_CONTROL`, FRED, LBR stack: mask the gating CPUID bit on mshv (§0a) so + the guest cannot set them, or supply a verified alternate VP-register path. An + exposed-but-unmapped class with no path is a backend-selection failure, not a + silent gap. A test asserts every reset-set index maps or has a documented path + (§12). + +## 6. Wire-up and snapshot semantics + +`snapshot()` captures `CommonMsrs` after `get_snapshot_sregs()`, `Stateful` +entries only. + +`reset_vcpu` order: `apply_sregs` with `EFER` first so `LMA` settles, then the +batched `SET_MSRS` of the generic `Stateful` set, then the APIC unit (§7), then +`TSC_ADJUST`/offset (§3). Skip read-only and write-only entries. The batched +`SET_MSRS` holds no `host_specific` entry. Raw `TSC` and `TSC_AUX` restore +separately in the TSC step, not the batch. Mask reserved bits. + +`restore(S)` sets the VM MSR state equal to `S`. Reset is the effect of writing +the captured values back. The invariant: + +> The reset set belongs to the sandbox. It equals the fixed core set plus the +> allow list. Every snapshot captures this whole set at snapshot time. Every +> `restore()` writes this whole set. The allow list drives access control (§9) +> alone. + +Rules: +* Capture and restore allowed MSRs the same way as any other. A snapshot of a + sandbox with allowed MSRs holds those MSRs. A baseline snapshot resets them to + baseline. +* Capture at snapshot time, the point `sregs` captures. Restoring a mid-run + snapshot reproduces those MSR values. +* Init baseline for from-binary snapshots. Capture an `init_baseline` of the full + reset set once at init. A from-binary snapshot (`msrs == None`) resets to it. A + snapshot carrying `msrs` restores those captured values. The init baseline + stands in only for `msrs == None`. See §11 for persistence. +* Capture the same set every time. A snapshot whose captured set differs from + what `restore()` writes leaves a live MSR in place. The set stays fixed for the + sandbox lifetime. + +Example, allowed `KERNEL_GS_BASE`: +1. init sets `init_baseline[KERNEL_GS_BASE] = v0`. Snapshot A captures `v0`. +2. guest writes `v1`. Snapshot B captures `v1`. +3. `restore(A)` writes `v0`. Baseline. +4. `restore(B)` writes `v1`. Reproduces the state. +5. Each restore overwrites the live value from the snapshot or init baseline. No + value crosses tenants. + +## 7. LAPIC + +Verified in the source. Hyperlight creates an in-kernel LAPIC on all three +backends, gated by the `hw-interrupts` feature. KVM calls `create_irq_chip` +(`kvm/x86_64.rs`, PIC+IOAPIC+LAPIC). mshv sets the partition `MSHV_PT_BIT_LAPIC` +and calls `set_lapic` (`mshv/x86_64.rs`). WHP sets +`WHvPartitionPropertyCodeLocalApicEmulationMode` to `XApic` (`whp.rs`). The LAPIC +is xAPIC (MMIO at `0xFEE00000`), not x2APIC. No backend sets the x2APIC CPUID bit +and WHP is explicitly XApic mode. + +The in-kernel LAPIC holds state a guest can mutate (timer, LVT, priorities) that +generic MSR get/set does not cover. The KVM MSR filter's x2APIC blind spot +(`0x800-0x8FF`) is not the driver here, since x2APIC is off. The driver is the +xAPIC page state. Reset it through the LAPIC state APIs. + +Reset `APIC_BASE`, then the LAPIC page, then `TSC_DEADLINE` as one ordered unit +after `apply_sregs`. This clears any pending timer interrupt. `KVM_SET_LAPIC` +restores the whole APIC page, including IRR, ISR, and TMR. A test asserts these +clear (§12). +* KVM: `KVM_GET_LAPIC` at init, `KVM_SET_LAPIC` on restore after `APIC_BASE`. +* mshv: `get_lapic`/`set_lapic` (`MSHV_*_VP_STATE_LAPIC`). +* WHP: `WHvGet/SetVirtualProcessorInterruptControllerState` after `APIC_BASE`. + +This is mandatory whenever `hw-interrupts` is enabled. Without that feature there +is no in-kernel LAPIC and this section does not apply. Test that a restore leaves +no pending timer interrupt. + +## 8. LBR and MTRR + +`DEBUGCTL 0x1D9` holds LBR enable bits. LBR works without a vPMU. The LBR stack +leaks prior-tenant control flow. Pick one and test it. Mask LBR and assert guest +LBR access faults on all three backends. Or add the KVM-exposed LBR control, TOS, +`FROM`, and `TO` stack MSRs to the reset set. + +MTRRs are architectural and guest-writable from ring 0. Reset is the guarantee on +mshv and WHP, so cover them. Gate on guest CPUID. Add the exposed MTRR MSRs +(`MTRRcap`, `MTRRdefType`, fixed and variable pairs) to the baseline. Or mask +MTRR from CPUID and test guest `WRMSR` faults on all backends. + +## 9. Access control + +This layer decides `#GP` on an MSR. It leaves reset alone. The default allow list +is empty (§0). Standard guests do not write MSRs and run under deny-all plus +reset. A guest that needs MSR writes (for example hyperlight-unikraft) supplies an +explicit allow list. Allowed MSRs still reset. + +* KVM. Build `KVM_X86_SET_MSR_FILTER` from the allow list. Default action DENY. + Allow ranges cover opted-in MSRs. Denied MSRs trap to `#GP` and poison. Allowed + MSRs pass through. KVM re-intercepts filtered pass-through MSRs on its own + (`vmx_set_intercept_for_msr` consults `kvm_msr_allowed`), so the filter covers + `FS_BASE`, `SPEC_CTRL`, and the rest. Encoding: DEFAULT_DENY needs at least one + range. Emit one all-zero-bit range. Validate + `KVM_CAP_X86_MSR_FILTER` at init. Fail closed when the filter cannot install. +* WHP. A real per-MSR filter. Set the partition properties before + `WHvSetupPartition`, the point WHP freezes partition configuration. Default-deny + is the union {`X64MsrExitBitmap` ∪ `MsrActionList` ∪ `UnimplementedMsrAction`}, + not `UnimplementedMsrAction` alone. `UnimplementedMsrAction` governs only MSRs + WHP does not implement. An implemented architectural MSR absent from both + `MsrActionList` and `X64MsrExitBitmap` falls to `WHvMsrActionArchitectureDefault` + and passes through. So generate an explicit `WHvMsrActionExit` + `WHV_MSR_ACTION_ENTRY` for every implemented, denied MSR. Give each allowed MSR + an explicit `WHvMsrActionArchitectureDefault` entry so it passes through. Assert + at init that the union covers every denied MSR. This is a hard assertion, not a + best-effort confirm. `WHvPartitionPropertyCodeX64MsrExitBitmap` (capability + `WHvCapabilityCodeX64MsrExitBitmap`) is a fixed named-MSR bitmap. A trapped + access raises `WHvRunVpExitReasonX64MsrAccess`. Hyperlight injects `#GP` and + poisons. `WHvMsrActionIgnoreWriteReadZero` drops a write and reads zero with no + exit. Symbols present in `windows` 0.62.2. A compile-time + `const _: WHV_MSR_ACTION_ENTRY` assertion guards against binding drift, since the + symbol claim has flipped across reviews. +* mshv. Coarser. Install the X64 MSR intercept (`install_intercept`). On an + intercept the handler explicitly injects `#GP` into the guest and poisons the + sandbox. It does not merely return an error. Both mshv and WHP sit on the same + Hyper-V hypervisor. Its sources show the catch-all reflects only + non-hypervisor-owned MSRs (see §4), and the pass-through set bypasses + interception, so the intercept alone cannot deny every MSR. Treat it as a + fail-safe and detector, backed by source and tests. Reset (§5, §6) protects + tenant isolation on both. + +## 10. Validating allowed MSRs + +At init, for each allowed MSR: +1. Confirm the host has it. KVM index-list membership, or a targeted probe on + mshv/WHP. Absent means error. +2. Confirm it is `Stateful` and settable back to baseline. Sticky or lock bits + (e.g. `IA32_FEATURE_CONTROL` lock), write-only, and read-only fail. Not + resettable means a hard error at init. No bypass. +3. Add it to the reset set and to the allow ranges. + +## 11. Persistence: `save` and `load` + +The in-memory `Snapshot` is not `Serialize`. Persistence goes through +`OciSnapshotConfig`, which serializes `sregs`. A `#[serde(skip)]` on +`Snapshot::msrs` would be inert. Preserving MSR values on disk means extending the +`OciSnapshotConfig` schema, not tagging the in-memory struct. + +Decision: persist the reset set for runnable snapshots. +* Extend `OciSnapshotConfig` with the non-host-specific `CommonMsrs`. Record the + captured schema, the set of indices. Keep host-specific MSRs (`TSC`, `TSC_AUX`) + out and re-derive them on load. +* On load, error when the sandbox reset set holds an MSR absent from the persisted + schema. A schema mismatch is a hard failure, not a silent fallback to init. +* A baseline or from-binary snapshot carries no modified allowed MSRs. It persists + no MSR values. `load` re-captures the `init_baseline` from the initialized VP. +* Phasing option, not a shipping fallback: until the schema extension lands, + `save()` rejects a snapshot whose reset set holds a modified allowed MSR. This + option cannot ship with allowed-MSR persistence enabled. Never let `save`/`load` + silently return an allowed MSR to init instead of its captured value. + +## 12. Testing and fuzzing + +* Fixed-list differential, per PR. For each reset-set MSR, guest writes a + sentinel, `restore()`, host reads back, assert baseline. Sub-millisecond. +* KVM index-list sweep, per PR. Sweep `KVM_GET_MSR_INDEX_LIST` (~100-200 + entries): write sentinel, restore, assert baseline. Covers the enumerable MSR + space. x2APIC, LBR, and vCPU events need the tests below. +* Opt-out, per PR. Allow one MSR (e.g. `KERNEL_GS_BASE`). Assert the guest reads + and writes it. Assert `restore()` returns it to baseline. Assert a + non-resettable allow request errors. +* Full-window sweep, nightly, Hyper-V and KVM. Guest writes `wrmsr` across + `0x0-0x1FFF` and `0xC0000000-0xC0001FFF` in one pass with a `#GP`-skipping + handler. Log the writes that land. One `restore()`. Assert no sentinel + survives. Batched, sub-second. Keep a denylist of control MSRs (`EFER`, `PAT`, + MTRRs, `APIC_BASE`) tested one at a time so the vCPU holds. +* Fuzz target in `fuzz/`. Input: an MSR index biased to the windows plus a value. + Invariant: post-restore host read equals baseline. Bounded per PR, longer + nightly. +* LAPIC. With a guest LAPIC (`hw-interrupts`), mutate xAPIC state, restore, assert + LAPIC baseline, no pending timer interrupt, and cleared IRR, ISR, and TMR (§7). +* Negative leak, all backends. Write a sentinel, `restore()`, assert the sentinel + is gone. +* Fail closed. The default-deny filter installs on KVM. A non-get/settable allow + request errors. A simulated partial `SET_MSRS` poisons the sandbox. An + enumerated writable MSR the static table cannot classify fails closed at VM + creation rather than dropping from the reset set (§4). +* Persistence fidelity. A persisted mid-run snapshot with a modified allowed MSR + round-trips through the extended `OciSnapshotConfig` schema (§11), or `save` + rejects it. A load-time schema mismatch errors. +* CPUID matrix, all backends. Assert the §0a exposed-CPUID contract per backend. + Assert the static table (§4) and the exposed CPUID set agree. Assert masked + features (`MTRR`, LBR, PMU, x2APIC, CET, FRED, synthetic) fault on `WRMSR` and + `RDMSR`. +* mshv register mapping. Assert every reset-set index either maps through + `msr_to_hv_reg_name()` and round-trips through batched `get_msrs`/`set_msrs`, or + has a documented alternate path (`sregs` for `FS_BASE`/`GS_BASE`) or a §0a mask + (`UMWAIT_CONTROL`, FRED, LBR). A reset-set index with no mapping and no path + fails the test. +* mshv partial apply. Simulate a mid-rep register rejection (`EINTR` after a short + `args.reps`). Assert the sandbox poisons and never runs the guest again. The KVM + short-count test does not cover this path. +* WHP setup. With the filter set before `WHvSetupPartition`, a denied MSR raises + `WHvRunVpExitReasonX64MsrAccess` and an allowed MSR passes through. Assert an + implemented architectural MSR absent from `MsrActionList` and `X64MsrExitBitmap` + still traps, which guards the union default-deny. A compile-time assertion binds + the `WHV_MSR_ACTION_ENTRY` symbols. +* LBR. Guest LBR access faults on all backends, or the LBR stack resets. +* Run on Intel and AMD CI. + +## 13. Backend matrix and phasing + +| Capability | KVM | mshv | WHP | +|---|---|---|---| +| Reset baseline get/set | `KVM_GET/SET_MSRS` | `get_msrs`/`set_msrs` | `WHvGet/SetVpRegisters` | +| List derivation | `KVM_GET_MSR_INDEX_LIST` | `get_msr_index_list` (static) | CPUID + table | +| Access control | `KVM_X86_SET_MSR_FILTER` | `install_intercept` | `MsrActionList` + `X64MsrExitBitmap` | +| LAPIC reset | `KVM_GET/SET_LAPIC` | `MSHV_*_VP_STATE_LAPIC` | interrupt-ctrl state | + +\* mshv `get_msrs`/`set_msrs` translate each index with `msr_to_hv_reg_name()` and +batch one rep hypercall. The mapping omits `FS_BASE`/`GS_BASE` (use `sregs`), +`UMWAIT_CONTROL`, FRED, and LBR (mask via §0a or supply a VP-register path, see +§5). An unmappable index returns `EINVAL`, a mid-rep rejection returns `EINTR` +after a partial apply. Both fail closed via poison. + +Phasing: +1. Split reset from access control. Add `CommonMsrs` with `MsrEntry` metadata. + Capture and restore on KVM with enumerate-and-expand (§4). Convert `allow_msr` + bool to the allow list. Fail-closed validation. Tests. +2. Add the reset baseline on mshv and WHP. Precondition: the §0a guest-CPUID + contract is defined and enforced as a hypervisor-selection check. mshv derives + from the static `get_msr_index_list` plus CPUID for MTRRs and captures through + batched `get_msrs`/`set_msrs`. WHP derives from CPUID plus table. Cached per + process. +3. LAPIC reset on all backends when the guest has a LAPIC. +4. Access control on Hyper-V. WHP per-MSR filter (`MsrActionList`). mshv + intercept as fail-safe and detector. +5. Nightly sweep and fuzz. Intel and AMD CI. + +## 14. Open questions + +* The §0a CPUID contract. Verified: today CPUID is host pass-through on KVM (only + leaf `0x8000_0008` modified) and hypervisor-default on mshv and WHP. Hyperlight + masks nothing. Establishing the pinned contract is new masking work and the real + Phase-2 precondition (§0a). KVM Phase 1 is unaffected because + `KVM_GET_MSR_INDEX_LIST` handles host variance. +* The final reset set once CET, FRED, and MTRR support lands. Keep it CPUID-gated + and test-asserted. +* Resolved: Hyperlight does no TSC setup, so the bare TSC runs monotonically + across reuse and resetting `TSC_ADJUST` to baseline suffices, no offset + re-derivation (§3). +* Resolved: Hyperlight creates an in-kernel xAPIC LAPIC on all backends when + `hw-interrupts` is on, and does not expose x2APIC (§7). §7 is mandatory under + `hw-interrupts`, and the x2APIC MSR blind spot does not apply. +* Whether the crate's `msr_to_hv_reg_name()` table covers every reset-set index + (CET, FRED, MTRR, LBR when exposed). An unmapped index returns `EINVAL` and + fails closed (§5). Audit the table against the reset set. +* How the init capture avoids mutating MSRs with read side effects. The static + table marks such indices, and capture skips a blind read (§4). +* Adjacent VP state (pending events and exceptions, interrupt shadow, XCR0/XSS). + Out of scope for this plan's guarantee (§1). Track a separate audit and test + gate before claiming full restore isolation. LAPIC IRR/ISR/TMR are in scope and + reset with the APIC page (§7). +* Per-process cost on mshv and WHP. Keep the resolved set cached. Read CPUID once. diff --git a/src/hyperlight_host/src/error.rs b/src/hyperlight_host/src/error.rs index c6738374d..c81898454 100644 --- a/src/hyperlight_host/src/error.rs +++ b/src/hyperlight_host/src/error.rs @@ -154,6 +154,26 @@ pub enum HyperlightError { #[error("Memory Access Violation at address {0:#x} of type {1}, but memory is marked as {2}")] MemoryAccessViolation(u64, MemoryRegionFlags, MemoryRegionFlags), + /// Guest read from an MSR. Denied by default. Allow via + /// [`crate::sandbox::SandboxConfiguration::allow_msr`]. + #[cfg(target_arch = "x86_64")] + #[error( + "Guest attempted to read from MSR {0:#x}. MSR access is denied by default. \ + SandboxConfiguration::allow_msr can allow it, but it weakens \ + sandbox isolation and should not be enabled without good reason" + )] + MsrReadViolation(u32), + + /// Guest wrote to an MSR. Denied by default. Allow via + /// [`crate::sandbox::SandboxConfiguration::allow_msr`]. + #[cfg(target_arch = "x86_64")] + #[error( + "Guest attempted to write {1:#x} to MSR {0:#x}. MSR access is denied by default. \ + SandboxConfiguration::allow_msr can allow it, but it weakens \ + sandbox isolation and should not be enabled without good reason" + )] + MsrWriteViolation(u32, u64), + /// Memory Allocation Failed. #[error("Memory Allocation Failed with OS Error {0:?}.")] MemoryAllocationFailed(Option), @@ -352,6 +372,10 @@ impl HyperlightError { // as poisoning here too for defense in depth. | HyperlightError::HyperlightVmError(HyperlightVmError::Restore(_)) => true, + #[cfg(target_arch = "x86_64")] + 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 56f7d635d..9609992f5 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", any(kvm, target_os = "windows")))] + DispatchGuestCallError::Run(RunVmError::MsrReadViolation(msr_index)) => { + HyperlightError::MsrReadViolation(msr_index) + } + + #[cfg(all(target_arch = "x86_64", any(kvm, target_os = "windows")))] + 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", any(kvm, target_os = "windows")))] + #[error("Guest attempted to read from MSR {0:#x}")] + MsrReadViolation(u32), + #[cfg(all(target_arch = "x86_64", any(kvm, target_os = "windows")))] + #[error("Guest attempted to write {value:#x} to 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,11 @@ pub(crate) struct HyperlightVm { pub(super) trace_info: MemTraceInfo, #[cfg(crashdump)] pub(super) rt_cfg: SandboxRuntimeConfig, + /// MSR reset state (reset-set indices + baseline captured at init). + /// `Some` only on KVM/mshv x86_64 where MSR reset is active; `None` + /// elsewhere, in which case MSR capture/restore is a no-op. + #[cfg(target_arch = "x86_64")] + pub(super) msr_reset: Option, } impl HyperlightVm { @@ -732,6 +753,14 @@ impl HyperlightVm { } } } + #[cfg(all(target_arch = "x86_64", any(kvm, target_os = "windows")))] + Ok(VmExit::MsrRead(msr_index)) => { + break Err(RunVmError::MsrReadViolation(msr_index)); + } + #[cfg(all(target_arch = "x86_64", any(kvm, target_os = "windows")))] + 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 82206f787..e69003827 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -41,16 +41,21 @@ use crate::hypervisor::gdb::{ #[cfg(gdb)] use crate::hypervisor::gdb::{DebugError, DebugMemoryAccessError}; use crate::hypervisor::regs::{ - CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, + CommonDebugRegs, CommonFpu, CommonMsrs, CommonRegisters, CommonSpecialRegisters, MsrResetState, + core_reset_indices, }; #[cfg(not(gdb))] use crate::hypervisor::virtual_machine::VirtualMachine; #[cfg(kvm)] use crate::hypervisor::virtual_machine::kvm::KvmVm; +#[cfg(all(kvm, target_arch = "x86_64"))] +use crate::hypervisor::virtual_machine::kvm::host_msr_indices; #[cfg(mshv3)] use crate::hypervisor::virtual_machine::mshv::MshvVm; +#[cfg(all(mshv3, target_arch = "x86_64"))] +use crate::hypervisor::virtual_machine::mshv::host_supports_msr; #[cfg(target_os = "windows")] -use crate::hypervisor::virtual_machine::whp::WhpVm; +use crate::hypervisor::virtual_machine::whp::{WhpVm, host_supports_msr}; use crate::hypervisor::virtual_machine::{ HypervisorType, RegisterError, VmError, get_available_hypervisor, }; @@ -92,11 +97,35 @@ impl HyperlightVm { let vm: VmType = 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) + } #[cfg(mshv3)] - Some(HypervisorType::Mshv) => Box::new(MshvVm::new().map_err(VmError::CreateVm)?), + Some(HypervisorType::Mshv) => { + let mshv_vm = MshvVm::new().map_err(VmError::CreateVm)?; + // mshv has no per-MSR filter, so the allow list does not + // gate guest access here (that is Phase 4). It only widens + // the reset set, so validate that each allowed MSR can be + // reset before it is added. + mshv_vm + .validate_allowed_msrs(config.get_allowed_msrs()) + .map_err(VmError::CreateVm)?; + Box::new(mshv_vm) + } #[cfg(target_os = "windows")] - Some(HypervisorType::Whp) => Box::new(WhpVm::new().map_err(VmError::CreateVm)?), + Some(HypervisorType::Whp) => { + let whp_vm = WhpVm::new().map_err(VmError::CreateVm)?; + // Validate that each allowed MSR can be reset before it is + // added to the reset set. + whp_vm + .validate_allowed_msrs(config.get_allowed_msrs()) + .map_err(VmError::CreateVm)?; + Box::new(whp_vm) + } None => return Err(CreateHyperlightVmError::NoHypervisorFound), }; @@ -168,11 +197,58 @@ impl HyperlightVm { trace_info, #[cfg(crashdump)] rt_cfg, + msr_reset: None, }; ret.update_snapshot_mapping(snapshot_mem)?; ret.update_scratch_mapping(scratch_mem)?; + // Capture the MSR reset baseline once the vCPU is fully set up. + // KVM and mshv on x86_64 support MSR reset; on other backends + // `msr_reset` stays `None` and restore is a no-op. + { + // The reset set is the core stateful table (filtered to what + // this host actually supports) unioned with the validated + // allow list, so every guest-writable MSR Hyperlight can reset + // is also reset. Host support is resolved per backend: KVM uses + // `KVM_GET_MSR_INDEX_LIST`; mshv probes each mappable index + // with a get, so an MSR that maps but is inactive on this host + // (for example CET where CET is not exposed) is skipped rather + // than failing init. + let core: Vec = match get_available_hypervisor() { + #[cfg(kvm)] + Some(HypervisorType::Kvm) => { + let host = host_msr_indices(); + core_reset_indices().filter(|i| host.contains(i)).collect() + } + #[cfg(mshv3)] + Some(HypervisorType::Mshv) => core_reset_indices() + .filter(|i| host_supports_msr(*i)) + .filter(|i| ret.vm.capture_msrs(&[*i]).is_ok()) + .collect(), + // WHP models MSRs as named registers with no index list. + // Bound the reset set to the mappable core MSRs that read + // back on this host. The deny-by-default filter keeps the + // guest from writing anything outside the allow list. + #[cfg(target_os = "windows")] + Some(HypervisorType::Whp) => core_reset_indices() + .filter(|i| host_supports_msr(*i)) + .filter(|i| ret.vm.capture_msrs(&[*i]).is_ok()) + .collect(), + _ => Vec::new(), + }; + if !core.is_empty() || !config.get_allowed_msrs().is_empty() { + let mut indices: Vec = core + .into_iter() + .chain(config.get_allowed_msrs().iter().copied()) + .collect(); + indices.sort_unstable(); + indices.dedup(); + let baseline = ret.vm.capture_msrs(&indices).map_err(VmError::Register)?; + ret.msr_reset = Some(MsrResetState { indices, baseline }); + } + } + // Send the interrupt handle to the GDB thread if debugging is enabled // This is used to allow the GDB thread to stop the vCPU #[cfg(gdb)] @@ -270,6 +346,56 @@ impl HyperlightVm { Ok(self.vm.sregs()?) } + /// Capture the current values of the reset-set MSRs, to be stored in a + /// snapshot. Returns `None` when MSR reset is not active (backends + /// without MSR reset), in which case restore is a no-op. + pub(crate) fn get_snapshot_msrs(&self) -> Result, AccessPageTableError> { + match &self.msr_reset { + Some(state) => Ok(Some(self.vm.capture_msrs(&state.indices)?)), + None => Ok(None), + } + } + + /// Restore the reset-set MSRs. Writes the snapshot's captured MSRs when + /// present, otherwise the baseline captured at init (a from-binary + /// snapshot carries no MSRs). A no-op when MSR reset is not active. + pub(crate) fn restore_msrs( + &mut self, + snap_msrs: Option<&CommonMsrs>, + ) -> std::result::Result<(), ResetVcpuError> { + if let Some(state) = &self.msr_reset { + let msrs = snap_msrs.unwrap_or(&state.baseline); + self.vm.apply_msrs(msrs)?; + } + Ok(()) + } + + /// Test-only: capture arbitrary MSRs host-side (bypassing the reset + /// set), used to sweep the full host-supported index list. + #[cfg(all(test, kvm, target_arch = "x86_64"))] + pub(crate) fn capture_msrs_for_test( + &self, + indices: &[u32], + ) -> std::result::Result { + self.vm.capture_msrs(indices) + } + + /// Test-only: best-effort host-side write of a single MSR. Returns + /// `true` if the write succeeded. Used to plant sentinels over the + /// full host index list, tolerating MSRs that reject arbitrary values. + #[cfg(all(test, kvm, target_arch = "x86_64"))] + pub(crate) fn try_set_msr_for_test(&self, index: u32, value: u64) -> bool { + use crate::hypervisor::regs::{MsrClass, MsrEntry, MsrFlags}; + self.vm + .apply_msrs(&vec![MsrEntry { + index, + value, + class: MsrClass::Stateful, + flags: MsrFlags::NONE, + }]) + .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..0fcd54597 --- /dev/null +++ b/src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs @@ -0,0 +1,267 @@ +/* +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) reset support. +//! +//! Reset is the isolation guarantee: on every snapshot restore Hyperlight +//! writes a captured baseline back to every MSR in the *reset set*, so no +//! guest-modified MSR state leaks across a restore. Access control (the +//! per-MSR allow list) is a separate, independent mechanism (see +//! [`crate::sandbox::SandboxConfiguration`]); allowing an MSR never removes +//! it from the reset set. +//! +//! This module holds the backend-agnostic data model plus the static table +//! that classifies the core MSRs Hyperlight knows how to capture and +//! restore. Backends (KVM and mshv) turn a set of indices into concrete +//! get/set batches. + +/// A single MSR captured for reset, together with the metadata that +/// decides how it is handled. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct MsrEntry { + /// The MSR index (as passed to `RDMSR`/`WRMSR`). + pub index: u32, + /// The captured value. + pub value: u64, + /// How the MSR is treated by capture/restore. + pub class: MsrClass, + /// Extra per-MSR handling flags. + pub flags: MsrFlags, +} + +/// How an MSR is treated by capture and restore. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[allow(dead_code)] // `ReadOnly`/`HostOwned` document the full taxonomy; not all are in the table yet. +pub(crate) enum MsrClass { + /// Ordinary readable/writable state. Captured into the baseline and + /// written back on restore. + Stateful, + /// Write-only command MSR (e.g. `PRED_CMD`, `FLUSH_CMD`). `RDMSR` + /// faults and it holds no state, so it is never captured or restored. + WriteOnlyCmd, + /// Read-only MSR. Never restored. + ReadOnly, + /// Owned by the host/hypervisor. Excluded from the reset set entirely. + HostOwned, +} + +/// Per-MSR handling flags. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub(crate) struct MsrFlags { + /// The value is host-specific (e.g. raw `TSC`, `TSC_AUX`) and must not + /// be written back in the generic batched restore. + /// + /// Currently dormant: no host-specific MSRs are in [`MSR_TABLE`] yet, + /// so nothing sets this. It exists so that adding `TSC`/`TSC_AUX` + /// later is a table edit, not a change to the capture/restore logic + /// (which already filters `host_specific` out on `apply`). + pub host_specific: bool, + /// Restore order matters relative to the rest of the batch. + pub ordering_sensitive: bool, +} + +impl MsrFlags { + /// No special handling. + pub const NONE: MsrFlags = MsrFlags { + host_specific: false, + ordering_sensitive: false, + }; +} + +/// A set of MSRs captured for reset. +pub(crate) type CommonMsrs = Vec; + +/// Host-side state that drives MSR reset for a single VM: the resolved +/// reset-set indices and the baseline captured at init. Used when a +/// snapshot carries no MSRs of its own (a from-binary snapshot). +#[derive(Debug, Clone)] +pub(crate) struct MsrResetState { + /// The indices that make up the reset set (core table entries present + /// on the host, plus the validated allow list). + pub indices: Vec, + /// The values captured at init, used as the reset target for + /// from-binary snapshots. + pub baseline: CommonMsrs, +} + +// Core stateful MSR indices. Numbers match on Intel and AMD. +// +// `EFER`/`APIC_BASE` are carried in `sregs` and restored there. +// `FS_BASE`/`GS_BASE` are segment-descriptor bases and also restored via +// `sregs`, so they are deliberately absent here. Raw `TSC` (0x10) is +// deliberately not reset: writing a stale captured value back would move +// the guest clock backward. Guest TSC manipulation is instead rolled back +// via `TSC_ADJUST`, which the CPU updates on a direct `TSC` write. +/// `IA32_SYSENTER_CS` +pub(crate) const MSR_SYSENTER_CS: u32 = 0x174; +/// `IA32_SYSENTER_ESP` +pub(crate) const MSR_SYSENTER_ESP: u32 = 0x175; +/// `IA32_SYSENTER_EIP` +pub(crate) const MSR_SYSENTER_EIP: u32 = 0x176; +/// `IA32_DEBUGCTL` +pub(crate) const MSR_DEBUGCTL: u32 = 0x1D9; +/// `IA32_PAT` +pub(crate) const MSR_PAT: u32 = 0x277; +/// `IA32_STAR` +pub(crate) const MSR_STAR: u32 = 0xC000_0081; +/// `IA32_LSTAR` +pub(crate) const MSR_LSTAR: u32 = 0xC000_0082; +/// `IA32_CSTAR` +pub(crate) const MSR_CSTAR: u32 = 0xC000_0083; +/// `IA32_FMASK` (`SFMASK`) +pub(crate) const MSR_SFMASK: u32 = 0xC000_0084; +/// `IA32_KERNEL_GS_BASE` +pub(crate) const MSR_KERNEL_GS_BASE: u32 = 0xC000_0102; +/// `IA32_SPEC_CTRL` +/// +/// A stateful MSR that Hyper-V passes through to the guest without an +/// intercept. It is captured and reset so a guest write does not survive a +/// restore. Included only when the host maps and supports it. +pub(crate) const MSR_SPEC_CTRL: u32 = 0x48; + +// CET (Control-flow Enforcement) MSRs. Stateful pass-through MSRs on +// Hyper-V that `msr_to_hv_reg_name` maps, so they are captured and reset +// like `SPEC_CTRL`. They are active only when CET is exposed to the guest, +// so backends include them only if they read back at init (KVM via its +// index list, mshv via a get probe). +/// `IA32_U_CET` +pub(crate) const MSR_U_CET: u32 = 0x6A0; +/// `IA32_S_CET` +pub(crate) const MSR_S_CET: u32 = 0x6A2; +/// `IA32_PL0_SSP` +pub(crate) const MSR_PL0_SSP: u32 = 0x6A4; +/// `IA32_PL1_SSP` +pub(crate) const MSR_PL1_SSP: u32 = 0x6A5; +/// `IA32_PL2_SSP` +pub(crate) const MSR_PL2_SSP: u32 = 0x6A6; +/// `IA32_PL3_SSP` +pub(crate) const MSR_PL3_SSP: u32 = 0x6A7; +/// `IA32_INTERRUPT_SSP_TABLE_ADDR` +pub(crate) const MSR_INTERRUPT_SSP_TABLE_ADDR: u32 = 0x6A8; +/// `IA32_XSS` (extended supervisor state mask). Selects which supervisor +/// state components `XSAVES`/`XRSTORS` manage (e.g. CET supervisor state, +/// processor trace). A guest with the feature exposed can set its bits and +/// the value is retained, so it must reset across a restore. +pub(crate) const MSR_XSS: u32 = 0xDA0; + +// TSC-related and MTRR MSRs. These are stateful pass-through MSRs a guest +// can freely write on a backend with no per-MSR deny filter (mshv and +// WHP), so their guest-written values would otherwise survive a restore. +// Reset rolls them back. Backends include only the ones that actually read +// back on the host (KVM via its index list; mshv/WHP via a capture probe), +// so an index the host does not model is skipped rather than failing init. +/// `IA32_TSC_ADJUST`. The architected TSC offset; a guest write here (or a +/// direct `WRMSR(IA32_TIME_STAMP_COUNTER)`, which updates it) shifts the +/// guest TSC. Reset restores the baseline offset. +pub(crate) const MSR_TSC_ADJUST: u32 = 0x3B; +/// `IA32_TSC_AUX` (the value `RDTSCP`/`RDPID` returns in `ECX`). +pub(crate) const MSR_TSC_AUX: u32 = 0xC000_0103; +/// `IA32_MTRR_DEF_TYPE` +pub(crate) const MSR_MTRR_DEF_TYPE: u32 = 0x2FF; +/// First fixed MTRR, `IA32_MTRR_FIX64K_00000`. Variable MTRRs run as +/// base/mask pairs starting at `IA32_MTRR_PHYSBASE0` (0x200): `PHYSBASE`n +/// at `0x200 + 2*n`, `PHYSMASK`n at `0x201 + 2*n`. +pub(crate) const MSR_MTRR_FIX64K_00000: u32 = 0x250; + +// Write-only command MSRs. Listed only so that an allow request for one +// produces a clear "not resettable" error rather than a confusing probe +// failure. +/// `IA32_PRED_CMD` +pub(crate) const MSR_PRED_CMD: u32 = 0x49; +/// `IA32_FLUSH_CMD` +pub(crate) const MSR_FLUSH_CMD: u32 = 0x10B; + +/// The static MSR classification table, keyed by index. +const MSR_TABLE: &[(u32, MsrClass, MsrFlags)] = &[ + (MSR_SYSENTER_CS, MsrClass::Stateful, MsrFlags::NONE), + (MSR_SYSENTER_ESP, MsrClass::Stateful, MsrFlags::NONE), + (MSR_SYSENTER_EIP, MsrClass::Stateful, MsrFlags::NONE), + (MSR_DEBUGCTL, MsrClass::Stateful, MsrFlags::NONE), + (MSR_PAT, MsrClass::Stateful, MsrFlags::NONE), + (MSR_STAR, MsrClass::Stateful, MsrFlags::NONE), + (MSR_LSTAR, MsrClass::Stateful, MsrFlags::NONE), + (MSR_CSTAR, MsrClass::Stateful, MsrFlags::NONE), + (MSR_SFMASK, MsrClass::Stateful, MsrFlags::NONE), + (MSR_KERNEL_GS_BASE, MsrClass::Stateful, MsrFlags::NONE), + (MSR_SPEC_CTRL, MsrClass::Stateful, MsrFlags::NONE), + (MSR_U_CET, MsrClass::Stateful, MsrFlags::NONE), + (MSR_S_CET, MsrClass::Stateful, MsrFlags::NONE), + (MSR_PL0_SSP, MsrClass::Stateful, MsrFlags::NONE), + (MSR_PL1_SSP, MsrClass::Stateful, MsrFlags::NONE), + (MSR_PL2_SSP, MsrClass::Stateful, MsrFlags::NONE), + (MSR_PL3_SSP, MsrClass::Stateful, MsrFlags::NONE), + ( + MSR_INTERRUPT_SSP_TABLE_ADDR, + MsrClass::Stateful, + MsrFlags::NONE, + ), + (MSR_XSS, MsrClass::Stateful, MsrFlags::NONE), + // TSC offset/aux. + (MSR_TSC_ADJUST, MsrClass::Stateful, MsrFlags::NONE), + (MSR_TSC_AUX, MsrClass::Stateful, MsrFlags::NONE), + // MTRRs: def type, variable base/mask pairs 0..=9, fixed ranges. + (MSR_MTRR_DEF_TYPE, MsrClass::Stateful, MsrFlags::NONE), + (0x200, MsrClass::Stateful, MsrFlags::NONE), // PHYSBASE0 + (0x201, MsrClass::Stateful, MsrFlags::NONE), // PHYSMASK0 + (0x202, MsrClass::Stateful, MsrFlags::NONE), // PHYSBASE1 + (0x203, MsrClass::Stateful, MsrFlags::NONE), // PHYSMASK1 + (0x204, MsrClass::Stateful, MsrFlags::NONE), // PHYSBASE2 + (0x205, MsrClass::Stateful, MsrFlags::NONE), // PHYSMASK2 + (0x206, MsrClass::Stateful, MsrFlags::NONE), // PHYSBASE3 + (0x207, MsrClass::Stateful, MsrFlags::NONE), // PHYSMASK3 + (0x208, MsrClass::Stateful, MsrFlags::NONE), // PHYSBASE4 + (0x209, MsrClass::Stateful, MsrFlags::NONE), // PHYSMASK4 + (0x20A, MsrClass::Stateful, MsrFlags::NONE), // PHYSBASE5 + (0x20B, MsrClass::Stateful, MsrFlags::NONE), // PHYSMASK5 + (0x20C, MsrClass::Stateful, MsrFlags::NONE), // PHYSBASE6 + (0x20D, MsrClass::Stateful, MsrFlags::NONE), // PHYSMASK6 + (0x20E, MsrClass::Stateful, MsrFlags::NONE), // PHYSBASE7 + (0x20F, MsrClass::Stateful, MsrFlags::NONE), // PHYSMASK7 + (0x210, MsrClass::Stateful, MsrFlags::NONE), // PHYSBASE8 + (0x211, MsrClass::Stateful, MsrFlags::NONE), // PHYSMASK8 + (0x212, MsrClass::Stateful, MsrFlags::NONE), // PHYSBASE9 + (0x213, MsrClass::Stateful, MsrFlags::NONE), // PHYSMASK9 + (MSR_MTRR_FIX64K_00000, MsrClass::Stateful, MsrFlags::NONE), // 0x250 + (0x258, MsrClass::Stateful, MsrFlags::NONE), // FIX16K_80000 + (0x259, MsrClass::Stateful, MsrFlags::NONE), // FIX16K_A0000 + (0x268, MsrClass::Stateful, MsrFlags::NONE), // FIX4K_C0000 + (0x269, MsrClass::Stateful, MsrFlags::NONE), // FIX4K_C8000 + (0x26A, MsrClass::Stateful, MsrFlags::NONE), // FIX4K_D0000 + (0x26B, MsrClass::Stateful, MsrFlags::NONE), // FIX4K_D8000 + (0x26C, MsrClass::Stateful, MsrFlags::NONE), // FIX4K_E0000 + (0x26D, MsrClass::Stateful, MsrFlags::NONE), // FIX4K_E8000 + (0x26E, MsrClass::Stateful, MsrFlags::NONE), // FIX4K_F0000 + (0x26F, MsrClass::Stateful, MsrFlags::NONE), // FIX4K_F8000 + (MSR_PRED_CMD, MsrClass::WriteOnlyCmd, MsrFlags::NONE), + (MSR_FLUSH_CMD, MsrClass::WriteOnlyCmd, MsrFlags::NONE), +]; + +/// Look up the static classification for an MSR index, if known. +pub(crate) fn classify_msr(index: u32) -> Option<(MsrClass, MsrFlags)> { + MSR_TABLE + .iter() + .find(|(i, _, _)| *i == index) + .map(|(_, class, flags)| (*class, *flags)) +} + +/// Iterate the core stateful reset-set indices. Callers filter this by +/// host availability before use. +pub(crate) fn core_reset_indices() -> impl Iterator { + MSR_TABLE + .iter() + .filter(|(_, class, _)| matches!(class, MsrClass::Stateful)) + .map(|(index, _, _)| *index) +} 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..5fc5b5335 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; @@ -33,8 +38,8 @@ use vmm_sys_util::eventfd::EventFd; #[cfg(gdb)] use crate::hypervisor::gdb::{DebugError, DebuggableVm}; use crate::hypervisor::regs::{ - CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, FP_CONTROL_WORD_DEFAULT, - MXCSR_DEFAULT, + CommonDebugRegs, CommonFpu, CommonMsrs, CommonRegisters, CommonSpecialRegisters, + FP_CONTROL_WORD_DEFAULT, MXCSR_DEFAULT, MsrClass, MsrEntry, MsrFlags, classify_msr, }; #[cfg(test)] use crate::hypervisor::virtual_machine::XSAVE_BUFFER_SIZE; @@ -110,6 +115,47 @@ pub(crate) struct KvmVm { static KVM: LazyLock> = LazyLock::new(|| Kvm::new().map_err(|e| CreateVmError::HypervisorNotAvailable(e.into()))); +/// The set of MSR indices the host KVM supports for get/set, as reported +/// by `KVM_GET_MSR_INDEX_LIST`. This tracks vendor, generation, and +/// microcode, and is fixed for the lifetime of the process, so it is +/// cached once. An empty set is used as a safe fallback if the list +/// cannot be queried (every membership check then fails 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; + +/// Coalesce a set of MSR indices into `(base, count)` ranges of contiguous +/// indices, minimising the number of KVM filter ranges. Input need not be +/// sorted or deduplicated. +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 +274,25 @@ impl KvmVm { } Ok(VcpuExit::MmioRead(addr, _)) => return Ok(VmExit::MmioRead(addr)), Ok(VcpuExit::MmioWrite(addr, _)) => return Ok(VmExit::MmioWrite(addr)), + // Filtered MSR access. See `run_vcpu_default` for the detailed + // explanation of the error=1 + immediate_exit handshake. + Ok(VcpuExit::X86Rdmsr(msr_exit)) => { + let msr_index = msr_exit.index; + *msr_exit.error = 1; + self.vcpu_fd.set_kvm_immediate_exit(1); + let _ = self.vcpu_fd.run(); + self.vcpu_fd.set_kvm_immediate_exit(0); + 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.vcpu_fd.set_kvm_immediate_exit(1); + let _ = self.vcpu_fd.run(); + self.vcpu_fd.set_kvm_immediate_exit(0); + return Ok(VmExit::MsrWrite { msr_index, value }); + } #[cfg(gdb)] Ok(VcpuExit::Debug(debug_exit)) => { return Ok(VmExit::Debug { @@ -275,6 +340,40 @@ 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)), + // KVM_EXIT_X86_RDMSR / KVM_EXIT_X86_WRMSR (KVM API §5, kvm_run structure): + // + // The "index" field tells userspace which MSR the guest wants to + // read/write. If the request was unsuccessful, userspace indicates + // that with a "1" in the "error" field. "This will inject a #GP + // into the guest when the VCPU is executed again." + // + // "for KVM_EXIT_IO, KVM_EXIT_MMIO, [...] KVM_EXIT_X86_RDMSR and + // KVM_EXIT_X86_WRMSR the corresponding operations are complete + // (and guest state is consistent) only after userspace has + // re-entered the kernel with KVM_RUN." + // + // We set error=1 and then re-run with `immediate_exit` to let KVM + // inject the #GP without executing further guest code. From the + // kvm_run docs: "[immediate_exit] is polled once when KVM_RUN + // starts; if non-zero, KVM_RUN exits immediately, returning + // -EINTR." + Ok(VcpuExit::X86Rdmsr(msr_exit)) => { + let msr_index = msr_exit.index; + *msr_exit.error = 1; + self.vcpu_fd.set_kvm_immediate_exit(1); + let _ = self.vcpu_fd.run(); + self.vcpu_fd.set_kvm_immediate_exit(0); + 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.vcpu_fd.set_kvm_immediate_exit(1); + let _ = self.vcpu_fd.run(); + self.vcpu_fd.set_kvm_immediate_exit(0); + Ok(VmExit::MsrWrite { msr_index, value }) + } #[cfg(gdb)] Ok(VcpuExit::Debug(debug_exit)) => Ok(VmExit::Debug { dr6: debug_exit.dr6, @@ -292,6 +391,193 @@ impl KvmVm { ))), } } + + /// Configure guest MSR access. Installs a deny-by-default KVM MSR + /// filter that lets the guest read/write only the `allowed` MSRs, and + /// validates that each allowed MSR is resettable. Every other MSR + /// access traps to userspace, where the run loop injects a `#GP` and + /// poisons the sandbox. + /// + /// Requires KVM_CAP_X86_USER_SPACE_MSR and KVM_CAP_X86_MSR_FILTER. + /// + /// ## Why the reset set (core ∪ allow) is complete + /// + /// The reset set is the fixed core table unioned with the allow list, + /// not an enumerate-and-expand of everything the CPU exposes. That is + /// sound precisely because of the deny-by-default filter installed + /// here: the guest can only execute `WRMSR` against an MSR in + /// `allowed`; any other write traps and poisons the sandbox before it + /// takes effect. So the set of MSRs a guest can actually mutate is a + /// subset of `allowed`, which is itself in the reset set. The core + /// table is pure defence-in-depth for state the loader/host may have + /// touched. + /// + /// This argument relies on the guest having no *indirect* way to make + /// the CPU write MSR-like state that escapes the filter: + /// * `IA32_DEBUGCTL` (0x1D9, which enables LBR/BTS last-branch + /// recording) is not in the default allow list, so the guest cannot + /// turn on branch recording that would persist across a restore. + /// * The APIC is left in xAPIC (MMIO) mode; x2APIC MSRs (0x800–0x8FF) + /// are denied, so the guest cannot reach APIC state through the MSR + /// space either. + /// + /// Allowing such an MSR would require also adding it (and any state it + /// enables) to the reset set — hence validation restricts the allow + /// list to plain stateful MSRs. + 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); + } + + // Validate every allowed MSR before installing the filter, so a + // non-resettable allow request is a hard error at creation, never + // a silent MSR that cannot be reset. + 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()))?; + + // Build the deny-by-default filter with one allow range per group + // of contiguous allowed MSRs. + let groups = coalesce_msr_ranges(allowed); + if groups.len() > KVM_MSR_FILTER_MAX_RANGES { + return Err(CreateVmError::TooManyMsrRanges(groups.len())); + } + + // The bitmaps must outlive the set_msr_filter call, so build them + // up front. A set bit allows the MSR; all MSRs in a coalesced + // group are allowed, so every bit is set. + 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(); + + // At least one range is required when using + // KVM_MSR_FILTER_DEFAULT_DENY. With no allowed MSRs, install a + // single dummy all-deny 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(()) + } + + /// Validate that an allowed MSR is resettable: present on the host, + /// classified as stateful (not write-only/read-only/host-owned), and + /// readable then writable back to its own value. + fn validate_allowed_msr(&self, msr: u32) -> std::result::Result<(), CreateVmError> { + if let Some((class, _)) = classify_msr(msr) + && class != MsrClass::Stateful + { + return Err(CreateVmError::MsrNotAllowable { + msr, + reason: "MSR is not stateful (write-only, read-only, or host-owned)".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(()) + } + + /// Read a single MSR from the vCPU (host-side; bypasses the guest MSR + /// 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) + } + + /// Write a single MSR to the vCPU (host-side; bypasses the guest MSR + /// 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 +689,76 @@ impl VirtualMachine for KvmVm { Ok(()) } + fn capture_msrs(&self, indices: &[u32]) -> std::result::Result { + 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| { + let (class, flags) = + classify_msr(e.index).unwrap_or((MsrClass::Stateful, MsrFlags::NONE)); + MsrEntry { + index: e.index, + value: e.data, + class, + flags, + } + }) + .collect()) + } + + fn apply_msrs(&self, msrs: &CommonMsrs) -> std::result::Result<(), RegisterError> { + // Only replay stateful, non-host-specific MSRs. Host-specific and + // non-stateful values captured from the host must not be written + // back into a restored guest. + let entries: Vec = msrs + .iter() + .filter(|e| !e.flags.host_specific && e.class == MsrClass::Stateful) + .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 diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs index dac344711..31480987b 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs @@ -21,6 +21,8 @@ use tracing::{Span, instrument}; #[cfg(gdb)] use crate::hypervisor::gdb::DebugError; +#[cfg(target_arch = "x86_64")] +use crate::hypervisor::regs::CommonMsrs; use crate::hypervisor::regs::{ CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, }; @@ -139,6 +141,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", any(kvm, target_os = "windows")))] + MsrRead(u32), + /// The vCPU tried to write to the given MSR with the given value + #[cfg(all(target_arch = "x86_64", any(kvm, target_os = "windows")))] + 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 +191,17 @@ pub enum CreateVmError { HypervisorNotAvailable(HypervisorError), #[error("Initialize VM failed: {0}")] InitializeVm(HypervisorError), + #[cfg(all(kvm, target_arch = "x86_64"))] + #[error( + "KVM MSR filtering not supported (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(all(kvm, target_arch = "x86_64"))] + #[error("Too many allowed MSR filter ranges: {0} (max 16)")] + TooManyMsrRanges(usize), #[error("Set Partition Property failed: {0}")] SetPartitionProperty(HypervisorError), #[cfg(target_os = "windows")] @@ -241,6 +260,26 @@ 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(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 +398,23 @@ pub(crate) trait VirtualMachine: Debug + Send { #[allow(dead_code)] fn set_debug_regs(&self, drs: &CommonDebugRegs) -> std::result::Result<(), RegisterError>; + /// Capture the given MSRs from the vCPU into a reset set. + /// + /// Implemented on KVM and mshv; the default is unsupported. Callers + /// must only invoke this on a backend that supports MSR reset. + #[cfg(target_arch = "x86_64")] + fn capture_msrs(&self, _indices: &[u32]) -> std::result::Result { + Err(RegisterError::MsrsUnsupported) + } + /// Write the given MSRs back to the vCPU. + /// + /// Implemented on KVM and mshv; the default is unsupported. Callers + /// must only invoke this on a backend that supports MSR reset. + #[cfg(target_arch = "x86_64")] + fn apply_msrs(&self, _msrs: &CommonMsrs) -> std::result::Result<(), RegisterError> { + Err(RegisterError::MsrsUnsupported) + } + /// Get xsave #[allow(dead_code)] #[cfg(not(target_arch = "aarch64"))] 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..0c628a9fe 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 @@ -26,13 +26,13 @@ use mshv_bindings::LapicState; #[cfg(gdb)] use mshv_bindings::{DebugRegisters, hv_message_type_HVMSG_X64_EXCEPTION_INTERCEPT}; use mshv_bindings::{ - FloatingPointUnit, HV_X64_REGISTER_CLASS_IP, SpecialRegisters, StandardRegisters, XSave, + FloatingPointUnit, HV_X64_REGISTER_CLASS_IP, Msrs, SpecialRegisters, StandardRegisters, XSave, hv_message_type, hv_message_type_HVMSG_GPA_INTERCEPT, hv_message_type_HVMSG_UNMAPPED_GPA, 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, + mshv_user_mem_region, msr_entry, msr_to_hv_reg_name, }; #[cfg(feature = "hw-interrupts")] use mshv_bindings::{ @@ -49,8 +49,8 @@ use tracing_opentelemetry::OpenTelemetrySpanExt; #[cfg(gdb)] use crate::hypervisor::gdb::{DebugError, DebuggableVm}; use crate::hypervisor::regs::{ - CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, FP_CONTROL_WORD_DEFAULT, - MXCSR_DEFAULT, + CommonDebugRegs, CommonFpu, CommonMsrs, CommonRegisters, CommonSpecialRegisters, + FP_CONTROL_WORD_DEFAULT, MXCSR_DEFAULT, MsrClass, MsrEntry, MsrFlags, classify_msr, }; #[cfg(test)] use crate::hypervisor::virtual_machine::XSAVE_BUFFER_SIZE; @@ -77,6 +77,34 @@ pub(crate) fn is_hypervisor_present() -> bool { } } +/// Whether this host can get and set the given MSR. +/// +/// mshv has no host-probed index list like KVM's +/// `KVM_GET_MSR_INDEX_LIST`. It reaches an MSR only if the crate can map +/// its index to an `hv_register_name`, so a successful mapping is the +/// availability oracle. An index that does not map returns `EINVAL` from +/// `get_msrs`/`set_msrs` and is excluded from the reset set. +/// +/// MSR reset completeness on mshv: mshv installs no per-MSR deny filter, +/// so a guest can write the architectural pass-through MSRs directly with +/// no intercept. The reset set covers every stateful pass-through MSR +/// Hyperlight can map and restore: the core syscall, sysenter, and base +/// MSRs, `KERNEL_GS_BASE`, `SPEC_CTRL`, and the CET MSRs (`U_CET`, +/// `S_CET`, `PL0_SSP`-`PL3_SSP`, `INTERRUPT_SSP_TABLE_ADDR`) where CET is +/// active. A guest write to any of these is rolled back on restore even +/// though nothing denied the write. `FS_BASE`/`GS_BASE` reset through +/// `sregs`. +/// +/// The remaining pass-through classes (PMU, AMX/`XFD`, FRED, and +/// architectural LBR) have no `hv_register_name` mapping, so they cannot +/// be reset. Completeness holds because Hyperlight's minimal partition +/// enables none of those features, so a guest `RDMSR`/`WRMSR` to them +/// faults and it cannot persist any state. `test_mshv_unresettable_msr_classes_fault` +/// asserts this on the host. +pub(crate) fn host_supports_msr(index: u32) -> bool { + msr_to_hv_reg_name(index).is_ok() +} + /// A MSHV implementation of a single-vcpu VM #[derive(Debug)] pub(crate) struct MshvVm { @@ -151,6 +179,38 @@ impl MshvVm { timer: None, }) } + + /// Validate that every MSR the user allowed can be reset. + /// + /// mshv has no per-MSR access filter, so the allow list does not gate + /// guest access. It only widens the reset set, so each allowed MSR + /// must be one Hyperlight can capture and restore: mappable to an + /// `hv_register_name` and stateful (not a write-only command, + /// read-only, or host-owned MSR). A request that cannot be reset is a + /// hard error at VM creation, never a silently unreset MSR. + pub(crate) fn validate_allowed_msrs( + &self, + allowed: &[u32], + ) -> std::result::Result<(), CreateVmError> { + for &msr in allowed { + if let Some((class, _)) = classify_msr(msr) + && class != MsrClass::Stateful + { + return Err(CreateVmError::MsrNotAllowable { + msr, + reason: "MSR is not stateful (write-only, read-only, or host-owned)" + .to_string(), + }); + } + if !host_supports_msr(msr) { + return Err(CreateVmError::MsrNotAllowable { + msr, + reason: "MSR has no hv_register_name mapping on this host".to_string(), + }); + } + } + Ok(()) + } } impl VirtualMachine for MshvVm { @@ -432,6 +492,72 @@ impl VirtualMachine for MshvVm { Ok(()) } + fn capture_msrs(&self, indices: &[u32]) -> std::result::Result { + if indices.is_empty() { + return Ok(Vec::new()); + } + let entries: Vec = indices + .iter() + .map(|&index| msr_entry { + index, + ..Default::default() + }) + .collect(); + let mut msrs = + Msrs::from_entries(&entries).map_err(|e| RegisterError::MsrBuild(format!("{e:?}")))?; + // `get_msrs` maps every index through `msr_to_hv_reg_name` and + // fails the whole batch with `EINVAL` if any index is unmappable. + // Callers filter indices with `host_supports_msr` first, so a + // failure here is a real hypervisor error. + self.vcpu_fd + .get_msrs(&mut msrs) + .map_err(|e| RegisterError::GetMsrs(e.into()))?; + Ok(msrs + .as_slice() + .iter() + .map(|e| { + let (class, flags) = + classify_msr(e.index).unwrap_or((MsrClass::Stateful, MsrFlags::NONE)); + MsrEntry { + index: e.index, + value: e.data, + class, + flags, + } + }) + .collect()) + } + + fn apply_msrs(&self, msrs: &CommonMsrs) -> std::result::Result<(), RegisterError> { + // Only replay stateful, non-host-specific MSRs. Host-specific and + // non-stateful values captured from the host must not be written + // back into a restored guest. + let entries: Vec = msrs + .iter() + .filter(|e| !e.flags.host_specific && e.class == MsrClass::Stateful) + .map(|e| msr_entry { + index: e.index, + data: e.value, + ..Default::default() + }) + .collect(); + if entries.is_empty() { + return Ok(()); + } + let mshv_msrs = + Msrs::from_entries(&entries).map_err(|e| RegisterError::MsrBuild(format!("{e:?}")))?; + // `set_msrs` batches every register into one rep hypercall and + // returns `Ok(0)` on success, so there is no count to check. A + // mid-rep rejection leaves earlier registers committed and returns + // `EINTR`: a partial apply, not all-or-nothing. The caller treats + // any error as fatal and poisons the sandbox, so a partially reset + // vCPU never runs the guest again. + self.vcpu_fd + .set_msrs(&mshv_msrs) + .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..5995dbefb 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs @@ -32,10 +32,13 @@ use windows_result::HRESULT; #[cfg(gdb)] 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, + Align16, CommonDebugRegs, CommonFpu, CommonMsrs, CommonRegisters, CommonSpecialRegisters, + FP_CONTROL_WORD_DEFAULT, MSR_CSTAR, MSR_INTERRUPT_SSP_TABLE_ADDR, MSR_KERNEL_GS_BASE, + MSR_LSTAR, 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_U_CET, + MXCSR_DEFAULT, MsrClass, MsrEntry, MsrFlags, 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, + WHP_SREGS_NAMES_LEN, classify_msr, }; use crate::hypervisor::surrogate_process::SurrogateProcess; use crate::hypervisor::surrogate_process_manager::{ @@ -73,6 +76,98 @@ pub(crate) fn is_hypervisor_present() -> bool { } } +/// Map an MSR index to its WHP named register, if WHP models it as one. +/// +/// WHP has no MSR index enumeration; it exposes MSRs as named +/// `WHV_REGISTER_NAME` values read/written through +/// `WHvGet/SetVirtualProcessorRegisters`. A successful mapping is the +/// availability oracle: an index with no register name is not resettable +/// on WHP and is excluded from the reset set. +/// +/// MSR reset completeness on WHP: WHP shares the Hyper-V hypervisor with +/// mshv, so the same architectural pass-through set applies, but WHP has no +/// per-MSR deny filter (the hypervisor refuses to intercept an MSR it +/// implements, so `MsrActionList` / `UnimplementedMsrAction` are rejected +/// by `WHvSetupPartition`). The reset set must therefore cover *every* +/// guest-writable, retained MSR by construction. A guest MSR access falls +/// into one of these buckets, none of which can leak across a restore: +/// +/// - Mapped here → captured and written back on restore (syscall, sysenter, +/// `KERNEL_GS_BASE`, `PAT`, `SPEC_CTRL`, CET, the MTRRs, `TSC_ADJUST`, +/// `TSC_AUX`). A direct `WRMSR(TSC)` is rolled back via `TSC_ADJUST`, +/// which the CPU updates on such a write, so raw `TSC` is not reset. +/// - Modeled but not mapped (e.g. `DEBUGCTL`): the hypervisor validates the +/// write and drops the unsupported bits, so it reads back unchanged — no +/// retained state to leak. +/// - Not modeled (PMU, AMX/`XFD`, FRED, architectural LBR): a guest +/// `RDMSR`/`WRMSR` faults with `#GP`, so nothing persists. +fn msr_to_whv_register_name(index: u32) -> Option { + Some(match index { + 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 offset/aux. + 0x3B => WHvX64RegisterTscAdjust, + 0xC000_0103 => WHvX64RegisterTscAux, + // 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, + 0x250 => WHvX64RegisterMsrMtrrFix64k00000, + 0x258 => WHvX64RegisterMsrMtrrFix16k80000, + 0x259 => WHvX64RegisterMsrMtrrFix16kA0000, + 0x268 => WHvX64RegisterMsrMtrrFix4kC0000, + 0x269 => WHvX64RegisterMsrMtrrFix4kC8000, + 0x26A => WHvX64RegisterMsrMtrrFix4kD0000, + 0x26B => WHvX64RegisterMsrMtrrFix4kD8000, + 0x26C => WHvX64RegisterMsrMtrrFix4kE0000, + 0x26D => WHvX64RegisterMsrMtrrFix4kE8000, + 0x26E => WHvX64RegisterMsrMtrrFix4kF0000, + 0x26F => WHvX64RegisterMsrMtrrFix4kF8000, + _ => return None, + }) +} + +/// Whether this host can get and set the given MSR through a WHP named +/// register. Used to bound the reset set and validate the allow list. +pub(crate) fn host_supports_msr(index: u32) -> bool { + msr_to_whv_register_name(index).is_some() +} + /// 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 +272,25 @@ impl WhpVm { #[cfg(feature = "hw-interrupts")] Self::enable_lapic_emulation(p)?; + // NOTE: A per-MSR deny filter cannot bound an *implemented* MSR + // on WHP/Hyper-V, so MSR isolation relies on reset alone. + // + // Both MSR-exit mechanisms only ever intercept MSRs the + // hypervisor treats as *unimplemented* for the guest: + // - `ExtendedVmExits.X64MsrExit` + `X64MsrExitBitmap` + // `UnhandledMsrs` installs the general `HvInterceptTypeX64Msr` + // intercept, which fires only for unimplemented MSRs (those + // already `#GP` without it). + // - `MsrActionList` (per-index) calls `ImInstallMsrIndexIntercept`, + // which returns `HV_STATUS_ACCESS_DENIED` (0xC0350006) for any + // MSR whose child access is not `MsrAccessUnimplemented`. + // - `UnimplementedMsrAction = Exit` is rejected by + // `WHvSetupPartition` (0x80070057) on the versions tested. + // Implemented MSRs such as `DEBUGCTL` are therefore un-interceptable, + // and WHP exposes no named register for them, so they cannot be + // captured/reset either. The `WHvRunVpExitReasonX64MsrAccess` arm in + // `run_vcpu` is kept ready should a future WHP expose one. + WHvSetupPartition(p).map_err(|e| CreateVmError::InitializeVm(e.into()))?; WHvCreateVirtualProcessor(p, 0, 0) .map_err(|e| CreateVmError::CreateVcpuFd(e.into()))?; @@ -226,6 +340,35 @@ impl WhpVm { ) } } + + /// Validate that every MSR the user allowed can be reset on WHP: it + /// must be classified as stateful (not a write-only command, + /// read-only, or host-owned MSR) and map to a WHP named register. A + /// request that cannot be reset is a hard error at VM creation, never a + /// silently unreset MSR. + pub(crate) fn validate_allowed_msrs( + &self, + allowed: &[u32], + ) -> std::result::Result<(), CreateVmError> { + for &msr in allowed { + if let Some((class, _)) = classify_msr(msr) + && class != MsrClass::Stateful + { + return Err(CreateVmError::MsrNotAllowable { + msr, + reason: "MSR is not stateful (write-only, read-only, or host-owned)" + .to_string(), + }); + } + if !host_supports_msr(msr) { + return Err(CreateVmError::MsrNotAllowable { + msr, + reason: "MSR has no WHP register mapping on this host".to_string(), + }); + } + } + Ok(()) + } } impl VirtualMachine for WhpVm { @@ -456,6 +599,23 @@ impl VirtualMachine for WhpVm { } return Ok(VmExit::Halt()); } + WHvRunVpExitReasonX64MsrAccess => { + // A trapped MSR access. Reported as a violation so the + // caller poisons the sandbox (the guest never resumes, + // so there is no need to inject `#GP` and re-enter). + // Currently unreachable: no MSR deny filter is enabled + // (see `WhpVm::new`). Kept ready for a WHP version that + // exposes an implemented-MSR intercept. + let ctx = unsafe { exit_context.Anonymous.MsrAccess }; + let msr_index = ctx.MsrNumber; + let is_write = unsafe { ctx.AccessInfo.Anonymous._bitfield } & 1 != 0; + if is_write { + // WRMSR value is EDX:EAX. + let value = ((ctx.Rdx as u32 as u64) << 32) | (ctx.Rax as u32 as u64); + return Ok(VmExit::MsrWrite { msr_index, value }); + } + return Ok(VmExit::MsrRead(msr_index)); + } WHvRunVpExitReasonMemoryAccess => { let gpa = unsafe { exit_context.Anonymous.MemoryAccess.Gpa }; let access_info = unsafe { @@ -667,6 +827,64 @@ impl VirtualMachine for WhpVm { } } + fn capture_msrs(&self, indices: &[u32]) -> std::result::Result { + if indices.is_empty() { + return Ok(Vec::new()); + } + // Map each index to its WHP register name. Callers filter indices + // with `host_supports_msr` first, so an unmapped index is a bug. + let names: Vec = indices + .iter() + .map(|&i| msr_to_whv_register_name(i).ok_or(RegisterError::MsrsUnsupported)) + .collect::>()?; + let mut values: Vec> = + vec![unsafe { std::mem::zeroed() }; names.len()]; + 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)| { + let (class, flags) = + classify_msr(index).unwrap_or((MsrClass::Stateful, MsrFlags::NONE)); + MsrEntry { + index, + value: unsafe { v.0.Reg64 }, + class, + flags, + } + }) + .collect()) + } + + fn apply_msrs(&self, msrs: &CommonMsrs) -> std::result::Result<(), RegisterError> { + // Only replay stateful, non-host-specific MSRs. Host-specific and + // non-stateful values captured from the host must not be written + // back into a restored guest. + let regs: Vec<(WHV_REGISTER_NAME, Align16)> = msrs + .iter() + .filter(|e| !e.flags.host_specific && e.class == MsrClass::Stateful) + .filter_map(|e| { + msr_to_whv_register_name(e.index) + .map(|name| (name, Align16(WHV_REGISTER_VALUE { Reg64: e.value }))) + }) + .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/sandbox/config.rs b/src/hyperlight_host/src/sandbox/config.rs index f12387a0b..3c4110ab7 100644 --- a/src/hyperlight_host/src/sandbox/config.rs +++ b/src/hyperlight_host/src/sandbox/config.rs @@ -74,6 +74,16 @@ pub struct SandboxConfiguration { interrupt_vcpu_sigrtmin_offset: u8, /// How much writable memory to offer the guest scratch_size: usize, + /// MSRs (Model Specific Registers) the guest is allowed to read and + /// write. Empty by default: the guest is denied all MSR access. Any + /// allowed MSR is still reset to its captured baseline across a + /// snapshot restore. Stored as a fixed-size array (with a separate + /// count) so that this struct stays `Copy` and FFI-compatible. + #[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 +103,9 @@ impl SandboxConfiguration { pub const DEFAULT_HEAP_SIZE: u64 = 131072; /// The default size of the scratch region pub const DEFAULT_SCRATCH_SIZE: usize = 0x48000; + /// The maximum number of MSRs that can be added to the guest allow list. + #[cfg(target_arch = "x86_64")] + pub const MAX_ALLOWED_MSRS: usize = 64; #[allow(clippy::too_many_arguments)] /// Create a new configuration for a sandbox with the given sizes. @@ -118,6 +131,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 +176,50 @@ impl SandboxConfiguration { self.interrupt_vcpu_sigrtmin_offset } + /// Allow the guest to read and write the given MSR (Model Specific + /// Register). By default all MSR access is denied for security + /// reasons; a denied access faults the guest and fails the call. + /// + /// An allowed MSR is still captured and reset to its baseline across + /// snapshot restores, so allowing an MSR does not weaken restore + /// isolation. The MSR must be resettable (readable and writable back + /// to its baseline); a non-resettable MSR is rejected at sandbox + /// creation. + /// + /// Adding an MSR that is already allowed, or adding more than + /// [`Self::MAX_ALLOWED_MSRS`] distinct MSRs, is a no-op. This setting + /// only applies when using KVM or mshv. + #[cfg(target_arch = "x86_64")] + #[instrument(skip_all, parent = Span::current(), level= "Trace")] + pub fn allow_msr(&mut self, index: u32) -> &mut Self { + if self.allowed_msrs[..self.allowed_msrs_count].contains(&index) { + return self; + } + if self.allowed_msrs_count < Self::MAX_ALLOWED_MSRS { + self.allowed_msrs[self.allowed_msrs_count] = index; + self.allowed_msrs_count += 1; + } + self + } + + /// Allow the guest to read and write each of the given MSRs. See + /// [`Self::allow_msr`]. + #[cfg(target_arch = "x86_64")] + #[instrument(skip_all, parent = Span::current(), level= "Trace")] + pub fn allow_msrs(&mut self, indices: &[u32]) -> &mut Self { + for &index in indices { + self.allow_msr(index); + } + self + } + + /// Get the MSRs the guest is allowed to read and write. + #[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. /// diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 6f9e87e17..6e1648ce3 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -304,6 +304,16 @@ impl MultiUseSandbox { crate::hypervisor::hyperlight_vm::HyperlightVmError::Restore(e.into()), ) })?; + + // Restore the snapshot's reset-set MSRs so the resumed guest + // sees the same MSR state. A no-op on backends without MSR + // reset. + #[cfg(target_arch = "x86_64")] + vm.restore_msrs(snapshot.msrs()).map_err(|e| { + crate::HyperlightError::HyperlightVmError( + crate::hypervisor::hyperlight_vm::HyperlightVmError::Restore(e), + ) + })?; } #[cfg(gdb)] @@ -399,6 +409,18 @@ impl MultiUseSandbox { entrypoint, host_functions, )?; + #[cfg_attr(not(target_arch = "x86_64"), allow(unused_mut))] + let mut memory_snapshot = memory_snapshot; + // Capture reset-set MSRs so they are restored alongside memory and + // sregs. `None` on backends without MSR reset. + #[cfg(target_arch = "x86_64")] + { + let msrs = self + .vm + .get_snapshot_msrs() + .map_err(|e| HyperlightError::HyperlightVmError(e.into()))?; + memory_snapshot.set_msrs(msrs); + } let snapshot = Arc::new(memory_snapshot); self.snapshot = Some(snapshot.clone()); Ok(snapshot) @@ -543,6 +565,15 @@ impl MultiUseSandbox { HyperlightVmError::Restore(e) })?; + // Reset the reset-set MSRs to the snapshot's captured values (or + // the init baseline for from-binary snapshots). A no-op on + // backends without MSR reset. + #[cfg(target_arch = "x86_64")] + self.vm.restore_msrs(snapshot.msrs()).map_err(|e| { + self.poisoned = true; + HyperlightVmError::Restore(e) + })?; + self.vm.set_stack_top(snapshot.stack_top_gva()); self.vm.set_entrypoint(snapshot.entrypoint()); @@ -2720,6 +2751,1027 @@ mod tests { ); } + #[test] + #[cfg(all(kvm, target_arch = "x86_64"))] + fn test_msr_access_denied_poisons_and_allow_override() { + 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 + + // RDMSR should be intercepted + 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()); + + // Restore before next call + sbox.restore(snapshot.clone()).unwrap(); + + // WRMSR should be intercepted + 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()); + + // Also verify that MSR access works when explicitly allowed, and + // that the allowed MSR is still reset across a snapshot restore. + let mut cfg = SandboxConfiguration::default(); + cfg.allow_msr(0xC000_0102); // IA32_KERNEL_GS_BASE + + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(cfg), + ) + .unwrap() + .evolve() + .unwrap(); + + let msr_index: u32 = 0xC000_0102; // IA32_KERNEL_GS_BASE + let value: u64 = 0x5; + + // Snapshot the pristine (post-init) state so we can restore to it. + let baseline_snapshot = sbox.snapshot().unwrap(); + + sbox.call::<()>("WriteMSR", (msr_index, value)).unwrap(); + let read_value: u64 = sbox.call("ReadMSR", msr_index).unwrap(); + assert_eq!(read_value, value); + + // Restoring the baseline must reset the allowed MSR back to its + // captured value, so the guest no longer sees the value it wrote. + sbox.restore(baseline_snapshot).unwrap(); + let read_value: u64 = sbox.call("ReadMSR", msr_index).unwrap(); + assert_ne!( + read_value, value, + "allowed MSR should be reset to baseline across restore" + ); + } + + /// Allowing an MSR that is not resettable (here a write-only command + /// MSR) must be a hard error at sandbox creation rather than silently + /// admitting an MSR that cannot be reset. Backend-agnostic: every + /// backend validates the allow list at creation, so this runs on + /// whichever hypervisor the CI runner provides. + #[test] + #[cfg(target_arch = "x86_64")] + fn test_allow_non_resettable_msr_fails_creation() { + let mut cfg = SandboxConfiguration::default(); + cfg.allow_msr(0x49); // 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!( + format!("{err:?}").to_lowercase().contains("msr"), + "expected an MSR validation error, got: {err:?}" + ); + } + + /// Multiple allowed MSRs (including non-contiguous indices that + /// exercise range coalescing) are all guest-accessible and all reset + /// across a restore. + #[test] + #[cfg(all(kvm, target_arch = "x86_64"))] + fn test_multiple_allowed_msrs_reset_across_restore() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + match get_available_hypervisor() { + Some(HypervisorType::Kvm) => {} + _ => { + return; + } + } + + // A contiguous trio (SYSENTER_CS/ESP/EIP) plus a distant index + // (KERNEL_GS_BASE) so more than one filter range is built. These + // accept arbitrary (canonical) values, unlike STAR/LSTAR. + let msrs: [u32; 4] = [0x174, 0x175, 0x176, 0xC000_0102]; + let mut cfg = SandboxConfiguration::default(); + cfg.allow_msrs(&msrs); + + 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" + ); + } + } + + /// Explicit no-leak assertion: a value a guest writes to an allowed + /// MSR must not survive a restore to a baseline snapshot taken before + /// the write. Backend-agnostic: allowing the MSR lets the guest write + /// it on every backend (KVM permits it through the filter; mshv/WHP + /// have no filter), and all three roll it back on 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_msr(msr_index); + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(cfg), + ) + .unwrap() + .evolve() + .unwrap(); + + // Baseline taken before the guest touches the MSR. + 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" + ); + + // Guest plants the sentinel, then we restore the pre-write baseline. + 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(); + + // The sentinel must be gone; the MSR is back at its baseline. + 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"); + } + + /// A guest `WRMSR` to an MSR that is not on the allow list must not + /// succeed and must render the sandbox unusable. Covers + /// `IA32_DEBUGCTL` (whose LBR/BTS bits would otherwise let a guest + /// enable branch recording that escapes the reset set) and an x2APIC + /// MSR — the two blind spots the core-∪-allow completeness argument + /// depends on being unreachable. + /// + /// `DEBUGCTL` is a normal filtered MSR, so the deny-by-default filter + /// turns the write into an injected `#GP` reported as + /// [`HyperlightError::MsrWriteViolation`]. An x2APIC MSR is + /// architecturally inaccessible while the APIC is in xAPIC mode, so + /// the CPU raises `#GP` directly (surfaced as `GuestAborted`) before + /// the filter is consulted. Either way the guest cannot mutate the + /// state and the sandbox is poisoned. + #[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; + } + + // IA32_DEBUGCTL is a filtered MSR; IA32_X2APIC_APICID (0x800) is an + // x2APIC MSR that faults directly in xAPIC mode. + 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 { + // Filter violation or a direct guest #GP are both acceptable + // — the point is the write did not take effect. + assert!( + matches!( + &result, + Err(HyperlightError::MsrWriteViolation(idx, _)) if *idx == msr_index + ) || matches!(&result, Err(HyperlightError::GuestAborted(_, _))), + "WRMSR 0x{msr_index:X}: expected the write to be rejected, got: {result:?}" + ); + } + assert!( + sbox.poisoned(), + "sandbox should be poisoned after a denied WRMSR to 0x{msr_index:X}" + ); + } + } + + /// Completeness sweep over the full host-supported MSR index list + /// (`KVM_GET_MSR_INDEX_LIST`), not just the handful in the static + /// table. For every host MSR that accepts a host-side write, planting + /// a sentinel and then applying the captured baseline must restore the + /// original value exactly — proving capture/restore is faithful across + /// the whole index space. + #[test] + #[cfg(all(kvm, target_arch = "x86_64"))] + fn test_msr_reset_sweep_over_host_index_list() { + use crate::hypervisor::virtual_machine::kvm::host_msr_indices; + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Kvm)) { + return; + } + + let sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + let mut indices: Vec = host_msr_indices().iter().copied().collect(); + indices.sort_unstable(); + assert!( + !indices.is_empty(), + "host must report a non-empty MSR index list" + ); + + let mut swept = 0usize; + let mut mutated = 0usize; + for &index in &indices { + // Host-side get is not subject to the guest filter. Skip any + // MSR that cannot be read in isolation. + let Ok(base) = sbox.vm.capture_msrs_for_test(&[index]) else { + continue; + }; + let base_val = base[0].value; + swept += 1; + + // Flip the low bit; this keeps canonical MSRs canonical, so it + // only fails on MSRs that reject arbitrary values (skipped). + let sentinel = base_val ^ 0x1; + if !sbox.vm.try_set_msr_for_test(index, sentinel) { + continue; + } + let after = match sbox.vm.capture_msrs_for_test(&[index]) { + Ok(v) => v[0].value, + Err(_) => { + let _ = sbox.vm.try_set_msr_for_test(index, base_val); + continue; + } + }; + if after != sentinel { + // MSR silently ignored/masked the write; not a meaningful + // reset target, so restore and skip. + let _ = sbox.vm.try_set_msr_for_test(index, base_val); + continue; + } + + // Apply the captured baseline back and confirm it sticks. + assert!( + sbox.vm.try_set_msr_for_test(index, base_val), + "restoring baseline of MSR 0x{index:X} failed" + ); + let restored = sbox.vm.capture_msrs_for_test(&[index]).unwrap()[0].value; + assert_eq!( + restored, base_val, + "MSR 0x{index:X} not restored to baseline" + ); + mutated += 1; + } + + assert!(swept > 0, "sweep should have read at least one host MSR"); + assert!( + mutated > 0, + "sweep should have exercised at least one writable host MSR" + ); + } + + /// A partial `SET_MSRS` (short count) during restore must fail closed: + /// the sandbox is poisoned and refuses further guest calls. Driven by + /// doctoring a snapshot with a non-canonical MSR value that KVM + /// rejects mid-batch. + #[test] + #[cfg(all(kvm, target_arch = "x86_64"))] + fn test_partial_set_msrs_poisons_sandbox() { + use crate::hypervisor::regs::{MsrClass, MsrEntry, MsrFlags}; + 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(); + + // Take a baseline snapshot, then take sole ownership of it so we + // can doctor its MSR list. + 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 non-canonical KERNEL_GS_BASE value: WRMSR faults, so KVM's + // SET_MSRS stops early and reports a short count. + snap.set_msrs(Some(vec![MsrEntry { + index: 0xC000_0102, + value: 0xDEAD_0000_0000_0000, + class: MsrClass::Stateful, + flags: MsrFlags::NONE, + }])); + let snap = Arc::new(snap); + + let err = sbox + .restore(snap) + .expect_err("restore should fail on short SET_MSRS"); + 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" + ); + + // A poisoned sandbox must refuse further guest calls. + let call = sbox.call::("Echo", "hi".to_string()); + assert!( + matches!(call, Err(HyperlightError::PoisonedSandbox)), + "poisoned sandbox should reject guest calls, got: {call:?}" + ); + } + + /// mshv completeness: a guest write to a pass-through MSR that is in + /// the reset set must not survive a restore. mshv has no per-MSR deny + /// filter, so the guest can write `KERNEL_GS_BASE` directly with no + /// intercept; reset is what rolls it back. + #[test] + #[cfg(all(mshv3, target_arch = "x86_64"))] + fn test_mshv_passthrough_msr_reset_across_restore() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Mshv)) { + return; + } + + let msr_index: u32 = 0xC000_0102; // IA32_KERNEL_GS_BASE + let sentinel: u64 = 0xCAFE_F00D; + + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + // Baseline taken before the guest touches the MSR. + 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" + ); + + // The guest writes the sentinel with no allow list, since mshv + // passes this MSR through. + sbox.call::<()>("WriteMSR", (msr_index, sentinel)).unwrap(); + assert_eq!( + sbox.call::("ReadMSR", msr_index).unwrap(), + sentinel, + "sentinel should be observable before restore" + ); + + // Restore rolls the MSR back even though no intercept denied the + // write. + 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"); + } + + /// Allowing an MSR that has no `hv_register_name` mapping on mshv + /// (here `IA32_FS_BASE`, which is reset via `sregs`, not `CommonMsrs`) + /// must be a hard error at sandbox creation. + #[test] + #[cfg(all(mshv3, target_arch = "x86_64"))] + fn test_mshv_allow_unmapped_msr_fails_creation() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Mshv)) { + return; + } + + let mut cfg = SandboxConfiguration::default(); + cfg.allow_msr(0xC000_0100); // IA32_FS_BASE, not mapped by msr_to_hv_reg_name + + let err = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + Some(cfg), + ) + .unwrap() + .evolve() + .unwrap_err(); + + assert!( + format!("{err:?}").to_lowercase().contains("msr"), + "expected an MSR mapping error, got: {err:?}" + ); + } + + /// Completeness backstop for the MSR classes mshv cannot reset. FRED, + /// AMX/XFD, PMU, and architectural LBR MSRs have no `hv_register_name` + /// mapping, so they cannot be captured or restored. The reset-set + /// argument only holds if the guest cannot write them at all. Hyperlight + /// enables none of these features on its minimal partition, so a guest + /// `RDMSR` to a representative MSR of each class must fault. A fault + /// leaves the guest aborted and the sandbox poisoned. If any of these + /// stops faulting, that class is guest-reachable and un-resettable, + /// which is a cross-restore leak that must be addressed. + #[test] + #[cfg(all(mshv3, target_arch = "x86_64"))] + fn test_mshv_unresettable_msr_classes_fault() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Mshv)) { + return; + } + + // One representative MSR per un-mappable pass-through class. + let cases: [(u32, &str); 5] = [ + (0xC1, "PMU IA32_PMC0"), + (0x186, "PMU IA32_PERFEVTSEL0"), + (0x1C4, "AMX IA32_XFD"), + (0x1D4, "FRED IA32_FRED_CONFIG"), + (0x14CE, "arch-LBR IA32_LBR_CTL"), + ]; + + for (msr, name) 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::("ReadMSR", msr); + assert!( + result.is_err(), + "guest RDMSR to {name} (0x{msr:X}) should fault, got: {result:?}. \ + If this MSR is readable, its class is guest-reachable and cannot be \ + reset, which is a cross-restore leak." + ); + assert!( + sbox.poisoned(), + "sandbox should be poisoned after a faulting RDMSR to {name} (0x{msr:X})" + ); + } + } + + /// A partial `set_msrs` during restore must fail closed on mshv: the + /// sandbox is poisoned and refuses further guest calls. mshv applies + /// the batch as one rep hypercall; a value the hypervisor rejects + /// leaves earlier registers committed and returns `EINTR` (a partial + /// apply, no count). Driven by doctoring a snapshot with a + /// non-canonical `KERNEL_GS_BASE` value. + #[test] + #[cfg(all(mshv3, target_arch = "x86_64"))] + fn test_mshv_partial_set_msrs_poisons_sandbox() { + use crate::hypervisor::regs::{MsrClass, MsrEntry, MsrFlags}; + 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(); + + // Take a baseline snapshot, then take sole ownership of it so we + // can doctor its MSR list. + 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 non-canonical KERNEL_GS_BASE value: the hypervisor rejects the + // register write, so the rep hypercall stops early and set_msrs + // returns an error. + snap.set_msrs(Some(vec![MsrEntry { + index: 0xC000_0102, + value: 0xDEAD_0000_0000_0000, + class: MsrClass::Stateful, + flags: MsrFlags::NONE, + }])); + let snap = Arc::new(snap); + + let err = sbox + .restore(snap) + .expect_err("restore should fail on a rejected set_msrs"); + 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" + ); + + // A poisoned sandbox must refuse further guest calls. + let call = sbox.call::("Echo", "hi".to_string()); + assert!( + matches!(call, Err(HyperlightError::PoisonedSandbox)), + "poisoned sandbox should reject guest calls, got: {call:?}" + ); + } + + /// mshv leak sweep over the MTRR and TSC MSRs the shared reset table + /// added for filterless backends. mshv has no per-MSR deny filter, so + /// each of these is either guest-unreachable (a write faults, no state + /// persists), masked (the write is dropped and reads back unchanged), + /// or retained (the write sticks). For any retained MSR, a restore must + /// roll it back to baseline. Asserts no value leaks across a restore, + /// whichever of those the host does. + #[test] + #[cfg(all(mshv3, target_arch = "x86_64"))] + fn test_mshv_mtrr_and_tsc_msrs_no_leak_across_restore() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Mshv)) { + return; + } + + // (index, sentinel) for a representative member of each group the + // shared table added. Sentinels differ from a zeroed baseline. + let cases: &[(u32, u64)] = &[ + (0x3B, 0x1000), // TSC_ADJUST + (0xC000_0103, 0x5), // TSC_AUX + (0x2FF, 0xC00), // MTRR_DEF_TYPE + (0x200, 0x6), // MTRR_PHYSBASE0 + (0x201, 0x800), // MTRR_PHYSMASK0 + (0x250, 0x0606_0606_0606_0606), // MTRR_FIX64K_00000 + ]; + + for &(msr, sentinel) in cases { + // A fresh sandbox per case: a faulting write poisons the + // sandbox, so it cannot be reused for the next MSR. + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + let baseline = sbox.snapshot().unwrap(); + let original: u64 = match sbox.call("ReadMSR", msr) { + Ok(v) => v, + // Guest cannot even read it: the class is unreachable, so + // it cannot hold guest state. Safe. + Err(_) => { + assert!(sbox.poisoned()); + continue; + } + }; + + if sbox.call::<()>("WriteMSR", (msr, sentinel)).is_err() { + // Guest write faults: unreachable, no state persists. Safe. + assert!(sbox.poisoned()); + continue; + } + + let observed: u64 = sbox.call("ReadMSR", msr).unwrap(); + if observed != sentinel { + // The write was masked/dropped: no retained state. Safe. + continue; + } + + // The write stuck. A restore must roll it back to baseline. + sbox.restore(baseline).unwrap(); + let after: u64 = sbox.call("ReadMSR", msr).unwrap(); + assert_eq!( + after, original, + "0x{msr:X}: retained MSR leaked across restore \ + (expected 0x{original:X}, got 0x{after:X})" + ); + } + } + + /// WHP: a guest MSR write to a reset-set MSR (`KERNEL_GS_BASE`) does not + /// survive a snapshot restore. WHP models MSRs as named registers, so + /// this also exercises the `WHV_REGISTER_NAME` capture/apply path. + #[test] + #[cfg(all(target_os = "windows", target_arch = "x86_64"))] + fn test_whp_msr_reset_across_restore() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Whp)) { + return; + } + + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + let msr_index: u32 = 0xC000_0102; // IA32_KERNEL_GS_BASE + let baseline = sbox.snapshot().unwrap(); + let original: u64 = sbox.call("ReadMSR", msr_index).unwrap(); + let sentinel: u64 = 0xCAFE_F00D; + assert_ne!(original, sentinel); + + 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, "MSR should be reset across restore on WHP"); + assert_eq!(after, original, "MSR not reset to its baseline value"); + } + + /// WHP: MSRs that the guest can freely write (WHP has no per-MSR deny + /// filter) but that hold retained state — the MTRRs, `TSC_ADJUST`, and + /// `TSC_AUX` — must be captured and reset across a restore, so a guest + /// write does not leak into the next use of the sandbox. + #[test] + #[cfg(all(target_os = "windows", target_arch = "x86_64"))] + fn test_whp_mtrr_and_tsc_msrs_reset_across_restore() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Whp)) { + return; + } + + // (index, sentinel) — representative members of each newly covered + // group. Sentinels are chosen to differ from the zeroed baseline. + let cases: &[(u32, u64)] = &[ + (0x3B, 0x1000), // TSC_ADJUST + (0xC000_0103, 0x5), // TSC_AUX + (0x2FF, 0xC00), // MTRR_DEF_TYPE + (0x200, 0x6), // MTRR_PHYSBASE0 + (0x201, 0x800), // MTRR_PHYSMASK0 + (0x250, 0x0606_0606_0606_0606), // MTRR_FIX64K_00000 + ]; + + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + for &(msr, sentinel) in cases { + let baseline = sbox.snapshot().unwrap(); + let original: u64 = sbox.call("ReadMSR", msr).unwrap(); + assert_ne!( + original, sentinel, + "0x{msr:X}: sentinel must differ from baseline" + ); + + sbox.call::<()>("WriteMSR", (msr, sentinel)).unwrap(); + assert_eq!( + sbox.call::("ReadMSR", msr).unwrap(), + sentinel, + "0x{msr:X}: write should be observable before restore" + ); + + sbox.restore(baseline).unwrap(); + let after: u64 = sbox.call("ReadMSR", msr).unwrap(); + assert_eq!( + after, original, + "0x{msr:X}: MSR leaked across restore (expected reset to 0x{original:X}, got 0x{after:X})" + ); + } + } + + /// WHP: the MSR classes the completeness argument relies on being + /// unreachable (PMU, AMX, FRED, LBR) or harmlessly masked (`DEBUGCTL`) + /// must never *retain* a guest write. None of these has a WHP named + /// register, so they cannot be reset; the argument only holds if a + /// guest write either faults or is dropped. For each, a `WRMSR` must + /// leave no observable state: it either errors (faults, poisoning the + /// sandbox) or reads back unchanged. If any starts retaining a written + /// value, that class became guest-reachable and un-resettable — a leak + /// — and this test fails to flag it. + #[test] + #[cfg(all(target_os = "windows", target_arch = "x86_64"))] + fn test_whp_unresettable_msr_classes_do_not_retain() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + if !matches!(get_available_hypervisor(), Some(HypervisorType::Whp)) { + return; + } + + // Representative MSRs per un-resettable class, plus DEBUGCTL (the + // one that is modeled-but-masked rather than faulting). + let cases: &[(u32, &str)] = &[ + (0x1D9, "DEBUGCTL"), + (0x1C8, "LBR_SELECT"), + (0x14CE, "arch-LBR IA32_LBR_CTL"), + (0xC1, "PMU IA32_PMC0"), + (0x186, "PMU IA32_PERFEVTSEL0"), + (0x38F, "PMU IA32_PERF_GLOBAL_CTRL"), + (0x1C4, "AMX IA32_XFD"), + (0xDA0, "IA32_XSS"), + (0x1D4, "FRED IA32_FRED_CONFIG"), + ]; + + let sentinel: u64 = 0x1; + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + for &(msr, name) in cases { + // Snapshot the healthy state so a faulting write can be rolled + // back (restore clears the poison) and the sandbox reused for + // the next case, rather than recreating a VM each time. + let baseline = sbox.snapshot().unwrap(); + + match sbox.call::<()>("WriteMSR", (msr, sentinel)) { + // Write faulted: the class is unreachable, nothing persists. + Err(_) => assert!( + sbox.poisoned(), + "{name} (0x{msr:X}): a faulting WRMSR should poison the sandbox" + ), + // Write accepted: it must have been masked, i.e. it holds no + // guest state and reads back as something other than what we + // wrote. A read-back equal to the sentinel means the value + // was retained in an un-resettable MSR — a leak. + Ok(()) => { + let observed = sbox.call::("ReadMSR", msr); + assert!( + !matches!(&observed, Ok(v) if *v == sentinel), + "{name} (0x{msr:X}): guest write was retained (read back {observed:?}); \ + this MSR is un-resettable on WHP, so a retained write leaks across restore" + ); + } + } + + sbox.restore(baseline).unwrap(); + } + } + + /// A backend-agnostic full-window MSR completeness sweep. It probes + /// every index in the two bands where architectural and model-specific + /// MSRs live (`0x0..=0x1FFF` and `0xC000_0000..=0xC000_1FFF`): for each, + /// the guest reads the current value, writes a near-identical sentinel + /// (only the low bits flipped, so a dangerous write tends to hit reserved + /// bits and fault rather than wedge the vCPU), we `restore`, and require + /// the exact baseline back. Whatever the host does with the write — + /// fault, mask, or retain-and-reset — no guest-written value may survive + /// a restore. This is the strongest feasible "no MSR can leak" check: it + /// catches a retained-but-unreset MSR that no hand-picked list enumerates. + /// + /// Runs on every backend. A faulting or filter-denied access poisons the + /// sandbox, but `restore` clears the poison, so the sweep never recreates + /// a VM. On KVM the deny-by-default filter intercepts every non-allowed + /// MSR access, so most indices self-skip there (its real MSR coverage is + /// `test_msr_reset_sweep_over_host_index_list` plus the deny tests); on + /// the filterless backends (mshv, WHP) this is the meaningful sweep. + #[test] + #[cfg(target_arch = "x86_64")] + fn test_no_msr_leaks_across_restore_full_window_sweep() { + use crate::hypervisor::virtual_machine::{HypervisorType, get_available_hypervisor}; + + // Free-running / time-reference MSRs: their value advances on its + // own, so an exact-match rollback check is meaningless. Guest `TSC` + // manipulation is covered via `TSC_ADJUST` elsewhere. + const SKIP: &[u32] = &[ + 0x10, // IA32_TIME_STAMP_COUNTER + 0xE7, // IA32_MPERF + 0xE8, // IA32_APERF + 0x6E0, // IA32_TSC_DEADLINE + ]; + + let mut sbox = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), + None, + ) + .unwrap() + .evolve() + .unwrap(); + + // One baseline for the whole sweep. `restore` takes an `Arc` + // by value, so each iteration passes a cheap clone instead of + // re-snapshotting on every index. + let baseline = sbox.snapshot().unwrap(); + + // Non-vacuity guard: count MSRs where the guest write actually + // changed the value (observed *before* restore) and `restore` then + // rolled it back to baseline. A sweep in which every index merely + // faults would prove nothing, so we require this to be positive. + // Coverage diagnostics: collect the actual indices in each bucket so + // the read-only / masked classes can be inspected and confirmed to be + // genuinely non-leaking (architectural read-only or write-masked). + let mut readable = 0usize; + let mut exercised: Vec = Vec::new(); + let mut read_only: Vec = Vec::new(); + let mut masked_only: Vec = Vec::new(); + + let windows = (0x0000_0000u32..=0x0000_1FFF).chain(0xC000_0000u32..=0xC000_1FFF); + for msr in windows { + if SKIP.contains(&msr) { + continue; + } + // The sandbox is healthy at the top of every iteration: each + // branch below restores the pre-access baseline, which also + // clears any poison a faulting or filter-denied access left. + let original: u64 = match sbox.call("ReadMSR", msr) { + Ok(v) => v, + // Not readable: an unreachable class faults, or the deny + // filter intercepts the read. No guest-writable state, so it + // cannot leak. Restore clears the poison and we move on. + Err(_) => { + sbox.restore(baseline.clone()).unwrap(); + continue; + } + }; + readable += 1; + + // A single fixed sentinel misses MSRs that are writable only with + // a valid value: flipping a reserved bit `#GP`s even though the + // MSR is writable. Try several diverse candidates and use the + // first the guest can both write *and* whose new value the MSR + // actually retains, so a writable MSR is genuinely exercised + // regardless of its reserved-bit layout. `0` is included last to + // catch MSRs that only accept a clear rather than a bit toggle. + 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() { + // Reserved/invalid for this value: restore and try the next. + sbox.restore(baseline.clone()).unwrap(); + continue; + } + saw_write = true; + match sbox.call::("ReadMSR", msr) { + // The write stuck and changed the value: leave it live and + // go verify that restore rolls it back. + Ok(v) if v != original => { + planted = true; + break; + } + // Masked (write dropped) or write-only: nothing retained + // for this value; restore and try the next candidate. + _ => { + sbox.restore(baseline.clone()).unwrap(); + } + } + } + + if planted { + // A guest-visible change is live in the MSR. Restore and + // require the exact baseline back, catching any retained bits. + 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 { + // Writable but every value was masked/dropped: no retained + // state, so it cannot leak. + masked_only.push(msr); + } else { + // No value was writable: effectively read-only to the guest. + 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)); + + // On KVM the deny-by-default MSR filter intercepts every non-allowed + // access, so a sweep with an empty allow list is vacuous *by design*: + // the only indices the filter can't deny are the x2APIC MSRs + // (0x800-0x8FF, which `kvm_msr_allowed()` short-circuits to allowed), + // and those fault architecturally in xAPIC mode. So every access here + // errors and nothing is exercised. Assert that vacuity rather than + // requiring coverage; KVM's real MSR coverage lives in + // `test_msr_reset_sweep_over_host_index_list` plus the deny tests. + // On the filterless backends (mshv, WHP) this is the meaningful sweep, + // so require it to have actually exercised the rollback path. + if matches!(get_available_hypervisor(), Some(HypervisorType::Kvm)) { + assert!( + exercised.is_empty(), + "KVM denies every non-allowed MSR, so the sweep should exercise \ + nothing; got: [{}]", + fmt(&exercised) + ); + } else { + 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" + ); + } + } + /// Tests for [`MultiUseSandbox::from_snapshot`] in-memory. mod from_snapshot { use std::sync::Arc; diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs index 59fcd6372..0963c0d80 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs @@ -871,6 +871,8 @@ 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: None, entrypoint, snapshot_generation, host_functions, diff --git a/src/hyperlight_host/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index 9a638a98e..a8ca5bbf1 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -31,6 +31,8 @@ use hyperlight_common::vmem::{ use tracing::{Span, instrument}; use crate::Result; +#[cfg(target_arch = "x86_64")] +use crate::hypervisor::regs::CommonMsrs; use crate::hypervisor::regs::CommonSpecialRegisters; use crate::mem::exe::{ExeInfo, LoadInfo}; use crate::mem::layout::SandboxMemoryLayout; @@ -92,6 +94,14 @@ pub struct Snapshot { /// tables are relocated during snapshot. sregs: Option, + /// Reset-set MSR values captured from the vCPU during snapshot. + /// None for snapshots created directly from a binary (the baseline + /// captured at init is used instead) and on backends without MSR + /// reset. Some for snapshots taken from a running sandbox on KVM or + /// mshv. + #[cfg(target_arch = "x86_64")] + msrs: Option, + /// The next action that should be performed on this snapshot entrypoint: NextAction, @@ -381,6 +391,8 @@ impl Snapshot { load_info, stack_top_gva: exn_stack_top_gva, sregs: None, + #[cfg(target_arch = "x86_64")] + msrs: None, entrypoint: NextAction::Initialise(load_addr + entrypoint_va - base_va), snapshot_generation: 0, host_functions: HostFunctionDetails { @@ -560,6 +572,8 @@ impl Snapshot { load_info, stack_top_gva, sregs: Some(sregs), + #[cfg(target_arch = "x86_64")] + msrs: None, entrypoint, snapshot_generation, host_functions, @@ -603,6 +617,20 @@ impl Snapshot { self.sregs.as_ref() } + /// Returns the reset-set MSRs stored in this snapshot, if any. `None` + /// for from-binary snapshots (the init baseline is used on restore + /// instead) and on backends without MSR reset. + #[cfg(target_arch = "x86_64")] + pub(crate) fn msrs(&self) -> Option<&CommonMsrs> { + self.msrs.as_ref() + } + + /// Store the reset-set MSRs captured for this snapshot. + #[cfg(target_arch = "x86_64")] + pub(crate) fn set_msrs(&mut self, msrs: Option) { + self.msrs = msrs; + } + pub(crate) fn entrypoint(&self) -> NextAction { self.entrypoint } diff --git a/src/tests/rust_guests/simpleguest/src/main.rs b/src/tests/rust_guests/simpleguest/src/main.rs index 2da2eb7ca..3e65f2178 100644 --- a/src/tests/rust_guests/simpleguest/src/main.rs +++ b/src/tests/rust_guests/simpleguest/src/main.rs @@ -1095,6 +1095,38 @@ 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); + 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("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; + 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() {