Disallow read/write of MSRs

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:
@@ -940,6 +963,7 @@ impl HyperlightVm {
                 .set_sregs(&CommonSpecialRegisters::standard_real_mode_defaults())?;
         }
 
+        // MSRs are not reset because we disallow read/write of MSRs, unless explicitly unsafely allowed by the user.
         Ok(())
     }
 

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,

src/hyperlight_host/src/hypervisor/virtual_machine/kvm.rs

@@ -130,6 +130,10 @@ 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
+    MsrRead(u32),
+    /// The vCPU tried to write to the given MSR with the given value
+    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
@@ -170,6 +174,8 @@ pub enum CreateVmError {
     CreateVcpuFd(HypervisorError),
     #[error("VM creation failed: {0}")]
     CreateVmFd(HypervisorError),
+    #[error("Failed to enable MSR intercept: {0}")]
+    EnableMsrIntercept(HypervisorError),
     #[error("Hypervisor is not available: {0}")]
     HypervisorNotAvailable(HypervisorError),
     #[error("Initialize VM failed: {0}")]
@@ -184,8 +190,10 @@ pub enum CreateVmError {
 /// RunVCPU error
 #[derive(Debug, Clone, thiserror::Error)]
 pub enum RunVcpuError {
-    #[error("Failed to decode message type: {0}")]
+    #[error("Failed to decode IO message type: {0}")]
     DecodeIOMessage(u32),
+    #[error("Failed to decode MSR message type: {0}")]
+    DecodeMsrMessage(u32),
     #[cfg(gdb)]
     #[error("Failed to get DR6 debug register: {0}")]
     GetDr6(HypervisorError),
@@ -344,6 +352,10 @@ pub(crate) trait VirtualMachine: Debug + Send {
     #[cfg(feature = "init-paging")]
     fn set_xsave(&self, xsave: &[u32]) -> std::result::Result<(), RegisterError>;
 
+    /// Enable MSR intercepts for this VM. When enabled, all MSR reads and
+    /// writes by the guest will cause a VM exit instead of being executed.
+    fn enable_msr_intercept(&mut self) -> std::result::Result<(), CreateVmError>;
+
     /// Get partition handle
     #[cfg(target_os = "windows")]
     fn partition_handle(&self) -> windows::Win32::System::Hypervisor::WHV_PARTITION_HANDLE;

src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs

@@ -21,12 +21,16 @@ use std::sync::LazyLock;
 #[cfg(gdb)]
 use mshv_bindings::{DebugRegisters, hv_message_type_HVMSG_X64_EXCEPTION_INTERCEPT};
 use mshv_bindings::{
-    FloatingPointUnit, SpecialRegisters, StandardRegisters, XSave, hv_message_type,
+    FloatingPointUnit, HV_INTERCEPT_ACCESS_MASK_READ, HV_INTERCEPT_ACCESS_MASK_WRITE,
+    HV_INTERCEPT_ACCESS_READ, HV_INTERCEPT_ACCESS_WRITE, SpecialRegisters, StandardRegisters,
+    XSave, hv_intercept_type_HV_INTERCEPT_TYPE_X64_MSR, 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_message_type_HVMSG_X64_MSR_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_user_mem_region,
+    hv_register_name_HV_X64_REGISTER_RIP, hv_register_value, mshv_install_intercept,
+    mshv_user_mem_region,
 };
 use mshv_ioctls::{Mshv, VcpuFd, VmFd};
 use tracing::{Span, instrument};
@@ -108,6 +112,18 @@ impl MshvVm {
 }
 
 impl VirtualMachine for MshvVm {
+    fn enable_msr_intercept(&mut self) -> std::result::Result<(), CreateVmError> {
+        let intercept = mshv_install_intercept {
+            access_type_mask: HV_INTERCEPT_ACCESS_MASK_WRITE | HV_INTERCEPT_ACCESS_MASK_READ,
+            intercept_type: hv_intercept_type_HV_INTERCEPT_TYPE_X64_MSR,
+            intercept_parameter: Default::default(),
+        };
+        self.vm_fd
+            .install_intercept(intercept)
+            .map_err(|e| CreateVmError::EnableMsrIntercept(e.into()))?;
+        Ok(())
+    }
+
     unsafe fn map_memory(
         &mut self,
         (_slot, region): (u32, &MemoryRegion),
@@ -137,6 +153,7 @@ impl VirtualMachine for MshvVm {
             hv_message_type_HVMSG_X64_IO_PORT_INTERCEPT;
         const UNMAPPED_GPA_MESSAGE: hv_message_type = hv_message_type_HVMSG_UNMAPPED_GPA;
         const INVALID_GPA_ACCESS_MESSAGE: hv_message_type = hv_message_type_HVMSG_GPA_INTERCEPT;
+        const MSR_MESSAGE: hv_message_type = hv_message_type_HVMSG_X64_MSR_INTERCEPT;
         #[cfg(gdb)]
         const EXCEPTION_INTERCEPT: hv_message_type = hv_message_type_HVMSG_X64_EXCEPTION_INTERCEPT;
 
@@ -196,6 +213,23 @@ impl VirtualMachine for MshvVm {
                         _ => VmExit::Unknown("Unknown MMIO access".to_string()),
                     }
                 }
+                MSR_MESSAGE => {
+                    let msr_message = m
+                        .to_msr_info()
+                        .map_err(|_| RunVcpuError::DecodeMsrMessage(m.header.message_type))?;
+                    let edx = msr_message.rdx;
+                    let eax = msr_message.rax;
+                    let written_value = (edx << 32) | eax;
+                    let access = msr_message.header.intercept_access_type as u32;
+                    match access {
+                        HV_INTERCEPT_ACCESS_READ => VmExit::MsrRead(msr_message.msr_number),
+                        HV_INTERCEPT_ACCESS_WRITE => VmExit::MsrWrite {
+                            msr_index: msr_message.msr_number,
+                            value: written_value,
+                        },
+                        _ => VmExit::Unknown(format!("Unknown MSR access type={}", access)),
+                    }
+                }
                 #[cfg(gdb)]
                 EXCEPTION_INTERCEPT => {
                     let ex_info = m

src/hyperlight_host/src/hypervisor/virtual_machine/mshv.rs

@@ -132,6 +132,23 @@ impl WhpVm {
 }
 
 impl VirtualMachine for WhpVm {
+    fn enable_msr_intercept(&mut self) -> std::result::Result<(), CreateVmError> {
+        let mut extended_exits_property = WHV_PARTITION_PROPERTY::default();
+        // X64MsrExit bit position (bit 1) in WHV_EXTENDED_VM_EXITS
+        // See https://learn.microsoft.com/en-us/virtualization/api/hypervisor-platform/funcs/whvpartitionpropertydatatypes
+        extended_exits_property.ExtendedVmExits.AsUINT64 = 1 << 1;
+        unsafe {
+            WHvSetPartitionProperty(
+                self.partition,
+                WHvPartitionPropertyCodeExtendedVmExits,
+                &extended_exits_property as *const _ as *const _,
+                std::mem::size_of::<WHV_PARTITION_PROPERTY>() as _,
+            )
+            .map_err(|e| CreateVmError::EnableMsrIntercept(e.into()))?
+        };
+        Ok(())
+    }
+
     unsafe fn map_memory(
         &mut self,
         (_slot, region): (u32, &MemoryRegion),
@@ -271,6 +288,21 @@ impl VirtualMachine for WhpVm {
             }
             // Execution was cancelled by the host.
             WHvRunVpExitReasonCanceled => VmExit::Cancelled(),
+            WHvRunVpExitReasonX64MsrAccess => {
+                let msr_access = unsafe { exit_context.Anonymous.MsrAccess };
+                let eax = msr_access.Rax;
+                let edx = msr_access.Rdx;
+                let written_value = (edx << 32) | eax;
+                let access = unsafe { msr_access.AccessInfo.AsUINT32 };
+                match access {
+                    0 => VmExit::MsrRead(msr_access.MsrNumber),
+                    1 => VmExit::MsrWrite {
+                        msr_index: msr_access.MsrNumber,
+                        value: written_value,
+                    },
+                    _ => VmExit::Unknown(format!("Unknown MSR access type={}", access)),
+                }
+            }
             #[cfg(gdb)]
             WHvRunVpExitReasonException => {
                 let exception = unsafe { exit_context.Anonymous.VpException };