Disallow read/write of MSRs

Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>

SOme clean stuff

Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>

Fix some tests

Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>

Author: ludfjig

Cargo.lock

@@ -75,8 +75,8 @@ windows-version = "0.1"
 lazy_static = "1.4.0"
 
 [target.'cfg(unix)'.dependencies]
-kvm-bindings = { version = "0.14", features = ["fam-wrappers"], optional = true }
-kvm-ioctls = { version = "0.24", optional = true }
+kvm-bindings = { git = "https://github.com/rust-vmm/kvm", rev = "3ffc9b62af5978553f73cc0ec79fad13fdd47146", features = ["fam-wrappers"], optional = true }
+kvm-ioctls = { git = "https://github.com/rust-vmm/kvm", rev = "3ffc9b62af5978553f73cc0ec79fad13fdd47146", optional = true }
 mshv-bindings = { version = "0.6", optional = true }
 mshv-ioctls = { version = "0.6", optional = true}
 

src/hyperlight_host/Cargo.toml

@@ -144,6 +144,14 @@ pub enum HyperlightError {
     #[error("Memory Access Violation at address {0:#x} of type {1}, but memory is marked as {2}")]
     MemoryAccessViolation(u64, MemoryRegionFlags, MemoryRegionFlags),
 
+    /// MSR Read Violation. Guest attempted to read from a Model-Specific Register
+    #[error("Guest attempted to read from MSR {0:#x}")]
+    MsrReadViolation(u32),
+
+    /// MSR Write Violation. Guest attempted to write to a Model-Specific Register
+    #[error("Guest attempted to write {1:#x} to MSR {0:#x}")]
+    MsrWriteViolation(u32, u64),
+
     /// Memory Allocation Failed.
     #[error("Memory Allocation Failed with OS Error {0:?}.")]
     MemoryAllocationFailed(Option<i32>),
@@ -325,6 +333,8 @@ impl HyperlightError {
             | HyperlightError::ExecutionAccessViolation(_)
             | HyperlightError::StackOverflow()
             | HyperlightError::MemoryAccessViolation(_, _, _)
+            | HyperlightError::MsrReadViolation(_)
+            | HyperlightError::MsrWriteViolation(_, _)
             | HyperlightError::SnapshotSizeMismatch(_, _)
             | HyperlightError::MemoryRegionSizeMismatch(_, _, _)
             // HyperlightVmError::Restore is already handled manually in restore(), but we mark it

src/hyperlight_host/src/error.rs

@@ -155,6 +155,14 @@ impl DispatchGuestCallError {
                 region_flags,
             }) => HyperlightError::MemoryAccessViolation(addr, access_type, region_flags),
 
+            DispatchGuestCallError::Run(RunVmError::MsrReadViolation(msr_index)) => {
+                HyperlightError::MsrReadViolation(msr_index)
+            }
+
+            DispatchGuestCallError::Run(RunVmError::MsrWriteViolation { msr_index, value }) => {
+                HyperlightError::MsrWriteViolation(msr_index, value)
+            }
+
             // Leave others as is
             other => HyperlightVmError::DispatchGuestCall(other).into(),
         };
@@ -203,6 +211,10 @@ pub enum RunVmError {
     MmioReadUnmapped(u64),
     #[error("MMIO WRITE access to unmapped address {0:#x}")]
     MmioWriteUnmapped(u64),
+    #[error("Guest attempted to read from MSR {0:#x}")]
+    MsrReadViolation(u32),
+    #[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}")]
@@ -340,7 +352,7 @@ impl HyperlightVm {
         _pml4_addr: u64,
         entrypoint: Option<u64>,
         rsp_gva: u64,
-        #[cfg_attr(target_os = "windows", allow(unused_variables))] config: &SandboxConfiguration,
+        config: &SandboxConfiguration,
         #[cfg(gdb)] gdb_conn: Option<DebugCommChannel<DebugResponse, DebugMsg>>,
         #[cfg(crashdump)] rt_cfg: SandboxRuntimeConfig,
         #[cfg(feature = "mem_profile")] trace_info: MemTraceInfo,
@@ -350,7 +362,7 @@ impl HyperlightVm {
         #[cfg(not(gdb))]
         type VmType = Box<dyn VirtualMachine>;
 
-        let vm: VmType = match get_available_hypervisor() {
+        let mut vm: VmType = match get_available_hypervisor() {
             #[cfg(kvm)]
             Some(HypervisorType::Kvm) => Box::new(KvmVm::new().map_err(VmError::CreateVm)?),
             #[cfg(mshv3)]
@@ -360,6 +372,11 @@ impl HyperlightVm {
             None => return Err(CreateHyperlightVmError::NoHypervisorFound),
         };
 
+        // Enable MSR intercepts unless the user explicitly allows MSR access
+        if !config.get_allow_msr() {
+            vm.enable_msr_intercept().map_err(VmError::CreateVm)?;
+        }
+
         #[cfg(feature = "init-paging")]
         vm.set_sregs(&CommonSpecialRegisters::standard_64bit_defaults(_pml4_addr))
             .map_err(VmError::Register)?;
@@ -811,6 +828,12 @@ impl HyperlightVm {
                         }
                     }
                 }
+                Ok(VmExit::MsrRead(msr_index)) => {
+                    break Err(RunVmError::MsrReadViolation(msr_index));
+                }
+                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:
@@ -906,6 +929,7 @@ impl HyperlightVm {
     }
 
     /// Resets the following vCPU state:
+    /// - MSRs (see [`VIRTUALIZED_MSRS`](super::virtual_machine::VIRTUALIZED_MSRS))
     /// - General purpose registers
     /// - Debug registers
     /// - XSAVE (includes FPU/SSE state with proper FCW and MXCSR defaults)
@@ -916,6 +940,8 @@ impl HyperlightVm {
         cr3: u64,
         sregs: &CommonSpecialRegisters,
     ) -> std::result::Result<(), RegisterError> {
+        self.vm.reset_msrs()?;
+
         self.vm.set_regs(&CommonRegisters {
             rflags: 1 << 1, // Reserved bit always set
             ..Default::default()

src/hyperlight_host/src/hypervisor/hyperlight_vm.rs

@@ -19,10 +19,14 @@ use std::sync::LazyLock;
 #[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,
+    kvm_debugregs, kvm_enable_cap, kvm_fpu, 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;
@@ -139,6 +143,36 @@ impl KvmVm {
 }
 
 impl VirtualMachine for KvmVm {
+    fn enable_msr_intercept(&mut self) -> std::result::Result<(), CreateVmError> {
+        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::EnableMsrIntercept(e.into()))?;
+
+        // Install a deny-all MSR filter (KVM_X86_SET_MSR_FILTER).
+        // At least one range is required when using KVM_MSR_FILTER_DEFAULT_DENY;
+        // from the docs: "Calling this ioctl with an empty set of ranges
+        // (all nmsrs == 0) disables MSR filtering. In that mode,
+        // KVM_MSR_FILTER_DEFAULT_DENY is invalid and causes an error."
+        let bitmap = [0u8; 1]; // 1 byte covers 8 MSRs, all bits 0 (deny)
+        self.vm_fd
+            .set_msr_filter(
+                MsrFilterDefaultAction::DENY,
+                &[MsrFilterRange {
+                    flags: MsrFilterRangeFlags::READ | MsrFilterRangeFlags::WRITE,
+                    base: 0,
+                    msr_count: 1,
+                    bitmap: &bitmap,
+                }],
+            )
+            .map_err(|e| CreateVmError::EnableMsrIntercept(e.into()))?;
+        Ok(())
+    }
+
     unsafe fn map_memory(
         &mut self,
         (slot, region): (u32, &MemoryRegion),
@@ -176,6 +210,40 @@ impl VirtualMachine for 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,
@@ -327,6 +395,13 @@ impl VirtualMachine for KvmVm {
 
         Ok(())
     }
+
+    fn reset_msrs(&self) -> std::result::Result<(), RegisterError> {
+        // The KVM MSR filter (KVM_MSR_FILTER_DEFAULT_DENY) blocks all guest
+        // MSR access at the hardware level, so no MSRs can be modified by the
+        // guest and there is nothing to reset.
+        Ok(())
+    }
 }
 
 #[cfg(gdb)]