diff --git a/Cargo.lock b/Cargo.lock index 4e191a298..fd72ecff8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1466,6 +1466,7 @@ dependencies = [ "criterion", "crossbeam-channel", "crossbeam-queue", + "displaydoc", "elfcore", "env_logger", "fallible-iterator", diff --git a/src/hyperlight_host/Cargo.toml b/src/hyperlight_host/Cargo.toml index a8642233f..d59f78795 100644 --- a/src/hyperlight_host/Cargo.toml +++ b/src/hyperlight_host/Cargo.toml @@ -51,6 +51,7 @@ metrics = "0.24.3" serde_json = "1.0" elfcore = "2.0" uuid = { version = "1.19.0", features = ["v4"] } +displaydoc = "0.2.5" [target.'cfg(windows)'.dependencies] windows = { version = "0.62", features = [ diff --git a/src/hyperlight_host/src/error.rs b/src/hyperlight_host/src/error.rs index 7da9a6eea..21ebd4e55 100644 --- a/src/hyperlight_host/src/error.rs +++ b/src/hyperlight_host/src/error.rs @@ -30,6 +30,7 @@ use hyperlight_common::flatbuffer_wrappers::function_types::{ParameterValue, Ret use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode; use thiserror::Error; +use crate::hypervisor::hyperlight_vm::HyperlightVmError; #[cfg(target_os = "windows")] use crate::hypervisor::wrappers::HandleWrapper; use crate::mem::memory_region::MemoryRegionFlags; @@ -111,6 +112,10 @@ pub enum HyperlightError { #[error("HostFunction {0} was not found")] HostFunctionNotFound(String), + /// Hyperlight VM error + #[error("Hyperlight VM error: {0}")] + HyperlightVmError(#[from] HyperlightVmError), + /// Reading Writing or Seeking data failed. #[error("Reading Writing or Seeking data failed {0:?}")] IOError(#[from] std::io::Error), @@ -127,11 +132,6 @@ pub enum HyperlightError { #[error("Conversion of str data to json failed")] JsonConversionFailure(#[from] serde_json::Error), - /// KVM Error Occurred - #[error("KVM Error {0:?}")] - #[cfg(kvm)] - KVMError(#[from] kvm_ioctls::Error), - /// An attempt to get a lock from a Mutex failed. #[error("Unable to lock resource")] LockAttemptFailed(String), @@ -168,11 +168,6 @@ pub enum HyperlightError { #[error("mprotect failed with os error {0:?}")] MprotectFailed(Option), - /// mshv Error Occurred - #[error("mshv Error {0:?}")] - #[cfg(mshv3)] - MSHVError(#[from] mshv_ioctls::MshvError), - /// No Hypervisor was found for Sandbox. #[error("No Hypervisor was found for Sandbox")] NoHypervisorFound(), @@ -242,11 +237,6 @@ pub enum HyperlightError { #[error("SystemTimeError {0:?}")] SystemTimeError(#[from] SystemTimeError), - /// Error occurred when translating guest address - #[error("An error occurred when translating guest address: {0:?}")] - #[cfg(gdb)] - TranslateGuestAddress(u64), - /// Error occurred converting a slice to an array #[error("TryFromSliceError {0:?}")] TryFromSliceError(#[from] TryFromSliceError), @@ -334,6 +324,11 @@ impl HyperlightError { | HyperlightError::SnapshotSizeMismatch(_, _) | HyperlightError::MemoryRegionSizeMismatch(_, _, _) => true, + // HyperlightVmError::DispatchGuestCall may poison the sandbox + HyperlightError::HyperlightVmError(HyperlightVmError::DispatchGuestCall(e)) => { + e.is_poison_error() + } + // All other errors do not poison the sandbox. HyperlightError::AnyhowError(_) | HyperlightError::BoundsCheckFailed(_, _) @@ -348,6 +343,10 @@ impl HyperlightError { | HyperlightError::GuestInterfaceUnsupportedType(_) | HyperlightError::GuestOffsetIsInvalid(_) | HyperlightError::HostFunctionNotFound(_) + | HyperlightError::HyperlightVmError(HyperlightVmError::Create(_)) + | HyperlightError::HyperlightVmError(HyperlightVmError::Initialize(_)) + | HyperlightError::HyperlightVmError(HyperlightVmError::MapRegion(_)) + | HyperlightError::HyperlightVmError(HyperlightVmError::UnmapRegion(_)) | HyperlightError::IOError(_) | HyperlightError::IntConversionFailure(_) | HyperlightError::InvalidFlatBuffer(_) @@ -384,12 +383,6 @@ impl HyperlightError { HyperlightError::WindowsAPIError(_) => false, #[cfg(target_os = "linux")] HyperlightError::VmmSysError(_) => false, - #[cfg(kvm)] - HyperlightError::KVMError(_) => false, - #[cfg(mshv3)] - HyperlightError::MSHVError(_) => false, - #[cfg(gdb)] - HyperlightError::TranslateGuestAddress(_) => false, } } } diff --git a/src/hyperlight_host/src/hypervisor/gdb/arch.rs b/src/hyperlight_host/src/hypervisor/gdb/arch.rs index 05f221e46..9afbc8a48 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/arch.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/arch.rs @@ -16,9 +16,18 @@ limitations under the License. //! This file contains architecture specific code for the x86_64 -use super::{DebuggableVm, VcpuStopReason}; -use crate::Result; +use super::{DebugError, DebuggableVm, VcpuStopReason}; use crate::hypervisor::regs::CommonRegisters; +use crate::hypervisor::virtual_machine::RegisterError; + +/// Errors that can occur when determining the vCPU stop reason +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum VcpuStopReasonError { + /// Failed to get registers: {0} + GetRegs(#[from] RegisterError), + /// Failed to remove hardware breakpoint: {0} + RemoveHwBreakpoint(#[from] DebugError), +} // Described in Table 6-1. Exceptions and Interrupts at Page 6-13 Vol. 1 // of Intel 64 and IA-32 Architectures Software Developer's Manual @@ -56,7 +65,7 @@ pub(crate) fn vcpu_stop_reason( dr6: u64, entrypoint: u64, exception: u32, -) -> Result { +) -> std::result::Result { let CommonRegisters { rip, .. } = vm.regs()?; if DB_EX_ID == exception { // If the BS flag in DR6 register is set, it means a single step diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 0e4d8ac9e..705a0b969 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -33,16 +33,16 @@ use x86_64_target::HyperlightSandboxTarget; use super::InterruptHandle; use super::regs::CommonRegisters; +use crate::HyperlightError; use crate::hypervisor::regs::CommonFpu; -use crate::hypervisor::virtual_machine::VirtualMachine; +use crate::hypervisor::virtual_machine::{HypervisorImplError, RegisterError, VirtualMachine}; use crate::mem::layout::SandboxMemoryLayout; use crate::mem::memory_region::MemoryRegion; use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::HostSharedMemory; -use crate::{HyperlightError, new_error}; #[derive(Debug, Error)] -pub(crate) enum GdbTargetError { +pub enum GdbTargetError { #[error("Error encountered while binding to address and port")] CannotBind, #[error("Error encountered while listening for connections")] @@ -86,6 +86,17 @@ pub(crate) struct DebugMemoryAccess { pub(crate) guest_mmap_regions: Vec, } +/// Errors that can occur during debug memory access operations +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum DebugMemoryAccessError { + /// Failed to translate guest address {0:#x} + TranslateGuestAddress(u64), + /// Failed to acquire lock at {0}:{1} - {2} + LockFailed(&'static str, u32, String), + /// Failed to copy memory: {0} + CopyFailed(Box), +} + impl DebugMemoryAccess { /// Reads memory from the guest's address space with a maximum length of a PAGE_SIZE /// @@ -94,8 +105,12 @@ impl DebugMemoryAccess { /// * `gpa` - Guest physical address to read from. /// This address is shall be translated before calling this function /// # Returns - /// * `Result<(), HyperlightError>` - Ok if successful, Err otherwise - pub(crate) fn read(&self, data: &mut [u8], gpa: u64) -> crate::Result<()> { + /// * `Result<(), DebugMemoryAccessError>` - Ok if successful, Err otherwise + pub(crate) fn read( + &self, + data: &mut [u8], + gpa: u64, + ) -> std::result::Result<(), DebugMemoryAccessError> { let read_len = data.len(); let mem_offset = (gpa as usize) @@ -107,7 +122,7 @@ impl DebugMemoryAccess { gpa, SandboxMemoryLayout::BASE_ADDRESS ); - HyperlightError::TranslateGuestAddress(gpa) + DebugMemoryAccessError::TranslateGuestAddress(gpa) })?; // First check the mapped memory regions to see if the address is within any of them @@ -123,7 +138,7 @@ impl DebugMemoryAccess { mem_offset, reg.guest_region.start, ); - HyperlightError::TranslateGuestAddress(mem_offset as u64) + DebugMemoryAccessError::TranslateGuestAddress(mem_offset as u64) })?; let bytes: &[u8] = unsafe { @@ -144,9 +159,10 @@ impl DebugMemoryAccess { self.dbg_mem_access_fn .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? + .map_err(|e| DebugMemoryAccessError::LockFailed(file!(), line!(), e.to_string()))? .get_shared_mem_mut() - .copy_to_slice(&mut data[..read_len], mem_offset)?; + .copy_to_slice(&mut data[..read_len], mem_offset) + .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e)))?; } Ok(()) @@ -159,8 +175,12 @@ impl DebugMemoryAccess { /// * `gpa` - Guest physical address to write to. /// This address is shall be translated before calling this function /// # Returns - /// * `Result<(), HyperlightError>` - Ok if successful, Err otherwise - pub(crate) fn write(&self, data: &[u8], gpa: u64) -> crate::Result<()> { + /// * `Result<(), DebugMemoryAccessError>` - Ok if successful, Err otherwise + pub(crate) fn write( + &self, + data: &[u8], + gpa: u64, + ) -> std::result::Result<(), DebugMemoryAccessError> { let write_len = data.len(); let mem_offset = (gpa as usize) @@ -172,7 +192,7 @@ impl DebugMemoryAccess { gpa, SandboxMemoryLayout::BASE_ADDRESS ); - HyperlightError::TranslateGuestAddress(gpa) + DebugMemoryAccessError::TranslateGuestAddress(gpa) })?; // First check the mapped memory regions to see if the address is within any of them @@ -188,7 +208,7 @@ impl DebugMemoryAccess { mem_offset, reg.guest_region.start, ); - HyperlightError::TranslateGuestAddress(mem_offset as u64) + DebugMemoryAccessError::TranslateGuestAddress(mem_offset as u64) })?; let bytes: &mut [u8] = unsafe { @@ -213,9 +233,10 @@ impl DebugMemoryAccess { self.dbg_mem_access_fn .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? + .map_err(|e| DebugMemoryAccessError::LockFailed(file!(), line!(), e.to_string()))? .get_shared_mem_mut() - .copy_from_slice(&data[..write_len], mem_offset)?; + .copy_from_slice(&data[..write_len], mem_offset) + .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e)))?; } Ok(()) @@ -275,24 +296,42 @@ pub(crate) enum DebugResponse { WriteRegisters, } +/// Errors that can occur during debug operations +#[derive(Debug, Clone, thiserror::Error, displaydoc::Display)] +pub enum DebugError { + /// Hardware breakpoint not found at address {0:#x} + HwBreakpointNotFound(u64), + /// Maximum hardware breakpoints ({0}) exceeded + TooManyHwBreakpoints(usize), + /// Register operation failed: {0} + Register(#[from] RegisterError), + /// Translation of guest virtual address failed: {0} + TranslateGva(u64), + /// Failed to enable/disable intercept: {enable}, {inner} + Intercept { + enable: bool, + inner: HypervisorImplError, + }, +} + /// Trait for VMs that support debugging capabilities. /// This extends the base VirtualMachine trait with GDB-specific functionality. pub(crate) trait DebuggableVm: VirtualMachine { /// Translates a guest virtual address to a guest physical address - fn translate_gva(&self, gva: u64) -> crate::Result; + fn translate_gva(&self, gva: u64) -> std::result::Result; /// Enable/disable debugging - fn set_debug(&mut self, enable: bool) -> crate::Result<()>; + fn set_debug(&mut self, enable: bool) -> std::result::Result<(), DebugError>; /// Enable/disable single stepping - fn set_single_step(&mut self, enable: bool) -> crate::Result<()>; + fn set_single_step(&mut self, enable: bool) -> std::result::Result<(), DebugError>; /// Add a hardware breakpoint at the given address. /// Must be idempotent. - fn add_hw_breakpoint(&mut self, addr: u64) -> crate::Result<()>; + fn add_hw_breakpoint(&mut self, addr: u64) -> std::result::Result<(), DebugError>; /// Remove a hardware breakpoint at the given address - fn remove_hw_breakpoint(&mut self, addr: u64) -> crate::Result<()>; + fn remove_hw_breakpoint(&mut self, addr: u64) -> std::result::Result<(), DebugError>; } /// Debug communication channel that is used for sending a request type and @@ -513,7 +552,9 @@ mod tests { } let mut read_data = [0u8; 1]; - mem_access.read(&mut read_data, (BASE_VIRT + offset) as u64)?; + mem_access + .read(&mut read_data, (BASE_VIRT + offset) as u64) + .unwrap(); assert_eq!(read_data[0], 0xAA); @@ -536,7 +577,9 @@ mod tests { } let mut read_data = [0u8; 16]; - mem_access.read(&mut read_data, (BASE_VIRT + offset) as u64)?; + mem_access + .read(&mut read_data, (BASE_VIRT + offset) as u64) + .unwrap(); assert_eq!( read_data, @@ -556,7 +599,9 @@ mod tests { } let write_data = [0xCCu8; 1]; - mem_access.write(&write_data, (BASE_VIRT + offset) as u64)?; + mem_access + .write(&write_data, (BASE_VIRT + offset) as u64) + .unwrap(); let slice = unsafe { get_mmap_slice(&mut mem_access) }; assert_eq!(slice[offset], write_data[0]); @@ -577,7 +622,9 @@ mod tests { } let write_data = [0xAAu8; 16]; - mem_access.write(&write_data, (BASE_VIRT + offset) as u64)?; + mem_access + .write(&write_data, (BASE_VIRT + offset) as u64) + .unwrap(); let slice = unsafe { get_mmap_slice(&mut mem_access) }; assert_eq!(slice[offset..offset + 16], write_data); diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm.rs index c836d3d80..5b52e9411 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm.rs @@ -31,15 +31,23 @@ use tracing::{Span, instrument}; use tracing_opentelemetry::OpenTelemetrySpanExt; #[cfg(gdb)] -use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, DebuggableVm, VcpuStopReason, arch}; +use super::gdb::arch::VcpuStopReasonError; +#[cfg(gdb)] +use super::gdb::{ + DebugCommChannel, DebugMsg, DebugResponse, DebuggableVm, GdbTargetError, VcpuStopReason, arch, +}; use super::regs::{CommonFpu, CommonRegisters}; #[cfg(target_os = "windows")] use super::{PartitionState, WindowsInterruptHandle}; -use crate::HyperlightError::{ExecutionCanceledByHost, NoHypervisorFound}; +use crate::HyperlightError; #[cfg(any(kvm, mshv3))] use crate::hypervisor::LinuxInterruptHandle; #[cfg(crashdump)] use crate::hypervisor::crashdump; +#[cfg(gdb)] +use crate::hypervisor::gdb::{DebugError, DebugMemoryAccessError}; +#[cfg(gdb)] +use crate::hypervisor::hyperlight_vm::debug::ProcessDebugRequestError; use crate::hypervisor::regs::CommonSpecialRegisters; #[cfg(not(gdb))] use crate::hypervisor::virtual_machine::VirtualMachine; @@ -49,7 +57,10 @@ use crate::hypervisor::virtual_machine::kvm::KvmVm; use crate::hypervisor::virtual_machine::mshv::MshvVm; #[cfg(target_os = "windows")] use crate::hypervisor::virtual_machine::whp::WhpVm; -use crate::hypervisor::virtual_machine::{HypervisorType, VmExit, get_available_hypervisor}; +use crate::hypervisor::virtual_machine::{ + HypervisorType, MapMemoryError, RegisterError, RunVcpuError, UnmapMemoryError, VmError, VmExit, + get_available_hypervisor, +}; #[cfg(target_os = "windows")] use crate::hypervisor::wrappers::HandleWrapper; use crate::hypervisor::{InterruptHandle, InterruptHandleImpl, get_max_log_level}; @@ -60,12 +71,11 @@ use crate::mem::shared_mem::HostSharedMemory; use crate::metrics::{METRIC_ERRONEOUS_VCPU_KICKS, METRIC_GUEST_CANCELLATION}; use crate::sandbox::SandboxConfiguration; use crate::sandbox::host_funcs::FunctionRegistry; -use crate::sandbox::outb::handle_outb; +use crate::sandbox::outb::{HandleOutbError, handle_outb}; #[cfg(feature = "mem_profile")] use crate::sandbox::trace::MemTraceInfo; #[cfg(crashdump)] use crate::sandbox::uninitialized::SandboxRuntimeConfig; -use crate::{HyperlightError, Result, log_then_return, new_error}; /// Represents a Hyperlight Virtual Machine instance. /// @@ -98,6 +108,211 @@ pub(crate) struct HyperlightVm { rt_cfg: SandboxRuntimeConfig, } +/// DispatchGuestCall error +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum DispatchGuestCallError { + /// Failed to convert RSP pointer: {0} + ConvertRspPointer(String), + /// Failed to setup registers: {0} + SetupRegs(RegisterError), + /// Failed to run vm: {0} + Run(#[from] RunVmError), +} + +impl DispatchGuestCallError { + /// Returns true if this error should poison the sandbox + pub(crate) fn is_poison_error(&self) -> bool { + match self { + // These errors poison the sandbox because they can leave it in an inconsistent state + // by returning before the guest can unwind properly + DispatchGuestCallError::Run(_) => true, + DispatchGuestCallError::ConvertRspPointer(_) | DispatchGuestCallError::SetupRegs(_) => { + false + } + } + } + + /// Converts a `DispatchGuestCallError` to a `HyperlightError`. Used for backwards compatibility. + /// Also determines if the sandbox should be poisoned. + /// + /// Returns a tuple of (error, should_poison) where should_poison indicates whether + /// the sandbox should be marked as poisoned due to incomplete guest execution. + pub(crate) fn promote(self) -> (HyperlightError, bool) { + let should_poison = self.is_poison_error(); + let promoted_error = match self { + // These errors poison the sandbox because the guest did not run to completion + DispatchGuestCallError::Run(RunVmError::StackOverflow) + | DispatchGuestCallError::Run(RunVmError::HandleIo(HandleIoError::Outb( + HandleOutbError::StackOverflow, + ))) => HyperlightError::StackOverflow(), + + DispatchGuestCallError::Run(RunVmError::ExecutionCancelledByHost) => { + HyperlightError::ExecutionCanceledByHost() + } + + DispatchGuestCallError::Run(RunVmError::HandleIo(HandleIoError::Outb( + HandleOutbError::GuestAborted { code, message }, + ))) => HyperlightError::GuestAborted(code, message), + + DispatchGuestCallError::Run(RunVmError::MemoryAccessViolation { + addr, + access_type, + region_flags, + }) => HyperlightError::MemoryAccessViolation(addr, access_type, region_flags), + + // Leave others as is + other => HyperlightVmError::DispatchGuestCall(other).into(), + }; + (promoted_error, should_poison) + } +} + +/// Initialize error +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum InitializeError { + /// Failed to convert pointer: {0} + ConvertPointer(String), + /// Failed to setup registers: {0} + SetupRegs(#[from] RegisterError), + /// Failed to run vm: {0} + Run(#[from] RunVmError), +} + +/// Errors that can occur during VM execution in the run loop +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum RunVmError { + /// Stack overflow detected + StackOverflow, + /// Memory access violation at address {addr:#x}: {access_type} access, but memory is marked as {region_flags} + MemoryAccessViolation { + addr: u64, + access_type: MemoryRegionFlags, + region_flags: MemoryRegionFlags, + }, + /// MMIO READ access to unmapped address {0:#x} + MmioReadUnmapped(u64), + /// MMIO WRITE access to unmapped address {0:#x} + MmioWriteUnmapped(u64), + /// Execution was cancelled by the host + ExecutionCancelledByHost, + /// Unexpected VM exit: {0} + UnexpectedVmExit(String), + /// vCPU run failed: {0} + RunVcpu(#[from] RunVcpuError), + /// IO handling error: {0} + HandleIo(#[from] HandleIoError), + /// Failed to get registers: {0} + #[cfg(feature = "trace_guest")] + GetRegs(RegisterError), + /// Error checking stack guard: {0} + CheckStackGuard(Box), + /// Debug handler error: {0} + #[cfg(gdb)] + DebugHandler(#[from] HandleDebugError), + /// vCPU stop reason error: {0} + #[cfg(gdb)] + VcpuStopReason(#[from] VcpuStopReasonError), + /// Crashdump generation error: {0} + #[cfg(crashdump)] + CrashdumpGeneration(Box), +} + +/// Errors that can occur during IO (outb) handling +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum HandleIoError { + /// No data was given in IO interrupt + NoData, + /// Failed to get registers: {0} + #[cfg(feature = "mem_profile")] + GetRegs(RegisterError), + /// {0} + Outb(#[from] HandleOutbError), +} + +/// Errors that can occur when mapping a memory region +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum MapRegionError { + /// Region is not page-aligned (page size: {0:#x}) + NotPageAligned(usize), + /// VM map memory error: {0} + MapMemory(#[from] MapMemoryError), +} + +/// Errors that can occur when unmapping a memory region +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum UnmapRegionError { + /// Region not found in mapped regions + RegionNotFound, + /// VM unmap memory error: {0} + UnmapMemory(#[from] UnmapMemoryError), +} + +/// Errors that can occur during HyperlightVm creation +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum CreateHyperlightVmError { + /// No hypervisor was found + NoHypervisorFound, + /// VM operation error: {0} + Vm(#[from] VmError), + /// Failed to convert RSP pointer: {0} + ConvertRspPointer(Box), + /// Failed to send debug message: {0} + #[cfg(gdb)] + SendDbgMsg(#[from] SendDbgMsgError), + /// Failed to add hardware breakpoint: {0} + #[cfg(gdb)] + AddHwBreakpoint(DebugError), +} + +/// Errors that can occur during debug exit handling +#[cfg(gdb)] +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum HandleDebugError { + /// Debug is not enabled + DebugNotEnabled, + /// Failed to send message to GDB thread: {0} + SendMessage(#[from] SendDbgMsgError), + /// Failed to receive message from GDB thread: {0} + ReceiveMessage(#[from] RecvDbgMsgError), + /// Error processing debug request: {0} + ProcessRequest(#[from] ProcessDebugRequestError), +} + +/// Errors that can occur when sending a debug message +#[cfg(gdb)] +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum SendDbgMsgError { + /// Debug is not enabled + DebugNotEnabled, + /// Failed to send message: {0} + SendFailed(#[from] GdbTargetError), +} + +/// Errors that can occur when receiving a debug message +#[cfg(gdb)] +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum RecvDbgMsgError { + /// Debug is not enabled + DebugNotEnabled, + /// Failed to receive message: {0} + RecvFailed(#[from] GdbTargetError), +} + +/// Unified error type for all HyperlightVm operations +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum HyperlightVmError { + /// Dispatch guest call error: {0} + DispatchGuestCall(#[from] DispatchGuestCallError), + /// Initialize error: {0} + Initialize(#[from] InitializeError), + /// Create VM error: {0} + Create(#[from] CreateHyperlightVmError), + /// Map region error: {0} + MapRegion(#[from] MapRegionError), + /// Unmap region error: {0} + UnmapRegion(#[from] UnmapRegionError), +} + impl HyperlightVm { /// Create a new HyperlightVm instance (will not run vm until calling `initialise`) #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] @@ -113,7 +328,7 @@ impl HyperlightVm { #[cfg(gdb)] gdb_conn: Option>, #[cfg(crashdump)] rt_cfg: SandboxRuntimeConfig, #[cfg(feature = "mem_profile")] trace_info: MemTraceInfo, - ) -> Result { + ) -> std::result::Result { #[cfg(gdb)] type VmType = Box; #[cfg(not(gdb))] @@ -122,17 +337,22 @@ impl HyperlightVm { #[cfg_attr(not(gdb), allow(unused_mut))] let mut vm: VmType = match get_available_hypervisor() { #[cfg(kvm)] - Some(HypervisorType::Kvm) => Box::new(KvmVm::new()?), + Some(HypervisorType::Kvm) => Box::new(KvmVm::new().map_err(VmError::CreateVm)?), #[cfg(mshv3)] - Some(HypervisorType::Mshv) => Box::new(MshvVm::new()?), + Some(HypervisorType::Mshv) => Box::new(MshvVm::new().map_err(VmError::CreateVm)?), #[cfg(target_os = "windows")] - Some(HypervisorType::Whp) => Box::new(WhpVm::new(handle, raw_size)?), - None => return Err(NoHypervisorFound()), + Some(HypervisorType::Whp) => { + Box::new(WhpVm::new(handle, raw_size).map_err(VmError::CreateVm)?) + } + None => return Err(CreateHyperlightVmError::NoHypervisorFound), }; for (i, region) in mem_regions.iter().enumerate() { // Safety: slots are unique and region points to valid memory since we created the regions - unsafe { vm.map_memory((i as u32, region))? }; + unsafe { + vm.map_memory((i as u32, region)) + .map_err(VmError::MapMemory)? + }; } // Mark initial setup as complete for Windows - subsequent map_memory calls will fail @@ -140,10 +360,13 @@ impl HyperlightVm { vm.complete_initial_memory_setup(); #[cfg(feature = "init-paging")] - vm.set_sregs(&CommonSpecialRegisters::standard_64bit_defaults(_pml4_addr))?; + vm.set_sregs(&CommonSpecialRegisters::standard_64bit_defaults(_pml4_addr)) + .map_err(VmError::Register)?; #[cfg(not(feature = "init-paging"))] - vm.set_sregs(&CommonSpecialRegisters::standard_real_mode_defaults())?; - let rsp_gp = GuestPtr::try_from(RawPtr::from(rsp))?; + vm.set_sregs(&CommonSpecialRegisters::standard_real_mode_defaults()) + .map_err(VmError::Register)?; + let rsp_gp = GuestPtr::try_from(RawPtr::from(rsp)) + .map_err(|e| CreateHyperlightVmError::ConvertRspPointer(Box::new(e)))?; #[cfg(any(kvm, mshv3))] let interrupt_handle: Arc = Arc::new(LinuxInterruptHandle { @@ -205,8 +428,10 @@ impl HyperlightVm { if ret.gdb_conn.is_some() { ret.send_dbg_msg(DebugResponse::InterruptHandle(ret.interrupt_handle.clone()))?; // Add breakpoint to the entry point address - ret.vm.set_debug(true)?; - ret.vm.add_hw_breakpoint(entrypoint)?; + ret.vm.set_debug(true).map_err(VmError::Debug)?; + ret.vm + .add_hw_breakpoint(entrypoint) + .map_err(CreateHyperlightVmError::AddHwBreakpoint)?; } Ok(ret) @@ -225,7 +450,7 @@ impl HyperlightVm { host_funcs: &Arc>, guest_max_log_level: Option, #[cfg(gdb)] dbg_mem_access_fn: Arc>>, - ) -> Result<()> { + ) -> std::result::Result<(), InitializeError> { self.page_size = page_size as usize; let guest_max_log_level: u64 = match guest_max_log_level { @@ -235,7 +460,10 @@ impl HyperlightVm { let regs = CommonRegisters { rip: self.entrypoint, - rsp: self.orig_rsp.absolute()?, + rsp: self + .orig_rsp + .absolute() + .map_err(|e| InitializeError::ConvertPointer(e.to_string()))?, // function args rdi: peb_addr.into(), @@ -254,6 +482,7 @@ impl HyperlightVm { #[cfg(gdb)] dbg_mem_access_fn, ) + .map_err(InitializeError::Run) } /// Map a region of host memory into the sandbox. @@ -262,7 +491,10 @@ impl HyperlightVm { /// that the memory is valid for the duration of Self's lifetime. /// Depending on the host platform, there are likely alignment /// requirements of at least one page for base and len. - pub(crate) unsafe fn map_region(&mut self, region: &MemoryRegion) -> Result<()> { + pub(crate) unsafe fn map_region( + &mut self, + region: &MemoryRegion, + ) -> std::result::Result<(), MapRegionError> { if [ region.guest_region.start, region.guest_region.end, @@ -272,10 +504,7 @@ impl HyperlightVm { .iter() .any(|x| x % self.page_size != 0) { - log_then_return!( - "region is not page-aligned {:x}, {region:?}", - self.page_size - ); + return Err(MapRegionError::NotPageAligned(self.page_size)); } // Try to reuse a freed slot first, otherwise use next_slot @@ -294,12 +523,15 @@ impl HyperlightVm { } /// Unmap a memory region from the sandbox - pub(crate) fn unmap_region(&mut self, region: &MemoryRegion) -> Result<()> { + pub(crate) fn unmap_region( + &mut self, + region: &MemoryRegion, + ) -> std::result::Result<(), UnmapRegionError> { let pos = self .mmap_regions .iter() .position(|(_, r)| r == region) - .ok_or_else(|| new_error!("Region not found in mapped regions"))?; + .ok_or(UnmapRegionError::RegionNotFound)?; let (slot, _) = self.mmap_regions.remove(pos); self.freed_slots.push(slot); @@ -326,18 +558,25 @@ impl HyperlightVm { mem_mgr: &mut SandboxMemoryManager, host_funcs: &Arc>, #[cfg(gdb)] dbg_mem_access_fn: Arc>>, - ) -> Result<()> { + ) -> std::result::Result<(), DispatchGuestCallError> { // set RIP and RSP, reset others let regs = CommonRegisters { rip: dispatch_func_addr.into(), - rsp: self.orig_rsp.absolute()?, + rsp: self + .orig_rsp + .absolute() + .map_err(|e| DispatchGuestCallError::ConvertRspPointer(e.to_string()))?, rflags: 1 << 1, ..Default::default() }; - self.vm.set_regs(®s)?; + self.vm + .set_regs(®s) + .map_err(DispatchGuestCallError::SetupRegs)?; // reset fpu - self.vm.set_fpu(&CommonFpu::default())?; + self.vm + .set_fpu(&CommonFpu::default()) + .map_err(DispatchGuestCallError::SetupRegs)?; self.run( mem_mgr, @@ -345,6 +584,7 @@ impl HyperlightVm { #[cfg(gdb)] dbg_mem_access_fn, ) + .map_err(DispatchGuestCallError::Run) } pub(crate) fn interrupt_handle(&self) -> Arc { @@ -360,7 +600,7 @@ impl HyperlightVm { mem_mgr: &mut SandboxMemoryManager, host_funcs: &Arc>, #[cfg(gdb)] dbg_mem_access_fn: Arc>>, - ) -> Result<()> { + ) -> std::result::Result<(), RunVmError> { // Keeps the trace context and open spans #[cfg(feature = "trace_guest")] let mut tc = crate::sandbox::trace::TraceContext::new(); @@ -395,7 +635,7 @@ impl HyperlightVm { { tc.end_host_trace(); // Handle the guest trace data if any - let regs = self.vm.regs()?; + let regs = self.vm.regs().map_err(RunVmError::GetRegs)?; if let Err(e) = tc.handle_trace(®s, mem_mgr) { // If no trace data is available, we just log a message and continue // Is this the right thing to do? @@ -429,14 +669,16 @@ impl HyperlightVm { let stop_reason = arch::vcpu_stop_reason(self.vm.as_mut(), dr6, self.entrypoint, exception)?; if let Err(e) = self.handle_debug(dbg_mem_access_fn.clone(), stop_reason) { - break Err(e); + break Err(e.into()); } } Ok(VmExit::Halt()) => { break Ok(()); } - Ok(VmExit::IoOut(port, data)) => self.handle_io(mem_mgr, host_funcs, port, data)?, + Ok(VmExit::IoOut(port, data)) => { + self.handle_io(mem_mgr, host_funcs, port, data)?; + } Ok(VmExit::MmioRead(addr)) => { let all_regions = self.sandbox_regions.iter().chain(self.get_mapped_regions()); match get_memory_access_violation( @@ -445,21 +687,24 @@ impl HyperlightVm { all_regions, ) { Some(MemoryAccess::StackGuardPageViolation) => { - break Err(HyperlightError::StackOverflow()); + break Err(RunVmError::StackOverflow); } Some(MemoryAccess::AccessViolation(region_flags)) => { - break Err(HyperlightError::MemoryAccessViolation( + break Err(RunVmError::MemoryAccessViolation { addr, - MemoryRegionFlags::READ, + access_type: MemoryRegionFlags::READ, region_flags, - )); + }); } None => { - if !mem_mgr.check_stack_guard()? { - break Err(HyperlightError::StackOverflow()); + if !mem_mgr + .check_stack_guard() + .map_err(|e| RunVmError::CheckStackGuard(Box::new(e)))? + { + break Err(RunVmError::StackOverflow); } - break Err(new_error!("MMIO READ access address {:#x}", addr)); + break Err(RunVmError::MmioReadUnmapped(addr)); } } } @@ -471,21 +716,24 @@ impl HyperlightVm { all_regions, ) { Some(MemoryAccess::StackGuardPageViolation) => { - break Err(HyperlightError::StackOverflow()); + break Err(RunVmError::StackOverflow); } Some(MemoryAccess::AccessViolation(region_flags)) => { - break Err(HyperlightError::MemoryAccessViolation( + break Err(RunVmError::MemoryAccessViolation { addr, - MemoryRegionFlags::WRITE, + access_type: MemoryRegionFlags::WRITE, region_flags, - )); + }); } None => { - if !mem_mgr.check_stack_guard()? { - break Err(HyperlightError::StackOverflow()); + if !mem_mgr + .check_stack_guard() + .map_err(|e| RunVmError::CheckStackGuard(Box::new(e)))? + { + break Err(RunVmError::StackOverflow); } - break Err(new_error!("MMIO WRITE access address {:#x}", addr)); + break Err(RunVmError::MmioWriteUnmapped(addr)); } } } @@ -508,43 +756,43 @@ impl HyperlightVm { if let Err(e) = self.handle_debug(dbg_mem_access_fn.clone(), VcpuStopReason::Interrupt) { - break Err(e); + break Err(e.into()); } } metrics::counter!(METRIC_GUEST_CANCELLATION).increment(1); - break Err(ExecutionCanceledByHost()); + break Err(RunVmError::ExecutionCancelledByHost); } Ok(VmExit::Unknown(reason)) => { - break Err(new_error!("Unexpected VM Exit: {:?}", reason)); + break Err(RunVmError::UnexpectedVmExit(reason)); } Ok(VmExit::Retry()) => continue, Err(e) => { - break Err(e); + break Err(RunVmError::RunVcpu(e)); } } }; match result { Ok(_) => Ok(()), - Err(HyperlightError::ExecutionCanceledByHost()) => { + Err(RunVmError::ExecutionCancelledByHost) => { // no need to crashdump this - Err(HyperlightError::ExecutionCanceledByHost()) + Err(RunVmError::ExecutionCancelledByHost) } Err(e) => { #[cfg(crashdump)] if self.rt_cfg.guest_core_dump { - crashdump::generate_crashdump(self)?; + crashdump::generate_crashdump(self) + .map_err(|e| RunVmError::CrashdumpGeneration(Box::new(e)))?; } // If GDB is enabled, we handle the debug memory access // Disregard return value as we want to return the error #[cfg(gdb)] if self.gdb_conn.is_some() { - self.handle_debug(dbg_mem_access_fn.clone(), VcpuStopReason::Crash)?; + self.handle_debug(dbg_mem_access_fn.clone(), VcpuStopReason::Crash)? } - - log_then_return!(e); + Err(e) } } } @@ -556,9 +804,9 @@ impl HyperlightVm { host_funcs: &Arc>, port: u16, data: Vec, - ) -> Result<()> { + ) -> std::result::Result<(), HandleIoError> { if data.is_empty() { - log_then_return!("no data was given in IO interrupt"); + return Err(HandleIoError::NoData); } #[allow(clippy::get_first)] @@ -571,7 +819,7 @@ impl HyperlightVm { #[cfg(feature = "mem_profile")] { - let regs = self.vm.regs()?; + let regs = self.vm.regs().map_err(HandleIoError::GetRegs)?; handle_outb(mem_mgr, host_funcs, port, val, ®s, &mut self.trace_info)?; } @@ -589,11 +837,12 @@ impl HyperlightVm { &mut self, dbg_mem_access_fn: Arc>>, stop_reason: VcpuStopReason, - ) -> Result<()> { + ) -> std::result::Result<(), HandleDebugError> { use crate::hypervisor::gdb::DebugMemoryAccess; + use crate::hypervisor::hyperlight_vm::debug::ProcessDebugRequestError; if self.gdb_conn.is_none() { - return Err(new_error!("Debugging is not enabled")); + return Err(HandleDebugError::DebugNotEnabled); } let mem_access = DebugMemoryAccess { @@ -607,10 +856,7 @@ impl HyperlightVm { // because the guest has crashed. // We only allow reading registers and memory VcpuStopReason::Crash => { - self.send_dbg_msg(DebugResponse::VcpuStopped(stop_reason)) - .map_err(|e| { - new_error!("Couldn't signal vCPU stopped event to GDB thread: {:?}", e) - })?; + self.send_dbg_msg(DebugResponse::VcpuStopped(stop_reason))?; loop { log::debug!("Debug wait for event to resume vCPU"); @@ -647,28 +893,29 @@ impl HyperlightVm { let result = self.process_dbg_request(req, &mem_access); match result { Ok(response) => response, - Err(HyperlightError::TranslateGuestAddress(_)) => { - // Treat non fatal errors separately so the guest doesn't fail - DebugResponse::ErrorOccurred - } + // Treat non-fatal errors separately so the guest doesn't fail + Err(ProcessDebugRequestError::ReadMemory( + DebugMemoryAccessError::TranslateGuestAddress(_), + )) + | Err(ProcessDebugRequestError::Debug(DebugError::TranslateGva( + _, + ))) => DebugResponse::ErrorOccurred, Err(e) => { log::error!("Error processing debug request: {:?}", e); - return Err(e); + return Err(HandleDebugError::ProcessRequest(e)); } } } }; // Send the response to the request back to gdb - self.send_dbg_msg(response) - .map_err(|e| new_error!("Couldn't send response to gdb: {:?}", e))?; + self.send_dbg_msg(response)?; // If we are denying continue or step requests, the debugger assumes the // execution started so we need to report a stop reason as a crash and let // it request to read registers/memory to figure out what happened if deny_continue { - self.send_dbg_msg(DebugResponse::VcpuStopped(VcpuStopReason::Crash)) - .map_err(|e| new_error!("Couldn't send response to gdb: {:?}", e))?; + self.send_dbg_msg(DebugResponse::VcpuStopped(VcpuStopReason::Crash))?; } // If we are detaching, we will break the loop and the Hypervisor will continue @@ -682,10 +929,7 @@ impl HyperlightVm { // normally _ => { // Send the stop reason to the gdb thread - self.send_dbg_msg(DebugResponse::VcpuStopped(stop_reason)) - .map_err(|e| { - new_error!("Couldn't signal vCPU stopped event to GDB thread: {:?}", e) - })?; + self.send_dbg_msg(DebugResponse::VcpuStopped(stop_reason))?; loop { log::debug!("Debug wait for event to resume vCPU"); @@ -696,12 +940,15 @@ impl HyperlightVm { let response = match result { Ok(response) => response, - // Treat non fatal errors separately so the guest doesn't fail - Err(HyperlightError::TranslateGuestAddress(_)) => { + // Treat non-fatal errors separately so the guest doesn't fail + Err(ProcessDebugRequestError::ReadMemory( + DebugMemoryAccessError::TranslateGuestAddress(_), + )) + | Err(ProcessDebugRequestError::Debug(DebugError::TranslateGva(_))) => { DebugResponse::ErrorOccurred } Err(e) => { - return Err(e); + return Err(HandleDebugError::ProcessRequest(e)); } }; @@ -710,8 +957,7 @@ impl HyperlightVm { DebugResponse::Continue | DebugResponse::Step | DebugResponse::DisableDebug ); - self.send_dbg_msg(response) - .map_err(|e| new_error!("Couldn't send response to gdb: {:?}", e))?; + self.send_dbg_msg(response)?; // Check if we should continue execution // We continue if the response is one of the following: Step, Continue, or DisableDebug @@ -726,7 +972,9 @@ impl HyperlightVm { } #[cfg(crashdump)] - pub(crate) fn crashdump_context(&self) -> Result> { + pub(crate) fn crashdump_context( + &self, + ) -> std::result::Result, RegisterError> { if self.rt_cfg.guest_core_dump { let mut regs = [0; 27]; @@ -826,49 +1074,63 @@ mod debug { use super::HyperlightVm; use crate::hypervisor::gdb::arch::{SW_BP, SW_BP_SIZE}; - use crate::hypervisor::gdb::{DebugMemoryAccess, DebugMsg, DebugResponse}; - use crate::{Result, new_error}; + use crate::hypervisor::gdb::{ + DebugError, DebugMemoryAccess, DebugMemoryAccessError, DebugMsg, DebugResponse, + }; + use crate::hypervisor::virtual_machine::VmError; + + /// Errors that can occur during GDB debug request processing + #[derive(Debug, thiserror::Error, displaydoc::Display)] + pub enum ProcessDebugRequestError { + /// Debug is not enabled + DebugNotEnabled, + /// Failed to acquire lock at {0}:{1} + TryLockError(&'static str, u32), + /// VM operation error: {0} + Vm(#[from] VmError), + /// Debug operation error: {0} + Debug(#[from] DebugError), + /// Address {0:#x} is not a software breakpoint + SwBreakpointNotFound(u64), + /// Failed to read memory: {0} + ReadMemory(#[from] DebugMemoryAccessError), + /// Failed to write memory: {0} + WriteMemory(DebugMemoryAccessError), + } impl HyperlightVm { pub(crate) fn process_dbg_request( &mut self, req: DebugMsg, mem_access: &DebugMemoryAccess, - ) -> Result { + ) -> std::result::Result { if self.gdb_conn.is_some() { match req { DebugMsg::AddHwBreakpoint(addr) => Ok(DebugResponse::AddHwBreakpoint( self.vm .add_hw_breakpoint(addr) - .map_err(|e| { + .inspect_err(|e| { log::error!("Failed to add hw breakpoint: {:?}", e); - - e }) .is_ok(), )), DebugMsg::AddSwBreakpoint(addr) => Ok(DebugResponse::AddSwBreakpoint( self.add_sw_breakpoint(addr, mem_access) - .map_err(|e| { + .inspect_err(|e| { log::error!("Failed to add sw breakpoint: {:?}", e); - - e }) .is_ok(), )), DebugMsg::Continue => { - self.vm.set_single_step(false).map_err(|e| { + self.vm.set_single_step(false).inspect_err(|e| { log::error!("Failed to continue execution: {:?}", e); - - e })?; Ok(DebugResponse::Continue) } DebugMsg::DisableDebug => { - self.vm.set_debug(false).map_err(|e| { + self.vm.set_debug(false).inspect_err(|e| { log::error!("Failed to disable debugging: {:?}", e); - e })?; Ok(DebugResponse::DisableDebug) @@ -877,9 +1139,7 @@ mod debug { let offset = mem_access .dbg_mem_access_fn .try_lock() - .map_err(|e| { - new_error!("Error locking at {}:{}: {}", file!(), line!(), e) - })? + .map_err(|_| ProcessDebugRequestError::TryLockError(file!(), line!()))? .layout .get_guest_code_address(); @@ -897,88 +1157,79 @@ mod debug { Ok(DebugResponse::ReadAddr(data)) } DebugMsg::ReadRegisters => { - let regs = self.vm.regs()?; - let fpu = self.vm.fpu()?; + let regs = self.vm.regs().map_err(VmError::Register)?; + let fpu = self.vm.fpu().map_err(VmError::Register)?; Ok(DebugResponse::ReadRegisters(Box::new((regs, fpu)))) } DebugMsg::RemoveHwBreakpoint(addr) => Ok(DebugResponse::RemoveHwBreakpoint( self.vm .remove_hw_breakpoint(addr) - .map_err(|e| { + .inspect_err(|e| { log::error!("Failed to remove hw breakpoint: {:?}", e); - - e }) .is_ok(), )), DebugMsg::RemoveSwBreakpoint(addr) => Ok(DebugResponse::RemoveSwBreakpoint( self.remove_sw_breakpoint(addr, mem_access) - .map_err(|e| { + .inspect_err(|e| { log::error!("Failed to remove sw breakpoint: {:?}", e); - - e }) .is_ok(), )), DebugMsg::Step => { - self.vm.set_single_step(true).map_err(|e| { + self.vm.set_single_step(true).inspect_err(|e| { log::error!("Failed to enable step instruction: {:?}", e); - - e })?; Ok(DebugResponse::Step) } DebugMsg::WriteAddr(addr, data) => { - self.write_addrs(addr, &data, mem_access).map_err(|e| { + self.write_addrs(addr, &data, mem_access).inspect_err(|e| { log::error!("Failed to write to address: {:?}", e); - - e })?; Ok(DebugResponse::WriteAddr) } DebugMsg::WriteRegisters(boxed_regs) => { let (regs, fpu) = boxed_regs.as_ref(); - self.vm.set_regs(regs)?; - self.vm.set_fpu(fpu)?; + self.vm.set_regs(regs).map_err(VmError::Register)?; + self.vm.set_fpu(fpu).map_err(VmError::Register)?; Ok(DebugResponse::WriteRegisters) } } } else { - Err(new_error!("Debugging is not enabled")) + Err(ProcessDebugRequestError::DebugNotEnabled) } } - pub(crate) fn recv_dbg_msg(&mut self) -> Result { + pub(crate) fn recv_dbg_msg( + &mut self, + ) -> std::result::Result { + use super::RecvDbgMsgError; + let gdb_conn = self .gdb_conn .as_mut() - .ok_or_else(|| new_error!("Debug is not enabled"))?; - - gdb_conn.recv().map_err(|e| { - new_error!( - "Got an error while waiting to receive a message from the gdb thread: {:?}", - e - ) - }) + .ok_or(RecvDbgMsgError::DebugNotEnabled)?; + + Ok(gdb_conn.recv()?) } - pub(crate) fn send_dbg_msg(&mut self, cmd: DebugResponse) -> Result<()> { + pub(crate) fn send_dbg_msg( + &mut self, + cmd: DebugResponse, + ) -> std::result::Result<(), super::SendDbgMsgError> { + use super::SendDbgMsgError; + log::debug!("Sending {:?}", cmd); let gdb_conn = self .gdb_conn .as_mut() - .ok_or_else(|| new_error!("Debug is not enabled"))?; - - gdb_conn.send(cmd).map_err(|e| { - new_error!( - "Got an error while sending a response message to the gdb thread: {:?}", - e - ) - }) + .ok_or(SendDbgMsgError::DebugNotEnabled)?; + + Ok(gdb_conn.send(cmd)?) } fn read_addrs( @@ -986,7 +1237,7 @@ mod debug { mut gva: u64, mut data: &mut [u8], mem_access: &DebugMemoryAccess, - ) -> crate::Result<()> { + ) -> std::result::Result<(), ProcessDebugRequestError> { let data_len = data.len(); log::debug!("Read addr: {:X} len: {:X}", gva, data_len); @@ -1014,7 +1265,7 @@ mod debug { mut gva: u64, mut data: &[u8], mem_access: &DebugMemoryAccess, - ) -> crate::Result<()> { + ) -> std::result::Result<(), ProcessDebugRequestError> { let data_len = data.len(); log::debug!("Write addr: {:X} len: {:X}", gva, data_len); @@ -1027,7 +1278,9 @@ mod debug { ); // Use the memory access to write to guest memory - mem_access.write(&data[..write_len], gpa)?; + mem_access + .write(&data[..write_len], gpa) + .map_err(ProcessDebugRequestError::WriteMemory)?; data = &data[write_len..]; gva += write_len as u64; @@ -1041,7 +1294,7 @@ mod debug { &mut self, gva: u64, mem_access: &DebugMemoryAccess, - ) -> crate::Result<()> { + ) -> std::result::Result<(), ProcessDebugRequestError> { // Check if breakpoint already exists if self.sw_breakpoints.contains_key(&gva) { return Ok(()); @@ -1062,14 +1315,14 @@ mod debug { &mut self, gva: u64, mem_access: &DebugMemoryAccess, - ) -> crate::Result<()> { + ) -> std::result::Result<(), ProcessDebugRequestError> { if let Some(saved_data) = self.sw_breakpoints.remove(&gva) { // Restore saved data to the guest's memory self.write_addrs(gva, &[saved_data], mem_access)?; Ok(()) } else { - Err(new_error!("The address: {:?} is not a sw breakpoint", gva)) + Err(ProcessDebugRequestError::SwBreakpointNotFound(gva)) } } } diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index 49db7e58f..e4efd8cfa 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -554,7 +554,8 @@ pub(crate) mod tests { guest_max_log_level, #[cfg(gdb)] dbg_mem_access_fn, - )?; + ) + .unwrap(); Ok(()) } diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm.rs index b5ca402fe..c83f4c75a 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm.rs @@ -24,11 +24,13 @@ use kvm_ioctls::{Kvm, VcpuExit, VcpuFd, VmFd}; use tracing::{Span, instrument}; #[cfg(gdb)] -use crate::hypervisor::gdb::DebuggableVm; +use crate::hypervisor::gdb::{DebugError, DebuggableVm}; use crate::hypervisor::regs::{CommonFpu, CommonRegisters, CommonSpecialRegisters}; -use crate::hypervisor::virtual_machine::{VirtualMachine, VmExit}; +use crate::hypervisor::virtual_machine::{ + CreateVmError, MapMemoryError, RegisterError, RunVcpuError, UnmapMemoryError, VirtualMachine, + VmExit, +}; use crate::mem::memory_region::MemoryRegion; -use crate::{Result, new_error}; /// Return `true` if the KVM API is available, version 12, and has UserMemory capability, or `false` otherwise #[instrument(skip_all, parent = Span::current(), level = "Trace")] @@ -63,18 +65,21 @@ pub(crate) struct KvmVm { debug_regs: kvm_guest_debug, } -static KVM: LazyLock> = - LazyLock::new(|| Kvm::new().map_err(|e| new_error!("Failed to open /dev/kvm: {}", e))); +static KVM: LazyLock> = + LazyLock::new(|| Kvm::new().map_err(|e| CreateVmError::HypervisorNotAvailable(e.into()))); impl KvmVm { /// Create a new instance of a `KvmVm` #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] - pub(crate) fn new() -> Result { - let hv = KVM - .as_ref() - .map_err(|e| new_error!("Failed to create KVM instance: {}", e))?; - let vm_fd = hv.create_vm_with_type(0)?; - let vcpu_fd = vm_fd.create_vcpu(0)?; + pub(crate) fn new() -> std::result::Result { + let hv = KVM.as_ref().map_err(|e| e.clone())?; + + let vm_fd = hv + .create_vm_with_type(0) + .map_err(|e| CreateVmError::CreateVmFd(e.into()))?; + let vcpu_fd = vm_fd + .create_vcpu(0) + .map_err(|e| CreateVmError::CreateVcpuFd(e.into()))?; Ok(Self { vm_fd, @@ -86,25 +91,31 @@ impl KvmVm { } impl VirtualMachine for KvmVm { - unsafe fn map_memory(&mut self, (slot, region): (u32, &MemoryRegion)) -> Result<()> { + unsafe fn map_memory( + &mut self, + (slot, region): (u32, &MemoryRegion), + ) -> std::result::Result<(), MapMemoryError> { let mut kvm_region: kvm_userspace_memory_region = region.into(); kvm_region.slot = slot; - unsafe { self.vm_fd.set_user_memory_region(kvm_region)? }; - Ok(()) + unsafe { self.vm_fd.set_user_memory_region(kvm_region) } + .map_err(|e| MapMemoryError::Hypervisor(e.into())) } - fn unmap_memory(&mut self, (slot, region): (u32, &MemoryRegion)) -> Result<()> { + fn unmap_memory( + &mut self, + (slot, region): (u32, &MemoryRegion), + ) -> std::result::Result<(), UnmapMemoryError> { let mut kvm_region: kvm_userspace_memory_region = region.into(); kvm_region.slot = slot; // Setting memory_size to 0 unmaps the slot's region // From https://docs.kernel.org/virt/kvm/api.html // > Deleting a slot is done by passing zero for memory_size. kvm_region.memory_size = 0; - unsafe { self.vm_fd.set_user_memory_region(kvm_region) }?; - Ok(()) + unsafe { self.vm_fd.set_user_memory_region(kvm_region) } + .map_err(|e| UnmapMemoryError::Hypervisor(e.into())) } - fn run_vcpu(&mut self) -> Result { + fn run_vcpu(&mut self) -> std::result::Result { match self.vcpu_fd.run() { Ok(VcpuExit::Hlt) => Ok(VmExit::Halt()), Ok(VcpuExit::IoOut(port, data)) => Ok(VmExit::IoOut(port, data.to_vec())), @@ -119,7 +130,7 @@ impl VirtualMachine for KvmVm { // InterruptHandle::kill() sends a signal (SIGRTMIN+offset) to interrupt the vcpu, which causes EINTR libc::EINTR => Ok(VmExit::Cancelled()), libc::EAGAIN => Ok(VmExit::Retry()), - _ => Ok(VmExit::Unknown(format!("Unknown KVM VCPU error: {}", e))), + _ => Err(RunVcpuError::Unknown(e.into())), }, Ok(other) => Ok(VmExit::Unknown(format!( "Unknown KVM VCPU exit: {:?}", @@ -128,42 +139,60 @@ impl VirtualMachine for KvmVm { } } - fn regs(&self) -> Result { - let kvm_regs = self.vcpu_fd.get_regs()?; + fn regs(&self) -> std::result::Result { + let kvm_regs = self + .vcpu_fd + .get_regs() + .map_err(|e| RegisterError::GetRegs(e.into()))?; Ok((&kvm_regs).into()) } - fn set_regs(&self, regs: &CommonRegisters) -> Result<()> { + fn set_regs(&self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { let kvm_regs: kvm_regs = regs.into(); - self.vcpu_fd.set_regs(&kvm_regs)?; + self.vcpu_fd + .set_regs(&kvm_regs) + .map_err(|e| RegisterError::SetRegs(e.into()))?; Ok(()) } - fn fpu(&self) -> Result { - let kvm_fpu = self.vcpu_fd.get_fpu()?; + fn fpu(&self) -> std::result::Result { + let kvm_fpu = self + .vcpu_fd + .get_fpu() + .map_err(|e| RegisterError::GetFpu(e.into()))?; Ok((&kvm_fpu).into()) } - fn set_fpu(&self, fpu: &CommonFpu) -> Result<()> { + fn set_fpu(&self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError> { let kvm_fpu: kvm_fpu = fpu.into(); - self.vcpu_fd.set_fpu(&kvm_fpu)?; + self.vcpu_fd + .set_fpu(&kvm_fpu) + .map_err(|e| RegisterError::SetFpu(e.into()))?; Ok(()) } - fn sregs(&self) -> Result { - let kvm_sregs = self.vcpu_fd.get_sregs()?; + fn sregs(&self) -> std::result::Result { + let kvm_sregs = self + .vcpu_fd + .get_sregs() + .map_err(|e| RegisterError::GetSregs(e.into()))?; Ok((&kvm_sregs).into()) } - fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> Result<()> { + fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> std::result::Result<(), RegisterError> { let kvm_sregs: kvm_sregs = sregs.into(); - self.vcpu_fd.set_sregs(&kvm_sregs)?; + self.vcpu_fd + .set_sregs(&kvm_sregs) + .map_err(|e| RegisterError::SetSregs(e.into()))?; Ok(()) } #[cfg(crashdump)] - fn xsave(&self) -> Result> { - let xsave = self.vcpu_fd.get_xsave()?; + fn xsave(&self) -> std::result::Result, RegisterError> { + let xsave = self + .vcpu_fd + .get_xsave() + .map_err(|e| RegisterError::GetXsave(e.into()))?; Ok(xsave .region .into_iter() @@ -174,18 +203,19 @@ impl VirtualMachine for KvmVm { #[cfg(gdb)] impl DebuggableVm for KvmVm { - fn translate_gva(&self, gva: u64) -> Result { - use crate::HyperlightError; - - let gpa = self.vcpu_fd.translate_gva(gva)?; + fn translate_gva(&self, gva: u64) -> std::result::Result { + let gpa = self + .vcpu_fd + .translate_gva(gva) + .map_err(|_| DebugError::TranslateGva(gva))?; if gpa.valid == 0 { - Err(HyperlightError::TranslateGuestAddress(gva)) + Err(DebugError::TranslateGva(gva)) } else { Ok(gpa.physical_address) } } - fn set_debug(&mut self, enable: bool) -> Result<()> { + fn set_debug(&mut self, enable: bool) -> std::result::Result<(), DebugError> { use kvm_bindings::{KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_USE_HW_BP, KVM_GUESTDBG_USE_SW_BP}; log::info!("Setting debug to {}", enable); @@ -196,11 +226,13 @@ impl DebuggableVm for KvmVm { self.debug_regs.control &= !(KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP | KVM_GUESTDBG_USE_SW_BP); } - self.vcpu_fd.set_guest_debug(&self.debug_regs)?; + self.vcpu_fd + .set_guest_debug(&self.debug_regs) + .map_err(|e| RegisterError::SetDebugRegs(e.into()))?; Ok(()) } - fn set_single_step(&mut self, enable: bool) -> Result<()> { + fn set_single_step(&mut self, enable: bool) -> std::result::Result<(), DebugError> { use kvm_bindings::KVM_GUESTDBG_SINGLESTEP; log::info!("Setting single step to {}", enable); @@ -209,7 +241,9 @@ impl DebuggableVm for KvmVm { } else { self.debug_regs.control &= !KVM_GUESTDBG_SINGLESTEP; } - self.vcpu_fd.set_guest_debug(&self.debug_regs)?; + self.vcpu_fd + .set_guest_debug(&self.debug_regs) + .map_err(|e| RegisterError::SetDebugRegs(e.into()))?; // Set TF Flag to enable Traps let mut regs = self.regs()?; @@ -222,7 +256,7 @@ impl DebuggableVm for KvmVm { Ok(()) } - fn add_hw_breakpoint(&mut self, addr: u64) -> Result<()> { + fn add_hw_breakpoint(&mut self, addr: u64) -> std::result::Result<(), DebugError> { use crate::hypervisor::gdb::arch::MAX_NO_OF_HW_BP; // Check if breakpoint already exists @@ -233,7 +267,7 @@ impl DebuggableVm for KvmVm { // Find the first available LOCAL (L0–L3) slot let i = (0..MAX_NO_OF_HW_BP) .position(|i| self.debug_regs.arch.debugreg[7] & (1 << (i * 2)) == 0) - .ok_or_else(|| new_error!("Tried to add more than 4 hardware breakpoints"))?; + .ok_or(DebugError::TooManyHwBreakpoints(MAX_NO_OF_HW_BP))?; // Assign to corresponding debug register self.debug_regs.arch.debugreg[i] = addr; @@ -241,16 +275,18 @@ impl DebuggableVm for KvmVm { // Enable LOCAL bit self.debug_regs.arch.debugreg[7] |= 1 << (i * 2); - self.vcpu_fd.set_guest_debug(&self.debug_regs)?; + self.vcpu_fd + .set_guest_debug(&self.debug_regs) + .map_err(|e| RegisterError::SetDebugRegs(e.into()))?; Ok(()) } - fn remove_hw_breakpoint(&mut self, addr: u64) -> Result<()> { + fn remove_hw_breakpoint(&mut self, addr: u64) -> std::result::Result<(), DebugError> { // Find the index of the breakpoint let index = self.debug_regs.arch.debugreg[..4] .iter() .position(|&a| a == addr) - .ok_or_else(|| new_error!("Tried to remove non-existing hw-breakpoint"))?; + .ok_or(DebugError::HwBreakpointNotFound(addr))?; // Clear the address self.debug_regs.arch.debugreg[index] = 0; @@ -258,7 +294,9 @@ impl DebuggableVm for KvmVm { // Disable LOCAL bit self.debug_regs.arch.debugreg[7] &= !(1 << (index * 2)); - self.vcpu_fd.set_guest_debug(&self.debug_regs)?; + self.vcpu_fd + .set_guest_debug(&self.debug_regs) + .map_err(|e| RegisterError::SetDebugRegs(e.into()))?; Ok(()) } } diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs index 55af78ebb..2c40b3095 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs @@ -19,7 +19,8 @@ use std::sync::OnceLock; use tracing::{Span, instrument}; -use crate::Result; +#[cfg(gdb)] +use crate::hypervisor::gdb::DebugError; use crate::hypervisor::regs::{CommonFpu, CommonRegisters, CommonSpecialRegisters}; use crate::mem::memory_region::MemoryRegion; @@ -129,6 +130,131 @@ pub(crate) enum VmExit { Retry(), } +/// VM error +#[derive(Debug, Clone, thiserror::Error, displaydoc::Display)] +pub enum VmError { + /// Failed to create vm: {0} + CreateVm(#[from] CreateVmError), + /// Failed to run vcpu: {0} + RunVcpu(#[from] RunVcpuError), + /// Register operation failed: {0} + Register(#[from] RegisterError), + /// Map memory operation failed: {0} + MapMemory(#[from] MapMemoryError), + /// Unmap memory operation failed: {0} + UnmapMemory(#[from] UnmapMemoryError), + /// Debug operation failed: {0} + #[cfg(gdb)] + Debug(#[from] DebugError), +} + +/// Create VM error +#[derive(Debug, Clone, thiserror::Error, displaydoc::Display)] +pub enum CreateVmError { + /// Hypervisor is not available: {0} + HypervisorNotAvailable(HypervisorImplError), + /// VM creation failed: {0} + CreateVmFd(HypervisorImplError), + /// VCPU creation failed: {0} + CreateVcpuFd(HypervisorImplError), + /// Set Partition Property failed: {0} + SetPartitionProperty(HypervisorImplError), + /// Initialize VM failed: {0} + InitializeVm(HypervisorImplError), + /// Surrogate process creation failed: {0} + #[cfg(target_os = "windows")] + SurrogateProcess(String), +} + +/// RunVCPU error +#[derive(Debug, Clone, thiserror::Error, displaydoc::Display)] +pub enum RunVcpuError { + /// Failed to decode message type: {0} + DecodeIOMessage(u32), + /// Failed to get DR6 debug register: {0} + #[cfg(gdb)] + GetDr6(HypervisorImplError), + /// Increment RIP failed: {0} + IncrementRip(HypervisorImplError), + /// Parse GPA access info failed + ParseGpaAccessInfo, + /// Unknown error: {0} + Unknown(HypervisorImplError), +} + +/// Register error +#[derive(Debug, Clone, thiserror::Error, displaydoc::Display)] +pub enum RegisterError { + /// Failed to get registers: {0} + GetRegs(HypervisorImplError), + /// Failed to set registers: {0} + SetRegs(HypervisorImplError), + /// Failed to get FPU registers: {0} + GetFpu(HypervisorImplError), + /// Failed to set FPU registers: {0} + SetFpu(HypervisorImplError), + /// Failed to get special registers: {0} + GetSregs(HypervisorImplError), + /// Failed to set special registers: {0} + SetSregs(HypervisorImplError), + /// Failed to get debug registers: {0} + GetDebugRegs(HypervisorImplError), + /// Failed to set debug registers: {0} + SetDebugRegs(HypervisorImplError), + /// Failed to get xsave: {0} + GetXsave(HypervisorImplError), + /// Xsave size mismatch: expected {expected} bytes, got {actual} + XsaveSizeMismatch { + /// Expected size in bytes + expected: u32, + /// Actual size in bytes + actual: u32, + }, +} + +/// Map memory error +#[derive(Debug, Clone, thiserror::Error, displaydoc::Display)] +pub enum MapMemoryError { + /// Hypervisor error: {0} + Hypervisor(HypervisorImplError), + /// Address conversion failed: {0} + #[cfg(target_os = "windows")] + AddressConversion(std::num::TryFromIntError), + /// Failed to load API: {0} + #[cfg(target_os = "windows")] + LoadApi(windows_result::Error), + /// Invalid memory region flags: {0} + #[cfg(target_os = "windows")] + InvalidFlags(String), + /// Operation not supported: {0} + #[cfg(target_os = "windows")] + NotSupported(String), +} + +/// Unmap memory error +#[derive(Debug, Clone, thiserror::Error, displaydoc::Display)] +pub enum UnmapMemoryError { + /// Hypervisor error: {0} + Hypervisor(HypervisorImplError), + /// Operation not supported: {0} + #[cfg(target_os = "windows")] + NotSupported(String), +} + +/// Implementation-specific Hypervisor error +#[derive(Debug, Clone, thiserror::Error, displaydoc::Display)] +pub enum HypervisorImplError { + /// KVM error: {0} + #[cfg(kvm)] + KvmError(#[from] kvm_ioctls::Error), + /// MSHV error: {0} + #[cfg(mshv3)] + MshvError(#[from] mshv_ioctls::MshvError), + /// Windows error: {0} + #[cfg(target_os = "windows")] + WindowsError(#[from] windows_result::Error), +} + /// Trait for single-vCPU VMs. Provides a common interface for basic VM operations. /// Abstracts over differences between KVM, MSHV and WHP implementations. pub(crate) trait VirtualMachine: Debug + Send { @@ -140,34 +266,40 @@ pub(crate) trait VirtualMachine: Debug + Send { /// The caller must ensure that the given u32 is not already mapped, otherwise previously mapped /// memory regions may be overwritten. /// The memory region must not overlap with an existing region, and depending on platform, must be aligned to page boundaries. - unsafe fn map_memory(&mut self, region: (u32, &MemoryRegion)) -> Result<()>; + unsafe fn map_memory( + &mut self, + region: (u32, &MemoryRegion), + ) -> std::result::Result<(), MapMemoryError>; /// Unmap memory region from this VM that has previously been mapped using `map_memory`. - fn unmap_memory(&mut self, region: (u32, &MemoryRegion)) -> Result<()>; + fn unmap_memory( + &mut self, + region: (u32, &MemoryRegion), + ) -> std::result::Result<(), UnmapMemoryError>; /// Runs the vCPU until it exits. /// Note: this function should not emit any traces or spans as it is called after guest span is setup - fn run_vcpu(&mut self) -> Result; + fn run_vcpu(&mut self) -> std::result::Result; /// Get regs #[allow(dead_code)] - fn regs(&self) -> Result; + fn regs(&self) -> std::result::Result; /// Set regs - fn set_regs(&self, regs: &CommonRegisters) -> Result<()>; + fn set_regs(&self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError>; /// Get fpu regs #[allow(dead_code)] - fn fpu(&self) -> Result; + fn fpu(&self) -> std::result::Result; /// Set fpu regs - fn set_fpu(&self, fpu: &CommonFpu) -> Result<()>; + fn set_fpu(&self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError>; /// Get special regs #[allow(dead_code)] - fn sregs(&self) -> Result; + fn sregs(&self) -> std::result::Result; /// Set special regs - fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> Result<()>; + fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> std::result::Result<(), RegisterError>; /// xsave #[cfg(crashdump)] - fn xsave(&self) -> Result>; + fn xsave(&self) -> std::result::Result, RegisterError>; /// Get partition handle #[cfg(target_os = "windows")] diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/mshv.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/mshv.rs index c0e352604..276ac9e8a 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/mshv.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/mshv.rs @@ -32,11 +32,13 @@ use mshv_ioctls::{Mshv, VcpuFd, VmFd}; use tracing::{Span, instrument}; #[cfg(gdb)] -use crate::hypervisor::gdb::DebuggableVm; +use crate::hypervisor::gdb::{DebugError, DebuggableVm}; use crate::hypervisor::regs::{CommonFpu, CommonRegisters, CommonSpecialRegisters}; -use crate::hypervisor::virtual_machine::{VirtualMachine, VmExit}; +use crate::hypervisor::virtual_machine::{ + CreateVmError, MapMemoryError, RegisterError, RunVcpuError, UnmapMemoryError, VirtualMachine, + VmExit, +}; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; -use crate::{Result, new_error}; /// Determine whether the HyperV for Linux hypervisor API is present /// and functional. @@ -58,52 +60,66 @@ pub(crate) struct MshvVm { vcpu_fd: VcpuFd, } -static MSHV: LazyLock> = - LazyLock::new(|| Mshv::new().map_err(|e| new_error!("Failed to open /dev/mshv: {}", e))); +static MSHV: LazyLock> = + LazyLock::new(|| Mshv::new().map_err(|e| CreateVmError::HypervisorNotAvailable(e.into()))); impl MshvVm { /// Create a new instance of a MshvVm #[instrument(skip_all, parent = Span::current(), level = "Trace")] - pub(crate) fn new() -> Result { - let mshv = MSHV - .as_ref() - .map_err(|e| new_error!("Failed to create MSHV instance: {}", e))?; - let pr = Default::default(); - let vm_fd = { - // It's important to avoid create_vm() and explicitly use - // create_vm_with_args() with an empty arguments structure - // here, because otherwise the partition is set up with a SynIC. + pub(crate) fn new() -> std::result::Result { + let mshv = MSHV.as_ref().map_err(|e| e.clone())?; - let vm_fd = mshv.create_vm_with_args(&pr)?; + let pr = Default::default(); + // It's important to avoid create_vm() and explicitly use + // create_vm_with_args() with an empty arguments structure + // here, because otherwise the partition is set up with a SynIC. + let vm_fd = mshv + .create_vm_with_args(&pr) + .map_err(|e| CreateVmError::CreateVmFd(e.into()))?; + + let vcpu_fd = { let features: hv_partition_synthetic_processor_features = Default::default(); - vm_fd.set_partition_property( - hv_partition_property_code_HV_PARTITION_PROPERTY_SYNTHETIC_PROC_FEATURES, - unsafe { features.as_uint64[0] }, - )?; - vm_fd.initialize()?; vm_fd - }; + .set_partition_property( + hv_partition_property_code_HV_PARTITION_PROPERTY_SYNTHETIC_PROC_FEATURES, + unsafe { features.as_uint64[0] }, + ) + .map_err(|e| CreateVmError::SetPartitionProperty(e.into()))?; + vm_fd + .initialize() + .map_err(|e| CreateVmError::InitializeVm(e.into()))?; - let vcpu_fd = vm_fd.create_vcpu(0)?; + vm_fd + .create_vcpu(0) + .map_err(|e| CreateVmError::CreateVcpuFd(e.into()))? + }; Ok(Self { vm_fd, vcpu_fd }) } } impl VirtualMachine for MshvVm { - unsafe fn map_memory(&mut self, (_slot, region): (u32, &MemoryRegion)) -> Result<()> { + unsafe fn map_memory( + &mut self, + (_slot, region): (u32, &MemoryRegion), + ) -> std::result::Result<(), MapMemoryError> { let mshv_region: mshv_user_mem_region = region.into(); - self.vm_fd.map_user_memory(mshv_region)?; - Ok(()) + self.vm_fd + .map_user_memory(mshv_region) + .map_err(|e| MapMemoryError::Hypervisor(e.into())) } - fn unmap_memory(&mut self, (_slot, region): (u32, &MemoryRegion)) -> Result<()> { + fn unmap_memory( + &mut self, + (_slot, region): (u32, &MemoryRegion), + ) -> std::result::Result<(), UnmapMemoryError> { let mshv_region: mshv_user_mem_region = region.into(); - self.vm_fd.unmap_user_memory(mshv_region)?; - Ok(()) + self.vm_fd + .unmap_user_memory(mshv_region) + .map_err(|e| UnmapMemoryError::Hypervisor(e.into())) } - fn run_vcpu(&mut self) -> Result { + fn run_vcpu(&mut self) -> std::result::Result { const HALT_MESSAGE: hv_message_type = hv_message_type_HVMSG_X64_HALT; const IO_PORT_INTERCEPT_MESSAGE: hv_message_type = hv_message_type_HVMSG_X64_IO_PORT_INTERCEPT; @@ -118,35 +134,46 @@ impl VirtualMachine for MshvVm { Ok(m) => match m.header.message_type { HALT_MESSAGE => VmExit::Halt(), IO_PORT_INTERCEPT_MESSAGE => { - let io_message = m.to_ioport_info().map_err(mshv_ioctls::MshvError::from)?; + let io_message = m + .to_ioport_info() + .map_err(|_| RunVcpuError::DecodeIOMessage(m.header.message_type))?; let port_number = io_message.port_number; let rip = io_message.header.rip; let rax = io_message.rax; let instruction_length = io_message.header.instruction_length() as u64; // mshv, unlike kvm, does not automatically increment RIP - self.vcpu_fd.set_reg(&[hv_register_assoc { - name: hv_register_name_HV_X64_REGISTER_RIP, - value: hv_register_value { - reg64: rip + instruction_length, - }, - ..Default::default() - }])?; + self.vcpu_fd + .set_reg(&[hv_register_assoc { + name: hv_register_name_HV_X64_REGISTER_RIP, + value: hv_register_value { + reg64: rip + instruction_length, + }, + ..Default::default() + }]) + .map_err(|e| RunVcpuError::IncrementRip(e.into()))?; VmExit::IoOut(port_number, rax.to_le_bytes().to_vec()) } UNMAPPED_GPA_MESSAGE => { - let mimo_message = m.to_memory_info().map_err(mshv_ioctls::MshvError::from)?; + let mimo_message = m + .to_memory_info() + .map_err(|_| RunVcpuError::DecodeIOMessage(m.header.message_type))?; let addr = mimo_message.guest_physical_address; - match MemoryRegionFlags::try_from(mimo_message)? { + match MemoryRegionFlags::try_from(mimo_message) + .map_err(|_| RunVcpuError::ParseGpaAccessInfo)? + { MemoryRegionFlags::READ => VmExit::MmioRead(addr), MemoryRegionFlags::WRITE => VmExit::MmioWrite(addr), _ => VmExit::Unknown("Unknown MMIO access".to_string()), } } INVALID_GPA_ACCESS_MESSAGE => { - let mimo_message = m.to_memory_info().map_err(mshv_ioctls::MshvError::from)?; + let mimo_message = m + .to_memory_info() + .map_err(|_| RunVcpuError::DecodeIOMessage(m.header.message_type))?; let gpa = mimo_message.guest_physical_address; - let access_info = MemoryRegionFlags::try_from(mimo_message)?; + let access_info = MemoryRegionFlags::try_from(mimo_message) + .map_err(|_| RunVcpuError::ParseGpaAccessInfo)?; match access_info { MemoryRegionFlags::READ => VmExit::MmioRead(gpa), MemoryRegionFlags::WRITE => VmExit::MmioWrite(gpa), @@ -157,8 +184,11 @@ impl VirtualMachine for MshvVm { EXCEPTION_INTERCEPT => { let ex_info = m .to_exception_info() - .map_err(mshv_ioctls::MshvError::from)?; - let DebugRegisters { dr6, .. } = self.vcpu_fd.get_debug_regs()?; + .map_err(|_| RunVcpuError::DecodeIOMessage(m.header.message_type))?; + let DebugRegisters { dr6, .. } = self + .vcpu_fd + .get_debug_regs() + .map_err(|e| RunVcpuError::GetDr6(e.into()))?; VmExit::Debug { dr6, exception: ex_info.exception_vector as u32, @@ -170,69 +200,85 @@ impl VirtualMachine for MshvVm { // InterruptHandle::kill() sends a signal (SIGRTMIN+offset) to interrupt the vcpu, which causes EINTR libc::EINTR => VmExit::Cancelled(), libc::EAGAIN => VmExit::Retry(), - _ => VmExit::Unknown(format!("Unknown MSHV VCPU error: {}", e)), + _ => Err(RunVcpuError::Unknown(e.into()))?, }, }; Ok(result) } - fn regs(&self) -> Result { - let mshv_regs = self.vcpu_fd.get_regs()?; + fn regs(&self) -> std::result::Result { + let mshv_regs = self + .vcpu_fd + .get_regs() + .map_err(|e| RegisterError::GetRegs(e.into()))?; Ok((&mshv_regs).into()) } - fn set_regs(&self, regs: &CommonRegisters) -> Result<()> { + fn set_regs(&self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { let mshv_regs: StandardRegisters = regs.into(); - self.vcpu_fd.set_regs(&mshv_regs)?; + self.vcpu_fd + .set_regs(&mshv_regs) + .map_err(|e| RegisterError::SetRegs(e.into()))?; Ok(()) } - fn fpu(&self) -> Result { - let mshv_fpu = self.vcpu_fd.get_fpu()?; + fn fpu(&self) -> std::result::Result { + let mshv_fpu = self + .vcpu_fd + .get_fpu() + .map_err(|e| RegisterError::GetFpu(e.into()))?; Ok((&mshv_fpu).into()) } - fn set_fpu(&self, fpu: &CommonFpu) -> Result<()> { + fn set_fpu(&self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError> { let mshv_fpu: FloatingPointUnit = fpu.into(); - self.vcpu_fd.set_fpu(&mshv_fpu)?; + self.vcpu_fd + .set_fpu(&mshv_fpu) + .map_err(|e| RegisterError::SetFpu(e.into()))?; Ok(()) } - fn sregs(&self) -> Result { - let mshv_sregs = self.vcpu_fd.get_sregs()?; + fn sregs(&self) -> std::result::Result { + let mshv_sregs = self + .vcpu_fd + .get_sregs() + .map_err(|e| RegisterError::GetSregs(e.into()))?; Ok((&mshv_sregs).into()) } - fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> Result<()> { + fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> std::result::Result<(), RegisterError> { let mshv_sregs: SpecialRegisters = sregs.into(); - self.vcpu_fd.set_sregs(&mshv_sregs)?; + self.vcpu_fd + .set_sregs(&mshv_sregs) + .map_err(|e| RegisterError::SetSregs(e.into()))?; Ok(()) } #[cfg(crashdump)] - fn xsave(&self) -> Result> { - let xsave = self.vcpu_fd.get_xsave()?; + fn xsave(&self) -> std::result::Result, RegisterError> { + let xsave = self + .vcpu_fd + .get_xsave() + .map_err(|e| RegisterError::GetXsave(e.into()))?; Ok(xsave.buffer.to_vec()) } } #[cfg(gdb)] impl DebuggableVm for MshvVm { - fn translate_gva(&self, gva: u64) -> Result { + fn translate_gva(&self, gva: u64) -> std::result::Result { use mshv_bindings::{HV_TRANSLATE_GVA_VALIDATE_READ, HV_TRANSLATE_GVA_VALIDATE_WRITE}; - use crate::HyperlightError; - let flags = (HV_TRANSLATE_GVA_VALIDATE_READ | HV_TRANSLATE_GVA_VALIDATE_WRITE) as u64; let (addr, _) = self .vcpu_fd .translate_gva(gva, flags) - .map_err(|_| HyperlightError::TranslateGuestAddress(gva))?; + .map_err(|_| DebugError::TranslateGva(gva))?; Ok(addr) } - fn set_debug(&mut self, enabled: bool) -> Result<()> { + fn set_debug(&mut self, enabled: bool) -> std::result::Result<(), DebugError> { use mshv_bindings::{ HV_INTERCEPT_ACCESS_MASK_EXECUTE, HV_INTERCEPT_ACCESS_MASK_NONE, hv_intercept_parameters, hv_intercept_type_HV_INTERCEPT_TYPE_EXCEPTION, @@ -256,19 +302,15 @@ impl DebuggableVm for MshvVm { exception_vector: vector as u16, }, }) - .map_err(|e| { - new_error!( - "Cannot {} exception intercept for vector {}: {}", - if enabled { "install" } else { "remove" }, - vector, - e - ) + .map_err(|e| DebugError::Intercept { + enable: enabled, + inner: e.into(), })?; } Ok(()) } - fn set_single_step(&mut self, enable: bool) -> Result<()> { + fn set_single_step(&mut self, enable: bool) -> std::result::Result<(), DebugError> { let mut regs = self.regs()?; if enable { regs.rflags |= 1 << 8; @@ -279,10 +321,13 @@ impl DebuggableVm for MshvVm { Ok(()) } - fn add_hw_breakpoint(&mut self, addr: u64) -> Result<()> { + fn add_hw_breakpoint(&mut self, addr: u64) -> std::result::Result<(), DebugError> { use crate::hypervisor::gdb::arch::MAX_NO_OF_HW_BP; - let mut debug_regs = self.vcpu_fd.get_debug_regs()?; + let mut debug_regs = self + .vcpu_fd + .get_debug_regs() + .map_err(|e| RegisterError::GetDebugRegs(e.into()))?; // Check if breakpoint already exists if [ @@ -299,7 +344,7 @@ impl DebuggableVm for MshvVm { // Find the first available LOCAL (L0–L3) slot let i = (0..MAX_NO_OF_HW_BP) .position(|i| debug_regs.dr7 & (1 << (i * 2)) == 0) - .ok_or_else(|| new_error!("Tried to add more than 4 hardware breakpoints"))?; + .ok_or(DebugError::TooManyHwBreakpoints(MAX_NO_OF_HW_BP))?; // Assign to corresponding debug register *[ @@ -312,12 +357,17 @@ impl DebuggableVm for MshvVm { // Enable LOCAL bit debug_regs.dr7 |= 1 << (i * 2); - self.vcpu_fd.set_debug_regs(&debug_regs)?; + self.vcpu_fd + .set_debug_regs(&debug_regs) + .map_err(|e| RegisterError::SetDebugRegs(e.into()))?; Ok(()) } - fn remove_hw_breakpoint(&mut self, addr: u64) -> Result<()> { - let mut debug_regs = self.vcpu_fd.get_debug_regs()?; + fn remove_hw_breakpoint(&mut self, addr: u64) -> std::result::Result<(), DebugError> { + let mut debug_regs = self + .vcpu_fd + .get_debug_regs() + .map_err(|e| RegisterError::GetDebugRegs(e.into()))?; let regs = [ &mut debug_regs.dr0, @@ -331,10 +381,12 @@ impl DebuggableVm for MshvVm { *regs[i] = 0; // Disable LOCAL bit debug_regs.dr7 &= !(1 << (i * 2)); - self.vcpu_fd.set_debug_regs(&debug_regs)?; + self.vcpu_fd + .set_debug_regs(&debug_regs) + .map_err(|e| RegisterError::SetDebugRegs(e.into()))?; Ok(()) } else { - Err(new_error!("Tried to remove non-existing hw-breakpoint")) + Err(DebugError::HwBreakpointNotFound(addr)) } } } diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs index 7effa3e9f..d5db13aea 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs @@ -24,17 +24,19 @@ use windows::core::s; use windows_result::HRESULT; #[cfg(gdb)] -use crate::hypervisor::gdb::DebuggableVm; +use crate::hypervisor::gdb::{DebugError, DebuggableVm}; use crate::hypervisor::regs::{ Align16, CommonFpu, CommonRegisters, CommonSpecialRegisters, WHP_FPU_NAMES, WHP_FPU_NAMES_LEN, WHP_REGS_NAMES, WHP_REGS_NAMES_LEN, WHP_SREGS_NAMES, WHP_SREGS_NAMES_LEN, }; use crate::hypervisor::surrogate_process::SurrogateProcess; use crate::hypervisor::surrogate_process_manager::get_surrogate_process_manager; -use crate::hypervisor::virtual_machine::{VirtualMachine, VmExit}; +use crate::hypervisor::virtual_machine::{ + CreateVmError, HypervisorImplError, MapMemoryError, RegisterError, RunVcpuError, + UnmapMemoryError, VirtualMachine, VmExit, +}; use crate::hypervisor::wrappers::HandleWrapper; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; -use crate::{Result, log_then_return, new_error}; #[allow(dead_code)] // Will be used for runtime hypervisor detection pub(crate) fn is_hypervisor_present() -> bool { @@ -80,24 +82,33 @@ pub(crate) struct WhpVm { unsafe impl Send for WhpVm {} impl WhpVm { - pub(crate) fn new(mmap_file_handle: HandleWrapper, raw_size: usize) -> Result { + pub(crate) fn new( + mmap_file_handle: HandleWrapper, + raw_size: usize, + ) -> std::result::Result { const NUM_CPU: u32 = 1; let partition = unsafe { - let partition = WHvCreatePartition()?; + let partition = + WHvCreatePartition().map_err(|e| CreateVmError::CreateVmFd(e.into()))?; WHvSetPartitionProperty( partition, WHvPartitionPropertyCodeProcessorCount, &NUM_CPU as *const _ as *const _, std::mem::size_of_val(&NUM_CPU) as _, - )?; - WHvSetupPartition(partition)?; - WHvCreateVirtualProcessor(partition, 0, 0)?; + ) + .map_err(|e| CreateVmError::SetPartitionProperty(e.into()))?; + WHvSetupPartition(partition).map_err(|e| CreateVmError::InitializeVm(e.into()))?; + WHvCreateVirtualProcessor(partition, 0, 0) + .map_err(|e| CreateVmError::CreateVcpuFd(e.into()))?; partition }; // Create the surrogate process with the total memory size - let mgr = get_surrogate_process_manager()?; - let surrogate_process = mgr.get_surrogate_process(raw_size, mmap_file_handle)?; + let mgr = get_surrogate_process_manager() + .map_err(|e| CreateVmError::SurrogateProcess(e.to_string()))?; + let surrogate_process = mgr + .get_surrogate_process(raw_size, mmap_file_handle) + .map_err(|e| CreateVmError::SurrogateProcess(e.to_string()))?; Ok(WhpVm { partition, @@ -112,7 +123,7 @@ impl WhpVm { fn set_registers( &self, registers: &[(WHV_REGISTER_NAME, Align16)], - ) -> Result<()> { + ) -> windows_result::Result<()> { let (names, values): (Vec<_>, Vec<_>) = registers.iter().copied().unzip(); unsafe { @@ -122,23 +133,24 @@ impl WhpVm { names.as_ptr(), names.len() as u32, values.as_ptr() as *const WHV_REGISTER_VALUE, // Casting Align16 away - )?; + ) } - - Ok(()) } } impl VirtualMachine for WhpVm { - unsafe fn map_memory(&mut self, (_slot, region): (u32, &MemoryRegion)) -> Result<()> { + unsafe fn map_memory( + &mut self, + (_slot, region): (u32, &MemoryRegion), + ) -> std::result::Result<(), MapMemoryError> { // Only allow memory mapping during initial setup (the first batch of regions). // After the initial setup is complete, subsequent calls should fail, // since it's not yet implemented. if self.initial_memory_setup_done { // Initial setup already completed - reject this mapping - log_then_return!( - "Mapping host memory into the guest not yet supported on this platform" - ); + return Err(MapMemoryError::NotSupported( + "Mapping host memory into the guest not yet supported on this platform".to_string(), + )); } // Calculate the offset on first call. The offset accounts for the guard page @@ -151,7 +163,11 @@ impl VirtualMachine for WhpVm { let surrogate_address = self.surrogate_process.allocated_address as usize + PAGE_SIZE_USIZE; let host_address = region.host_region.start; - let offset = isize::try_from(surrogate_address)? - isize::try_from(host_address)?; + let surrogate_isize = + isize::try_from(surrogate_address).map_err(MapMemoryError::AddressConversion)?; + let host_isize = + isize::try_from(host_address).map_err(MapMemoryError::AddressConversion)?; + let offset = surrogate_isize - host_isize; self.surrogate_offset = Some(offset); offset }; @@ -161,7 +177,7 @@ impl VirtualMachine for WhpVm { let whvmapgparange2_func = unsafe { match try_load_whv_map_gpa_range2() { Ok(func) => func, - Err(e) => return Err(new_error!("Can't find API: {}", e)), + Err(e) => return Err(MapMemoryError::LoadApi(e)), } }; @@ -174,14 +190,19 @@ impl VirtualMachine for WhpVm { MemoryRegionFlags::WRITE => Ok(WHvMapGpaRangeFlagWrite), MemoryRegionFlags::EXECUTE => Ok(WHvMapGpaRangeFlagExecute), MemoryRegionFlags::STACK_GUARD => Ok(WHvMapGpaRangeFlagNone), - _ => Err(new_error!("Invalid Memory Region Flag")), + _ => Err(MapMemoryError::InvalidFlags(format!( + "Invalid memory region flag: {:?}", + flag + ))), }) - .collect::>>()? + .collect::, MapMemoryError>>()? .iter() .fold(WHvMapGpaRangeFlagNone, |acc, flag| acc | *flag); // Calculate the surrogate process address for this region - let surrogate_addr = (isize::try_from(region.host_region.start)? + offset) as *const c_void; + let host_start_isize = + isize::try_from(region.host_region.start).map_err(MapMemoryError::AddressConversion)?; + let surrogate_addr = (host_start_isize + offset) as *const c_void; let res = unsafe { whvmapgparange2_func( @@ -194,18 +215,25 @@ impl VirtualMachine for WhpVm { ) }; if res.is_err() { - return Err(new_error!("Call to WHvMapGpaRange2 failed")); + return Err(MapMemoryError::Hypervisor( + HypervisorImplError::WindowsError(windows_result::Error::from_hresult(res)), + )); } Ok(()) } - fn unmap_memory(&mut self, (_slot, _region): (u32, &MemoryRegion)) -> Result<()> { - log_then_return!("Mapping host memory into the guest not yet supported on this platform"); + fn unmap_memory( + &mut self, + (_slot, _region): (u32, &MemoryRegion), + ) -> std::result::Result<(), UnmapMemoryError> { + Err(UnmapMemoryError::NotSupported( + "Unmapping host memory from the guest not yet supported on this platform".to_string(), + )) } #[expect(non_upper_case_globals, reason = "Windows API constant are lower case")] - fn run_vcpu(&mut self) -> Result { + fn run_vcpu(&mut self) -> std::result::Result { let mut exit_context: WHV_RUN_VP_EXIT_CONTEXT = Default::default(); unsafe { @@ -214,7 +242,8 @@ impl VirtualMachine for WhpVm { 0, &mut exit_context as *mut _ as *mut c_void, std::mem::size_of::() as u32, - )?; + ) + .map_err(|e| RunVcpuError::Unknown(e.into()))?; } let result = match exit_context.ExitReason { @@ -224,7 +253,8 @@ impl VirtualMachine for WhpVm { self.set_registers(&[( WHvX64RegisterRip, Align16(WHV_REGISTER_VALUE { Reg64: rip }), - )])?; + )]) + .map_err(|e| RunVcpuError::IncrementRip(e.into()))?; VmExit::IoOut( exit_context.Anonymous.IoPortAccess.PortNumber, exit_context @@ -244,7 +274,8 @@ impl VirtualMachine for WhpVm { (exit_context.Anonymous.MemoryAccess.AccessInfo.AsUINT32 & 0b11) as i32, ) }; - let access_info = MemoryRegionFlags::try_from(access_info)?; + let access_info = MemoryRegionFlags::try_from(access_info) + .map_err(|_| RunVcpuError::ParseGpaAccessInfo)?; match access_info { MemoryRegionFlags::READ => VmExit::MmioRead(gpa), MemoryRegionFlags::WRITE => VmExit::MmioWrite(gpa), @@ -268,7 +299,8 @@ impl VirtualMachine for WhpVm { names.as_ptr(), 1, out.as_mut_ptr() as *mut WHV_REGISTER_VALUE, - )?; + ) + .map_err(|e| RunVcpuError::GetDr6(e.into()))?; } unsafe { out[0].0.Reg64 } }; @@ -286,7 +318,7 @@ impl VirtualMachine for WhpVm { Ok(result) } - fn regs(&self) -> Result { + fn regs(&self) -> std::result::Result { let mut whv_regs_values: [Align16; WHP_REGS_NAMES_LEN] = unsafe { std::mem::zeroed() }; @@ -297,7 +329,8 @@ impl VirtualMachine for WhpVm { WHP_REGS_NAMES.as_ptr(), whv_regs_values.len() as u32, whv_regs_values.as_mut_ptr() as *mut WHV_REGISTER_VALUE, - )?; + ) + .map_err(|e| RegisterError::GetRegs(e.into()))?; } WHP_REGS_NAMES @@ -307,21 +340,29 @@ impl VirtualMachine for WhpVm { .as_slice() .try_into() .map_err(|e| { - new_error!( - "Failed to convert WHP registers to CommonRegisters: {:?}", - e + RegisterError::GetRegs( + crate::hypervisor::virtual_machine::HypervisorImplError::WindowsError( + windows_result::Error::new( + HRESULT::from_win32(160), // ERROR_BAD_ARGUMENTS + format!( + "Failed to convert WHP registers to CommonRegisters: {:?}", + e + ), + ), + ), ) }) } - fn set_regs(&self, regs: &CommonRegisters) -> Result<()> { + fn set_regs(&self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { let whp_regs: [(WHV_REGISTER_NAME, Align16); WHP_REGS_NAMES_LEN] = regs.into(); - self.set_registers(&whp_regs)?; + self.set_registers(&whp_regs) + .map_err(|e| RegisterError::SetRegs(e.into()))?; Ok(()) } - fn fpu(&self) -> Result { + fn fpu(&self) -> std::result::Result { let mut whp_fpu_values: [Align16; WHP_FPU_NAMES_LEN] = unsafe { std::mem::zeroed() }; @@ -332,7 +373,8 @@ impl VirtualMachine for WhpVm { WHP_FPU_NAMES.as_ptr(), whp_fpu_values.len() as u32, whp_fpu_values.as_mut_ptr() as *mut WHV_REGISTER_VALUE, - )?; + ) + .map_err(|e| RegisterError::GetFpu(e.into()))?; } WHP_FPU_NAMES @@ -341,17 +383,27 @@ impl VirtualMachine for WhpVm { .collect::)>>() .as_slice() .try_into() - .map_err(|e| new_error!("Failed to convert WHP registers to CommonFpu: {:?}", e)) + .map_err(|e| { + RegisterError::GetFpu( + crate::hypervisor::virtual_machine::HypervisorImplError::WindowsError( + windows_result::Error::new( + HRESULT::from_win32(160), // ERROR_BAD_ARGUMENTS + format!("Failed to convert WHP registers to CommonFpu: {:?}", e), + ), + ), + ) + }) } - fn set_fpu(&self, fpu: &CommonFpu) -> Result<()> { + fn set_fpu(&self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError> { let whp_fpu: [(WHV_REGISTER_NAME, Align16); WHP_FPU_NAMES_LEN] = fpu.into(); - self.set_registers(&whp_fpu)?; + self.set_registers(&whp_fpu) + .map_err(|e| RegisterError::SetFpu(e.into()))?; Ok(()) } - fn sregs(&self) -> Result { + fn sregs(&self) -> std::result::Result { let mut whp_sregs_values: [Align16; WHP_SREGS_NAMES_LEN] = unsafe { std::mem::zeroed() }; @@ -362,7 +414,8 @@ impl VirtualMachine for WhpVm { WHP_SREGS_NAMES.as_ptr(), whp_sregs_values.len() as u32, whp_sregs_values.as_mut_ptr() as *mut WHV_REGISTER_VALUE, - )?; + ) + .map_err(|e| RegisterError::GetSregs(e.into()))?; } WHP_SREGS_NAMES @@ -372,24 +425,30 @@ impl VirtualMachine for WhpVm { .as_slice() .try_into() .map_err(|e| { - new_error!( - "Failed to convert WHP registers to CommonSpecialRegisters: {:?}", - e + RegisterError::GetSregs( + crate::hypervisor::virtual_machine::HypervisorImplError::WindowsError( + windows_result::Error::new( + HRESULT::from_win32(160), // ERROR_BAD_ARGUMENTS + format!( + "Failed to convert WHP registers to CommonSpecialRegisters: {:?}", + e + ), + ), + ), ) }) } - fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> Result<()> { + fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> std::result::Result<(), RegisterError> { let whp_regs: [(WHV_REGISTER_NAME, Align16); WHP_SREGS_NAMES_LEN] = sregs.into(); - self.set_registers(&whp_regs)?; + self.set_registers(&whp_regs) + .map_err(|e| RegisterError::SetSregs(e.into()))?; Ok(()) } #[cfg(crashdump)] - fn xsave(&self) -> Result> { - use crate::HyperlightError; - + fn xsave(&self) -> std::result::Result, RegisterError> { // Get the required buffer size by calling with NULL buffer. // If the buffer is not large enough (0 won't be), WHvGetVirtualProcessorXsaveState returns // WHV_E_INSUFFICIENT_BUFFER and sets buffer_size_needed to the required size. @@ -409,7 +468,7 @@ impl VirtualMachine for WhpVm { if let Err(e) = result && e.code() != windows::Win32::Foundation::WHV_E_INSUFFICIENT_BUFFER { - return Err(HyperlightError::WindowsAPIError(e)); + return Err(RegisterError::GetXsave(e.into())); } // Allocate buffer with the required size @@ -425,15 +484,15 @@ impl VirtualMachine for WhpVm { buffer_size_needed, &mut written_bytes, ) - }?; + } + .map_err(|e| RegisterError::GetXsave(e.into()))?; // Verify the number of written bytes matches the expected size if written_bytes != buffer_size_needed { - return Err(new_error!( - "Failed to get Xsave state: expected {} bytes, got {}", - buffer_size_needed, - written_bytes - )); + return Err(RegisterError::XsaveSizeMismatch { + expected: buffer_size_needed, + actual: written_bytes, + }); } Ok(xsave_buffer) @@ -452,7 +511,7 @@ impl VirtualMachine for WhpVm { #[cfg(gdb)] impl DebuggableVm for WhpVm { - fn translate_gva(&self, gva: u64) -> Result { + fn translate_gva(&self, gva: u64) -> std::result::Result { let mut gpa = 0; let mut result = WHV_TRANSLATE_GVA_RESULT::default(); @@ -468,13 +527,14 @@ impl DebuggableVm for WhpVm { translateflags, &mut result, &mut gpa, - )?; + ) + .map_err(|_| DebugError::TranslateGva(gva))?; } Ok(gpa) } - fn set_debug(&mut self, enable: bool) -> Result<()> { + fn set_debug(&mut self, enable: bool) -> std::result::Result<(), DebugError> { let extended_vm_exits = if enable { 1 << 2 } else { 0 }; let exception_exit_bitmap = if enable { (1 << WHvX64ExceptionTypeDebugTrapOrFault.0) @@ -507,13 +567,17 @@ impl DebuggableVm for WhpVm { code, &property as *const _ as *const c_void, std::mem::size_of::() as u32, - )?; + ) + .map_err(|e| DebugError::Intercept { + enable, + inner: e.into(), + })?; } } Ok(()) } - fn set_single_step(&mut self, enable: bool) -> Result<()> { + fn set_single_step(&mut self, enable: bool) -> std::result::Result<(), DebugError> { let mut regs = self.regs()?; if enable { regs.rflags |= 1 << 8; @@ -524,7 +588,7 @@ impl DebuggableVm for WhpVm { Ok(()) } - fn add_hw_breakpoint(&mut self, addr: u64) -> Result<()> { + fn add_hw_breakpoint(&mut self, addr: u64) -> std::result::Result<(), DebugError> { use crate::hypervisor::gdb::arch::MAX_NO_OF_HW_BP; // Get current debug registers @@ -546,7 +610,8 @@ impl DebuggableVm for WhpVm { names.as_ptr(), LEN as u32, out.as_mut_ptr() as *mut WHV_REGISTER_VALUE, - )?; + ) + .map_err(|e| RegisterError::GetDebugRegs(e.into()))?; } let mut dr0 = unsafe { out[0].0.Reg64 }; @@ -563,7 +628,7 @@ impl DebuggableVm for WhpVm { // Find the first available LOCAL (L0–L3) slot let i = (0..MAX_NO_OF_HW_BP) .position(|i| dr7 & (1 << (i * 2)) == 0) - .ok_or_else(|| new_error!("Tried to add more than 4 hardware breakpoints"))?; + .ok_or(DebugError::TooManyHwBreakpoints(MAX_NO_OF_HW_BP))?; // Assign to corresponding debug register *[&mut dr0, &mut dr1, &mut dr2, &mut dr3][i] = addr; @@ -594,11 +659,12 @@ impl DebuggableVm for WhpVm { Align16(WHV_REGISTER_VALUE { Reg64: dr7 }), ), ]; - self.set_registers(®isters)?; + self.set_registers(®isters) + .map_err(|e| RegisterError::SetDebugRegs(e.into()))?; Ok(()) } - fn remove_hw_breakpoint(&mut self, addr: u64) -> Result<()> { + fn remove_hw_breakpoint(&mut self, addr: u64) -> std::result::Result<(), DebugError> { // Get current debug registers const LEN: usize = 6; let names: [WHV_REGISTER_NAME; LEN] = [ @@ -618,7 +684,8 @@ impl DebuggableVm for WhpVm { names.as_ptr(), LEN as u32, out.as_mut_ptr() as *mut WHV_REGISTER_VALUE, - )?; + ) + .map_err(|e| RegisterError::GetDebugRegs(e.into()))?; } let mut dr0 = unsafe { out[0].0.Reg64 }; @@ -658,10 +725,11 @@ impl DebuggableVm for WhpVm { Align16(WHV_REGISTER_VALUE { Reg64: dr7 }), ), ]; - self.set_registers(®isters)?; + self.set_registers(®isters) + .map_err(|e| RegisterError::SetDebugRegs(e.into()))?; Ok(()) } else { - Err(new_error!("Tried to remove non-existing hw-breakpoint")) + Err(DebugError::HwBreakpointNotFound(addr)) } } } @@ -695,29 +763,22 @@ type WHvMapGpaRange2Func = unsafe extern "C" fn( WHV_MAP_GPA_RANGE_FLAGS, ) -> HRESULT; -unsafe fn try_load_whv_map_gpa_range2() -> Result { +unsafe fn try_load_whv_map_gpa_range2() -> windows_result::Result { let library = unsafe { LoadLibraryExA( s!("winhvplatform.dll"), None, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS, ) - }; - - if let Err(e) = library { - return Err(new_error!("{}", e)); - } - - #[allow(clippy::unwrap_used)] - // We know this will succeed because we just checked for an error above - let library = library.unwrap(); + }?; let address = unsafe { GetProcAddress(library, s!("WHvMapGpaRange2")) }; if address.is_none() { unsafe { FreeLibrary(library)? }; - return Err(new_error!( - "Failed to find WHvMapGpaRange2 in winhvplatform.dll" + return Err(windows_result::Error::new( + HRESULT::from_win32(0x8007007F), // ERROR_PROC_NOT_FOUND + "Failed to find WHvMapGpaRange2 in winhvplatform.dll", )); } diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 349536cfc..953b752ff 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -38,7 +38,7 @@ use super::snapshot::Snapshot; use crate::HyperlightError::{self, SnapshotSandboxMismatch}; use crate::func::{ParameterTuple, SupportedReturnType}; use crate::hypervisor::InterruptHandle; -use crate::hypervisor::hyperlight_vm::HyperlightVm; +use crate::hypervisor::hyperlight_vm::{HyperlightVm, HyperlightVmError}; use crate::mem::memory_region::MemoryRegion; #[cfg(unix)] use crate::mem::memory_region::{MemoryRegionFlags, MemoryRegionType}; @@ -274,13 +274,15 @@ impl MultiUseSandbox { let regions_to_map = snapshot_regions.difference(¤t_regions); for region in regions_to_unmap { - self.vm.unmap_region(region)?; + self.vm + .unmap_region(region) + .map_err(HyperlightVmError::UnmapRegion)?; } for region in regions_to_map { // Safety: The region has been mapped before, and at that point the caller promised that the memory region is valid // in their call to `MultiUseSandbox::map_region` - unsafe { self.vm.map_region(region)? }; + unsafe { self.vm.map_region(region) }.map_err(HyperlightVmError::MapRegion)?; } // The restored snapshot is now our most current snapshot @@ -490,7 +492,7 @@ impl MultiUseSandbox { } // Reset snapshot since we are mutating the sandbox state self.snapshot = None; - unsafe { self.vm.map_region(rgn) }?; + unsafe { self.vm.map_region(rgn) }.map_err(HyperlightVmError::MapRegion)?; self.mem_mgr.mapped_rgns += 1; Ok(()) } @@ -597,13 +599,21 @@ impl MultiUseSandbox { self.mem_mgr.write_guest_function_call(buffer)?; - self.vm.dispatch_call_from_host( + let dispatch_res = self.vm.dispatch_call_from_host( self.dispatch_ptr.clone(), &mut self.mem_mgr, &self.host_funcs, #[cfg(gdb)] self.dbg_mem_access_fn.clone(), - )?; + ); + + // Convert dispatch errors to HyperlightErrors to maintain backwards compatibility + // but first determine if sandbox should be poisoned + if let Err(e) = dispatch_res { + let (error, should_poison) = e.promote(); + self.poisoned |= should_poison; + return Err(error); + } self.mem_mgr.check_stack_guard()?; diff --git a/src/hyperlight_host/src/sandbox/outb.rs b/src/hyperlight_host/src/sandbox/outb.rs index 62d8d4c17..76e415a23 100644 --- a/src/hyperlight_host/src/sandbox/outb.rs +++ b/src/hyperlight_host/src/sandbox/outb.rs @@ -31,10 +31,42 @@ use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::HostSharedMemory; #[cfg(feature = "mem_profile")] use crate::sandbox::trace::MemTraceInfo; -use crate::{HyperlightError, Result, new_error}; + +/// Errors that can occur when handling an outb operation from the guest. +#[derive(Debug, thiserror::Error, displaydoc::Display)] +pub enum HandleOutbError { + /// Stack overflow detected (signaled by guest) + StackOverflow, + /// Guest aborted: error code {code}, message: {message} + GuestAborted { + /// The error code from the guest + code: u8, + /// The error message from the guest + message: String, + }, + /// Invalid outb port: {0} + InvalidPort(String), + /// Failed to read guest log data: {0} + ReadLogData(String), + /// Trace formatting error: {0} + TraceFormat(String), + /// Failed to read host function call: {0} + ReadHostFunctionCall(String), + /// Failed to acquire lock at {0}:{1} - {2} + LockFailed(&'static str, u32, String), + /// Failed to write host function response: {0} + WriteHostFunctionResponse(String), + /// Invalid character for debug print: {0} + InvalidDebugPrintChar(u32), + /// Memory profiling error: {0} + #[cfg(feature = "mem_profile")] + MemProfile(String), +} #[instrument(err(Debug), skip_all, parent = Span::current(), level="Trace")] -pub(super) fn outb_log(mgr: &mut SandboxMemoryManager) -> Result<()> { +pub(super) fn outb_log( + mgr: &mut SandboxMemoryManager, +) -> Result<(), HandleOutbError> { // This code will create either a logging record or a tracing record for the GuestLogData depending on if the host has set up a tracing subscriber. // In theory as we have enabled the log feature in the Cargo.toml for tracing this should happen // automatically (based on if there is tracing subscriber present) but only works if the event created using macros. (see https://github.com/tokio-rs/tracing/blob/master/tracing/src/macros.rs#L2421 ) @@ -42,7 +74,9 @@ pub(super) fn outb_log(mgr: &mut SandboxMemoryManager) -> Resu // set the file and line number for the log record which is not possible with macros. // This is because the file and line number come from the guest not the call site. - let log_data: GuestLogData = mgr.read_guest_log_data()?; + let log_data: GuestLogData = mgr + .read_guest_log_data() + .map_err(|e| HandleOutbError::ReadLogData(e.to_string()))?; let record_level: Level = (&log_data.level).into(); @@ -77,7 +111,8 @@ pub(super) fn outb_log(mgr: &mut SandboxMemoryManager) -> Resu .line(line) .module_path(source) .build(), - )?; + ) + .map_err(|e| HandleOutbError::TraceFormat(e.to_string()))?; } else { // Create a log record for the GuestLogData log::logger().log( @@ -98,7 +133,10 @@ pub(super) fn outb_log(mgr: &mut SandboxMemoryManager) -> Resu const ABORT_TERMINATOR: u8 = 0xFF; const MAX_ABORT_BUFFER_LEN: usize = 1024; -fn outb_abort(mem_mgr: &mut SandboxMemoryManager, data: u32) -> Result<()> { +fn outb_abort( + mem_mgr: &mut SandboxMemoryManager, + data: u32, +) -> Result<(), HandleOutbError> { let buffer = mem_mgr.get_abort_buffer_mut(); let bytes = data.to_le_bytes(); // [len, b1, b2, b3] @@ -110,7 +148,7 @@ fn outb_abort(mem_mgr: &mut SandboxMemoryManager, data: u32) - let guest_error = ErrorCode::from(guest_error_code as u64); let result = match guest_error { - ErrorCode::StackOverflow => Err(HyperlightError::StackOverflow()), + ErrorCode::StackOverflow => Err(HandleOutbError::StackOverflow), _ => { let message = if let Some(&maybe_exception_code) = buffer.get(1) { match Exception::try_from(maybe_exception_code) { @@ -124,7 +162,10 @@ fn outb_abort(mem_mgr: &mut SandboxMemoryManager, data: u32) - String::new() }; - Err(HyperlightError::GuestAborted(guest_error_code, message)) + Err(HandleOutbError::GuestAborted { + code: guest_error_code, + message, + }) } }; @@ -134,10 +175,10 @@ fn outb_abort(mem_mgr: &mut SandboxMemoryManager, data: u32) - if buffer.len() >= MAX_ABORT_BUFFER_LEN { buffer.clear(); - return Err(HyperlightError::GuestAborted( - 0, - "Guest abort buffer overflowed".into(), - )); + return Err(HandleOutbError::GuestAborted { + code: 0, + message: "Guest abort buffer overflowed".into(), + }); } buffer.push(b); @@ -154,22 +195,29 @@ pub(crate) fn handle_outb( data: u32, #[cfg(feature = "mem_profile")] regs: &CommonRegisters, #[cfg(feature = "mem_profile")] trace_info: &mut MemTraceInfo, -) -> Result<()> { - match port.try_into()? { +) -> Result<(), HandleOutbError> { + match port + .try_into() + .map_err(|e: anyhow::Error| HandleOutbError::InvalidPort(e.to_string()))? + { OutBAction::Log => outb_log(mem_mgr), OutBAction::CallFunction => { - let call = mem_mgr.get_host_function_call()?; // pop output buffer + let call = mem_mgr + .get_host_function_call() + .map_err(|e| HandleOutbError::ReadHostFunctionCall(e.to_string()))?; let name = call.function_name.clone(); let args: Vec = call.parameters.unwrap_or(vec![]); let res = host_funcs .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? + .map_err(|e| HandleOutbError::LockFailed(file!(), line!(), e.to_string()))? .call_host_function(&name, args) .map_err(|e| GuestError::new(ErrorCode::HostFunctionError, e.to_string())); let func_result = FunctionCallResult::new(res); - mem_mgr.write_response_from_host_function_call(&func_result)?; + mem_mgr + .write_response_from_host_function_call(&func_result) + .map_err(|e| HandleOutbError::WriteHostFunctionResponse(e.to_string()))?; Ok(()) } @@ -178,7 +226,7 @@ pub(crate) fn handle_outb( let ch: char = match char::from_u32(data) { Some(c) => c, None => { - return Err(new_error!("Invalid character for logging: {}", data)); + return Err(HandleOutbError::InvalidDebugPrintChar(data)); } }; diff --git a/src/hyperlight_host/src/sandbox/trace/mem_profile.rs b/src/hyperlight_host/src/sandbox/trace/mem_profile.rs index f181b0a7e..9b436145b 100644 --- a/src/hyperlight_host/src/sandbox/trace/mem_profile.rs +++ b/src/hyperlight_host/src/sandbox/trace/mem_profile.rs @@ -23,6 +23,7 @@ use crate::hypervisor::regs::CommonRegisters; use crate::mem::layout::SandboxMemoryLayout; use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::HostSharedMemory; +use crate::sandbox::outb::HandleOutbError; use crate::{Result, new_error}; /// The type of trace frame being recorded. @@ -147,7 +148,7 @@ impl MemTraceInfo { regs: &CommonRegisters, mem_mgr: &SandboxMemoryManager, trace_identifier: TraceFrameType, - ) -> Result<()> { + ) -> std::result::Result<(), HandleOutbError> { let Ok(stack) = self.unwind(regs, mem_mgr) else { return Ok(()); }; @@ -156,20 +157,20 @@ impl MemTraceInfo { let ptr = regs.rcx; match trace_identifier { - TraceFrameType::MemAlloc => { - self.record_trace_frame(self.epoch, trace_identifier as u64, |f| { + TraceFrameType::MemAlloc => self + .record_trace_frame(self.epoch, trace_identifier as u64, |f| { let _ = f.write_all(&ptr.to_ne_bytes()); let _ = f.write_all(&amt.to_ne_bytes()); self.write_stack(f, &stack); }) - } + .map_err(|e| HandleOutbError::MemProfile(e.to_string())), // The MemFree case does not expect an amount, only a pointer - TraceFrameType::MemFree => { - self.record_trace_frame(self.epoch, trace_identifier as u64, |f| { + TraceFrameType::MemFree => self + .record_trace_frame(self.epoch, trace_identifier as u64, |f| { let _ = f.write_all(&ptr.to_ne_bytes()); self.write_stack(f, &stack); }) - } + .map_err(|e| HandleOutbError::MemProfile(e.to_string())), } } @@ -178,7 +179,7 @@ impl MemTraceInfo { &self, regs: &CommonRegisters, mem_mgr: &SandboxMemoryManager, - ) -> Result<()> { + ) -> std::result::Result<(), HandleOutbError> { self.handle_trace(regs, mem_mgr, TraceFrameType::MemAlloc) } @@ -187,7 +188,7 @@ impl MemTraceInfo { &self, regs: &CommonRegisters, mem_mgr: &SandboxMemoryManager, - ) -> Result<()> { + ) -> std::result::Result<(), HandleOutbError> { self.handle_trace(regs, mem_mgr, TraceFrameType::MemFree) } } diff --git a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs index c9f6bc220..afb7df5ff 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs @@ -23,7 +23,7 @@ use tracing::{Span, instrument}; use super::SandboxConfiguration; #[cfg(any(crashdump, gdb))] use super::uninitialized::SandboxRuntimeConfig; -use crate::hypervisor::hyperlight_vm::HyperlightVm; +use crate::hypervisor::hyperlight_vm::{HyperlightVm, HyperlightVmError}; use crate::mem::exe::LoadInfo; use crate::mem::mgr::SandboxMemoryManager; use crate::mem::ptr::{GuestPtr, RawPtr}; @@ -74,7 +74,8 @@ pub(super) fn evolve_impl_multi_use(u_sbox: UninitializedSandbox) -> Result