Skip to content

Richer errors for HyperlightVm #1162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ludfjig wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Richer errors for HyperlightVm #1162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 18 additions & 21 deletions src/hyperlight_host/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -111,6 +112,14 @@ pub enum HyperlightError {
#[error("HostFunction {0} was not found")]
HostFunctionNotFound(String),

/// Hyperlight VM error.
///
/// **Note:** This error variant is considered internal and its structure is not stable.
/// It may change between versions without notice. Users should not rely on this.
#[doc(hidden)]
#[error("Internal 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),
Expand All @@ -127,11 +136,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),
Expand Down Expand Up @@ -168,11 +172,6 @@ pub enum HyperlightError {
#[error("mprotect failed with os error {0:?}")]
MprotectFailed(Option<i32>),

/// 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(),
Expand Down Expand Up @@ -242,11 +241,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),
Expand Down Expand Up @@ -334,6 +328,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(_, _)
Expand All @@ -348,6 +347,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(_)
Expand Down Expand Up @@ -384,12 +387,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,
}
}
}
Expand Down
15 changes: 12 additions & 3 deletions src/hyperlight_host/src/hypervisor/gdb/arch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
pub enum VcpuStopReasonError {
#[error("Failed to get registers: {0}")]
GetRegs(#[from] RegisterError),
#[error("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
Expand Down Expand Up @@ -56,7 +65,7 @@ pub(crate) fn vcpu_stop_reason(
dr6: u64,
entrypoint: u64,
exception: u32,
) -> Result<VcpuStopReason> {
) -> std::result::Result<VcpuStopReason, VcpuStopReasonError> {
let CommonRegisters { rip, .. } = vm.regs()?;
if DB_EX_ID == exception {
// If the BS flag in DR6 register is set, it means a single step
Expand Down
95 changes: 71 additions & 24 deletions src/hyperlight_host/src/hypervisor/gdb/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::{HypervisorError, 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")]
Expand Down Expand Up @@ -86,6 +86,17 @@ pub(crate) struct DebugMemoryAccess {
pub(crate) guest_mmap_regions: Vec<MemoryRegion>,
}

/// Errors that can occur during debug memory access operations
#[derive(Debug, thiserror::Error)]
pub enum DebugMemoryAccessError {
#[error("Failed to copy memory: {0}")]
CopyFailed(Box<HyperlightError>),
#[error("Failed to acquire lock at {0}:{1} - {2}")]
LockFailed(&'static str, u32, String),
#[error("Failed to translate guest address {0:#x}")]
TranslateGuestAddress(u64),
}

impl DebugMemoryAccess {
/// Reads memory from the guest's address space with a maximum length of a PAGE_SIZE
///
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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(())
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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(())
Expand Down Expand Up @@ -275,24 +296,42 @@ pub(crate) enum DebugResponse {
WriteRegisters,
}

/// Errors that can occur during debug operations
#[derive(Debug, Clone, thiserror::Error)]
pub enum DebugError {
#[error("Hardware breakpoint not found at address {0:#x}")]
HwBreakpointNotFound(u64),
#[error("Failed to enable/disable intercept: {enable}, {inner}")]
Intercept {
enable: bool,
inner: HypervisorError,
},
#[error("Register operation failed: {0}")]
Register(#[from] RegisterError),
#[error("Maximum hardware breakpoints ({0}) exceeded")]
TooManyHwBreakpoints(usize),
#[error("Translation of guest virtual address failed: {0}")]
TranslateGva(u64),
}

/// 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<u64>;
fn translate_gva(&self, gva: u64) -> std::result::Result<u64, DebugError>;

/// 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
Expand Down Expand Up @@ -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);

Expand All @@ -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,
Expand All @@ -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]);
Expand All @@ -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);
Expand Down
Loading
Loading