Disallow MSR read/writes

.github/workflows/ValidatePullRequest.yml

@@ -147,7 +147,7 @@ jobs:
       # pull goldens from GHCR in the called workflow
       packages: read
     strategy:
-      fail-fast: true
+      fail-fast: false # temp will remove
       matrix:
         hypervisor: ['hyperv-ws2025', mshv3, kvm]
         cpu_vendor: [amd, intel, apple]

docs/msr.md

@@ -0,0 +1,129 @@
+# MSR state across restore
+
+## Requirement
+
+A snapshot restore must remove all model-specific register (MSR) state written
+after the snapshot. The guest must observe the MSR values saved with the
+restored state.
+
+## Reset policy
+
+The reset set contains every MSR whose guest-written value can persist. Each
+running snapshot stores values for this set. A snapshot created from a guest
+binary has no saved MSR values, so restore uses the baseline captured when the
+VM was created.
+
+`MSR_TABLE` classifies known MSRs:
+
+* `Stateful` entries hold values that restore must write.
+* `WriteOnlyCommand` entries hold no state. Their classification gives allow
+  requests a clear error.
+
+The resolved reset set contains the backend core set, required MTRRs, and the
+validated allow list. Hyperlight sorts and deduplicates the indices before
+capturing the initialization baseline.
+
+The required invariant is:
+
+```text
+guest-writable retained state => host-readable and host-writable state
+```
+
+Host-readable state need not be guest-writable. Extra reset entries are safe.
+`EFER`, `APIC_BASE`, `FS_BASE`, and `GS_BASE` belong to the special-register
+state.
+
+## Snapshot validation
+
+Snapshot MSR entries are untrusted. `validate_snapshot` checks their count,
+indices, and order against the locally resolved reset set. It also replaces
+their classifications with values from `MSR_TABLE`.
+
+A mismatch rejects the restore. A restore failure poisons the sandbox before
+the guest can run. Equivalent allow lists produce the same sorted reset set,
+regardless of insertion order.
+
+## Allow list
+
+`SandboxConfiguration::allow_msr` and `allow_msrs` add indices to the requested
+allow list. They enforce capacity only. VM creation verifies that each index is
+stateful and supported by the selected backend.
+
+KVM requires the index in `KVM_GET_MSR_INDEX_LIST` and a successful host read
+and write. MSHV and WHP require a named-register mapping.
+
+At most 64 distinct MSRs may be requested. KVM also limits the resulting
+contiguous filter groups to 16.
+
+## KVM
+
+KVM installs a deny filter over the full MSR space. Allowed indices form the
+only guest `RDMSR` and `WRMSR` paths through that filter. A denied access exits
+to Hyperlight, injects `#GP`, and poisons the sandbox. The denied write stores
+no state.
+
+The KVM reset set contains the allow list plus `KERNEL_GS_BASE` and `TSC`.
+`KERNEL_GS_BASE` is required because `WRGSBASE` followed by `SWAPGS` changes it
+without `WRMSR`. `TSC` gives restore the same clock semantics on every backend.
+
+KVM does not filter x2APIC indices `0x800..=0x8FF`. Hyperlight keeps the APIC in
+xAPIC mode, where MSR access to that range raises `#GP`.
+
+## MTRRs
+
+MSHV and WHP read `IA32_MTRRCAP` when the VM is created. The required set
+contains `MTRR_DEF_TYPE`, each variable pair reported by `VCNT`, and all fixed
+MTRRs.
+
+Hyper-V accepts fixed-MTRR writes even when `MTRRCAP.FIX` is clear. All fixed
+MTRRs are therefore required. Hyper-V supports at most 16 variable pairs. VM
+creation fails when the count is larger or a required MTRR cannot be read.
+
+## MSHV
+
+MSHV has no per-MSR filter. Its reset set contains every stateful table entry
+that has a Hyper-V register mapping and can be read, plus the allow list.
+
+`msr_to_hv_reg_name` determines which indices the get and set path can reach.
+The enumerated host index list does not classify retained state, so it does not
+define the reset set.
+
+MSHV maps `IA32_XSS` through `MSR_IA32_REGISTER_U_XSS`. It maps `IA32_MPERF`
+and `IA32_APERF` to the per-VP `MCount` and `ACount` registers. TSX control and
+XFD state enter the reset set when their host-register probes succeed.
+
+## WHP
+
+WHP has no per-MSR filter. Its reset set contains every stateful table entry
+that has a WHP register name and can be read, plus the allow list.
+
+WHP uses Germanium compatibility. PMU, architectural LBR, and experimental
+`DEBUGCTL` bits remain disabled. FRED is in a processor feature bank that the
+WHP API does not expose.
+
+Each guest MSR write is either captured for restore or unsupported by the
+partition. Unsupported writes store no state.
+
+## TSC
+
+MSHV and WHP expose `TSC` as a host-writable register. Hyper-V stores `TSC` and
+`TSC_ADJUST` independently, so restoring `TSC_ADJUST` cannot undo a guest
+`WRMSR(TSC)`.
+
+Hyper-V does not permit an intercept for its implemented `TSC` MSR. Restore
+must therefore write the captured `TSC` value. KVM also restores `TSC` so all
+backends rewind guest time with the rest of the snapshot state.
+
+## Limitations
+
+KVM's security boundary is structural because its deny filter bounds guest
+writes. MSHV and WHP depend on the classified table and exposed processor
+features.
+
+The filterless backend tests run on one CPU model per runner. Model-specific
+state absent on that CPU is not exercised. A backend that exposes a new
+retained MSR feature needs a matching table entry before Hyperlight can use it
+safely.
+
+The ignored full-window audit probes fixed index ranges with a small set of
+values. It cannot prove that every vendor MSR or accepted value is covered.

src/hyperlight_host/src/error.rs

@@ -154,6 +154,16 @@ pub enum HyperlightError {
     #[error("Memory Access Violation at address {0:#x} of type {1}, but memory is marked as {2}")]
     MemoryAccessViolation(u64, MemoryRegionFlags, MemoryRegionFlags),
 
+    /// A denied guest MSR read.
+    #[cfg(target_arch = "x86_64")]
+    #[error("Guest read from denied MSR {0:#x}")]
+    MsrReadViolation(u32),
+
+    /// A denied guest MSR write.
+    #[cfg(target_arch = "x86_64")]
+    #[error("Guest write of {1:#x} to denied MSR {0:#x}")]
+    MsrWriteViolation(u32, u64),
+
     /// Memory Allocation Failed.
     #[error("Memory Allocation Failed with OS Error {0:?}.")]
     MemoryAllocationFailed(Option<i32>),
@@ -352,6 +362,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

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 read from denied MSR {0:#x}")]
+    MsrReadViolation(u32),
+    #[cfg(all(target_arch = "x86_64", any(kvm, target_os = "windows")))]
+    #[error("Guest write of {value:#x} to denied MSR {msr_index:#x}")]
+    MsrWriteViolation { msr_index: u32, value: u64 },
     #[error("vCPU run failed: {0}")]
     RunVcpu(#[from] RunVcpuError),
     #[error("Unexpected VM exit: {0}")]
@@ -409,6 +425,9 @@ pub(crate) struct HyperlightVm {
     pub(super) trace_info: MemTraceInfo,
     #[cfg(crashdump)]
     pub(super) rt_cfg: SandboxRuntimeConfig,
+    /// MSR indices and initialization baseline used by snapshot restore.
+    #[cfg(target_arch = "x86_64")]
+    pub(super) msr_reset: Option<crate::hypervisor::regs::MsrResetState>,
 }
 
 impl HyperlightVm {
@@ -732,6 +751,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:

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, is_mtrr_reset_index,
 };
+#[cfg(kvm)]
+use crate::hypervisor::regs::{MSR_KERNEL_GS_BASE, MSR_TSC};
 #[cfg(not(gdb))]
 use crate::hypervisor::virtual_machine::VirtualMachine;
 #[cfg(kvm)]
 use crate::hypervisor::virtual_machine::kvm::KvmVm;
 #[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,
 };
@@ -90,13 +95,35 @@ impl HyperlightVm {
         #[cfg(not(gdb))]
         type VmType = Box<dyn VirtualMachine>;
 
-        let vm: VmType = match get_available_hypervisor() {
+        let (vm, required_mtrrs): (VmType, Vec<u32>) = match get_available_hypervisor() {
             #[cfg(kvm)]
-            Some(HypervisorType::Kvm) => Box::new(KvmVm::new().map_err(VmError::CreateVm)?),
+            Some(HypervisorType::Kvm) => {
+                let kvm_vm = KvmVm::new().map_err(VmError::CreateVm)?;
+                kvm_vm
+                    .configure_msr_access(config.get_allowed_msrs())
+                    .map_err(VmError::CreateVm)?;
+                (Box::new(kvm_vm), Vec::new())
+            }
             #[cfg(mshv3)]
-            Some(HypervisorType::Mshv) => Box::new(MshvVm::new().map_err(VmError::CreateVm)?),
+            Some(HypervisorType::Mshv) => {
+                let mshv_vm = MshvVm::new().map_err(VmError::CreateVm)?;
+                let required_mtrrs = mshv_vm.mtrr_reset_indices().map_err(VmError::CreateVm)?;
+                // The allow list adds reset state because MSHV has no MSR filter.
+                mshv_vm
+                    .validate_allowed_msrs(config.get_allowed_msrs())
+                    .map_err(VmError::CreateVm)?;
+                (Box::new(mshv_vm), required_mtrrs)
+            }
             #[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)?;
+                let required_mtrrs = whp_vm.mtrr_reset_indices().map_err(VmError::CreateVm)?;
+                // The allow list adds reset state because WHP has no MSR filter.
+                whp_vm
+                    .validate_allowed_msrs(config.get_allowed_msrs())
+                    .map_err(VmError::CreateVm)?;
+                (Box::new(whp_vm), required_mtrrs)
+            }
             None => return Err(CreateHyperlightVmError::NoHypervisorFound),
         };
 
@@ -168,11 +195,50 @@ impl HyperlightVm {
             trace_info,
             #[cfg(crashdump)]
             rt_cfg,
+            msr_reset: None,
         };
 
         ret.update_snapshot_mapping(snapshot_mem)?;
         ret.update_scratch_mapping(scratch_mem)?;
 
+        // Resolve the reset policy after vCPU initialization exposes host state.
+        {
+            // KVM resets SWAPGS state and TSC in addition to allowed MSRs.
+            // Filterless backends probe every core stateful MSR.
+            let core: Vec<u32> = match get_available_hypervisor() {
+                #[cfg(kvm)]
+                Some(HypervisorType::Kvm) => vec![MSR_KERNEL_GS_BASE, MSR_TSC],
+                #[cfg(mshv3)]
+                Some(HypervisorType::Mshv) => core_reset_indices()
+                    .filter(|i| !is_mtrr_reset_index(*i))
+                    .filter(|i| host_supports_msr(*i))
+                    .filter(|i| ret.vm.msrs(&[*i]).is_ok())
+                    .collect(),
+                // WHP can reset named registers that read successfully.
+                #[cfg(target_os = "windows")]
+                Some(HypervisorType::Whp) => core_reset_indices()
+                    .filter(|i| !is_mtrr_reset_index(*i))
+                    .filter(|i| host_supports_msr(*i))
+                    .filter(|i| ret.vm.msrs(&[*i]).is_ok())
+                    .collect(),
+                _ => Vec::new(),
+            };
+            if !core.is_empty()
+                || !required_mtrrs.is_empty()
+                || !config.get_allowed_msrs().is_empty()
+            {
+                let mut indices: Vec<u32> = core
+                    .into_iter()
+                    .chain(required_mtrrs)
+                    .chain(config.get_allowed_msrs().iter().copied())
+                    .collect();
+                indices.sort_unstable();
+                indices.dedup();
+                let baseline = ret.vm.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 +336,62 @@ impl HyperlightVm {
         Ok(self.vm.sregs()?)
     }
 
+    /// Returns the current state selected by the MSR reset policy.
+    /// Returns `None` when the reset policy is inactive.
+    pub(crate) fn get_msr_reset_state(&self) -> Result<Option<CommonMsrs>, AccessPageTableError> {
+        match &self.msr_reset {
+            Some(state) => Ok(Some(self.vm.msrs(&state.indices)?)),
+            None => Ok(None),
+        }
+    }
+
+    /// Restores snapshot MSRs or the initialization baseline.
+    pub(crate) fn restore_msrs(
+        &mut self,
+        snap_msrs: Option<&CommonMsrs>,
+    ) -> std::result::Result<(), ResetVcpuError> {
+        match (&self.msr_reset, snap_msrs) {
+            (None, None) => {}
+            (None, Some(msrs)) if msrs.is_empty() => {}
+            (None, Some(msrs)) => {
+                return Err(RegisterError::InvalidSnapshotMsrCount {
+                    expected: 0,
+                    actual: msrs.len(),
+                }
+                .into());
+            }
+            (Some(state), None) => self.vm.set_msrs(&state.baseline)?,
+            (Some(state), Some(msrs)) => {
+                let validated = state.validate_snapshot(msrs)?;
+                self.vm.set_msrs(&validated)?;
+            }
+        }
+        Ok(())
+    }
+
+    /// Reads arbitrary backend MSRs for tests.
+    #[cfg(all(test, mshv3, target_arch = "x86_64"))]
+    pub(crate) fn capture_msrs_for_test(
+        &self,
+        indices: &[u32],
+    ) -> std::result::Result<CommonMsrs, RegisterError> {
+        self.vm.msrs(indices)
+    }
+
+    /// Attempts one backend MSR write for tests.
+    #[cfg(all(test, target_arch = "x86_64"))]
+    #[allow(dead_code)]
+    pub(crate) fn try_set_msr_for_test(&self, index: u32, value: u64) -> bool {
+        use crate::hypervisor::regs::{MsrEntry, MsrKind};
+        self.vm
+            .set_msrs(&vec![MsrEntry {
+                index,
+                value,
+                kind: MsrKind::Stateful,
+            }])
+            .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_.
     ///