diff --git a/CHANGELOG.md b/CHANGELOG.md index 516928815..51c781fae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Prerelease] - Unreleased ### Added +* Add `MultiUseSandbox::status()`, which returns `SandboxStatus` for inspecting sandbox lifecycle state. ### Changed * **Breaking:** Guest MSR state is now saved and restored across snapshots. @@ -13,10 +14,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). resets to a clean default. On KVM the guest may only read or write declared MSRs, on MSHV and WHP this is not enforced. by @ludfjig in https://github.com/hyperlight-dev/hyperlight/pull/991 * **Breaking:** Filesystem paths are now represented using `PathBuf`. `GuestBinary::FilePath` now stores a `PathBuf` instead of a `String`, and `MultiUseSandbox::generate_crashdump_to_dir` accepts `Into` instead of `Into`. Callers passing a `String` to `GuestBinary::FilePath` must convert it using `.into()`. +* Deprecate `MultiUseSandbox::poisoned` in favor of `MultiUseSandbox::status().is_poisoned()`. ### Removed ### Fixed +* Mark a sandbox unrecoverable when snapshot restore fails while updating its VM mappings. * Fix symbol resolution in guest core dumps for sandboxes created from snapshots by @ludfjig in https://github.com/hyperlight-dev/hyperlight/pull/1618 * Reject malformed OCI snapshot metadata and non-regular artifact files during load. * Reset XCR0 during x86 snapshot restore. diff --git a/src/hyperlight_host/src/error.rs b/src/hyperlight_host/src/error.rs index c6738374d..85d56565b 100644 --- a/src/hyperlight_host/src/error.rs +++ b/src/hyperlight_host/src/error.rs @@ -224,6 +224,10 @@ pub enum HyperlightError { #[error("The sandbox was poisoned")] PoisonedSandbox, + /// The sandbox cannot safely perform further operations. + #[error("The sandbox is unrecoverable and must be discarded")] + UnrecoverableSandbox, + /// Raw pointer is less than base address #[error("Raw pointer ({0:?}) was less than the base address ({1})")] RawPointerLessThanBaseAddress(RawPtr, u64), @@ -408,6 +412,7 @@ impl HyperlightError { | HyperlightError::UnexpectedNoOfArguments(_, _) | HyperlightError::UnexpectedParameterValueType(_, _) | HyperlightError::UnexpectedReturnValueType(_, _) + | HyperlightError::UnrecoverableSandbox | HyperlightError::UTF8StringConversionFailure(_) | HyperlightError::VectorCapacityIncorrect(_, _, _) => false, diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs index 93368a335..9e3c6da19 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs @@ -19,6 +19,8 @@ mod x86_64; #[cfg(target_arch = "aarch64")] mod aarch64; +#[cfg(all(test, not(gdb), any(kvm, mshv3, target_os = "windows")))] +pub(crate) mod test_support; #[cfg(gdb)] use std::collections::HashMap; use std::str::FromStr; @@ -532,11 +534,12 @@ impl HyperlightVm { let guest_base = crate::mem::layout::SandboxMemoryLayout::BASE_ADDRESS as u64; let rgn = snapshot.mapping_at(guest_base, MemoryRegionType::Snapshot); - if let Some(old_snapshot) = self.snapshot_memory.replace(snapshot) { + if let Some(old_snapshot) = self.snapshot_memory.as_ref() { let old_rgn = old_snapshot.mapping_at(guest_base, MemoryRegionType::Snapshot); self.vm.unmap_memory((self.snapshot_slot, &old_rgn))?; } unsafe { self.vm.map_memory((self.snapshot_slot, &rgn))? }; + self.snapshot_memory = Some(snapshot); Ok(()) } @@ -549,12 +552,13 @@ impl HyperlightVm { let guest_base = hyperlight_common::layout::scratch_base_gpa(scratch.mem_size()); let rgn = scratch.mapping_at(guest_base, MemoryRegionType::Scratch); - if let Some(old_scratch) = self.scratch_memory.replace(scratch) { + if let Some(old_scratch) = self.scratch_memory.as_ref() { let old_base = hyperlight_common::layout::scratch_base_gpa(old_scratch.mem_size()); let old_rgn = old_scratch.mapping_at(old_base, MemoryRegionType::Scratch); self.vm.unmap_memory((self.scratch_slot, &old_rgn))?; } unsafe { self.vm.map_memory((self.scratch_slot, &rgn))? }; + self.scratch_memory = Some(scratch); Ok(()) } diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/test_support.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/test_support.rs new file mode 100644 index 000000000..ed4513109 --- /dev/null +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/test_support.rs @@ -0,0 +1,292 @@ +/* +Copyright 2025 The Hyperlight Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +use std::collections::VecDeque; + +use super::*; +#[cfg(target_arch = "x86_64")] +use crate::hypervisor::regs::MsrEntry; +use crate::hypervisor::regs::{ + CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, +}; +use crate::hypervisor::virtual_machine::{CreateVmError, HypervisorError}; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum VmOperation { + Map(MemoryRegionType), + Unmap(MemoryRegionType), + #[cfg(target_arch = "x86_64")] + SetRegs, + #[cfg(target_arch = "x86_64")] + SetDebugRegs, + #[cfg(target_arch = "x86_64")] + ResetXsave, + #[cfg(target_arch = "x86_64")] + SetSregs, + #[cfg(target_arch = "x86_64")] + SetMsrs, + #[cfg(target_arch = "aarch64")] + ResetVcpu, +} + +#[derive(Clone, Debug)] +pub(crate) struct VmFaultPlan { + operations: Arc>>, +} + +impl VmFaultPlan { + fn new(operations: impl IntoIterator) -> Self { + Self { + operations: Arc::new(Mutex::new(operations.into_iter().collect())), + } + } + + pub(crate) fn is_consumed(&self) -> bool { + self.operations.lock().unwrap().is_empty() + } + + fn should_fail(&self, operation: VmOperation) -> bool { + let mut operations = self.operations.lock().unwrap(); + if operations.front() == Some(&operation) { + operations.pop_front(); + true + } else { + false + } + } +} + +#[derive(Debug)] +struct FaultInjectingVirtualMachine { + inner: Option>, + fault_plan: VmFaultPlan, +} + +impl FaultInjectingVirtualMachine { + fn new( + inner: Box, + operations: impl IntoIterator, + ) -> (Self, VmFaultPlan) { + let fault_plan = VmFaultPlan::new(operations); + ( + Self { + inner: Some(inner), + fault_plan: fault_plan.clone(), + }, + fault_plan, + ) + } + + fn placeholder() -> Self { + Self { + inner: None, + fault_plan: VmFaultPlan::new([]), + } + } + + fn inner(&self) -> &dyn VirtualMachine { + self.inner.as_deref().expect("placeholder VM was used") + } + + fn inner_mut(&mut self) -> &mut dyn VirtualMachine { + self.inner.as_deref_mut().expect("placeholder VM was used") + } + + fn should_fail(&self, operation: VmOperation) -> bool { + self.fault_plan.should_fail(operation) + } + + fn injected_error() -> HypervisorError { + #[cfg(kvm)] + let error = kvm_ioctls::Error::new(libc::EIO); + #[cfg(all(not(kvm), mshv3))] + let error = mshv_ioctls::MshvError::from(libc::EIO); + #[cfg(target_os = "windows")] + let error = windows_result::Error::from_hresult(windows_result::HRESULT::from_win32(5)); + error.into() + } +} + +impl VirtualMachine for FaultInjectingVirtualMachine { + unsafe fn map_memory( + &mut self, + region: (u32, &MemoryRegion), + ) -> std::result::Result<(), MapMemoryError> { + if self.should_fail(VmOperation::Map(region.1.region_type)) { + return Err(MapMemoryError::Hypervisor(Self::injected_error())); + } + // SAFETY: The decorator forwards the caller's preconditions unchanged. + unsafe { self.inner_mut().map_memory(region) } + } + + fn unmap_memory( + &mut self, + region: (u32, &MemoryRegion), + ) -> std::result::Result<(), UnmapMemoryError> { + if self.should_fail(VmOperation::Unmap(region.1.region_type)) { + return Err(UnmapMemoryError::Hypervisor(Self::injected_error())); + } + self.inner_mut().unmap_memory(region) + } + + fn run_vcpu( + &mut self, + #[cfg(feature = "trace_guest")] tc: &mut crate::sandbox::trace::TraceContext, + ) -> std::result::Result { + self.inner_mut().run_vcpu( + #[cfg(feature = "trace_guest")] + tc, + ) + } + + fn regs(&self) -> std::result::Result { + self.inner().regs() + } + + fn set_regs(&self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { + #[cfg(target_arch = "x86_64")] + if self.should_fail(VmOperation::SetRegs) { + return Err(RegisterError::SetRegs(Self::injected_error())); + } + self.inner().set_regs(regs) + } + + fn fpu(&self) -> std::result::Result { + self.inner().fpu() + } + + fn set_fpu(&self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError> { + self.inner().set_fpu(fpu) + } + + fn sregs(&self) -> std::result::Result { + self.inner().sregs() + } + + fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> std::result::Result<(), RegisterError> { + #[cfg(target_arch = "x86_64")] + if self.should_fail(VmOperation::SetSregs) { + return Err(RegisterError::SetSregs(Self::injected_error())); + } + self.inner().set_sregs(sregs) + } + + fn debug_regs(&self) -> std::result::Result { + self.inner().debug_regs() + } + + fn set_debug_regs(&self, drs: &CommonDebugRegs) -> std::result::Result<(), RegisterError> { + #[cfg(target_arch = "x86_64")] + if self.should_fail(VmOperation::SetDebugRegs) { + return Err(RegisterError::SetDebugRegs(Self::injected_error())); + } + self.inner().set_debug_regs(drs) + } + + #[cfg(target_arch = "x86_64")] + fn msrs(&self, indices: &[u32]) -> std::result::Result, RegisterError> { + self.inner().msrs(indices) + } + + #[cfg(target_arch = "x86_64")] + fn set_msrs(&self, msrs: &[MsrEntry]) -> std::result::Result<(), RegisterError> { + if self.should_fail(VmOperation::SetMsrs) { + return Err(RegisterError::SetMsrs(Self::injected_error())); + } + self.inner().set_msrs(msrs) + } + + #[cfg(target_arch = "x86_64")] + fn msr_reset_indices( + &self, + guest_msrs: &[u32], + ) -> std::result::Result, CreateVmError> { + self.inner().msr_reset_indices(guest_msrs) + } + + #[cfg(not(target_arch = "aarch64"))] + fn xsave(&self) -> std::result::Result, RegisterError> { + self.inner().xsave() + } + + #[cfg(not(target_arch = "aarch64"))] + fn reset_xsave(&self) -> std::result::Result<(), RegisterError> { + #[cfg(target_arch = "x86_64")] + if self.should_fail(VmOperation::ResetXsave) { + return Err(RegisterError::SetXsave(Self::injected_error())); + } + self.inner().reset_xsave() + } + + #[cfg(not(target_arch = "aarch64"))] + fn set_xsave(&self, xsave: &[u32]) -> std::result::Result<(), RegisterError> { + self.inner().set_xsave(xsave) + } + + #[cfg(all(test, target_arch = "x86_64"))] + fn xcr0(&self) -> std::result::Result { + self.inner().xcr0() + } + + #[cfg(target_arch = "x86_64")] + fn set_xcr0(&self, value: u64) -> std::result::Result<(), RegisterError> { + self.inner().set_xcr0(value) + } + + #[cfg(target_arch = "aarch64")] + fn can_reset_vcpu(&self) -> bool { + self.inner().can_reset_vcpu() + } + + #[cfg(target_arch = "aarch64")] + fn reset_vcpu(&mut self) -> std::result::Result<(), ResetVcpuError> { + if self.should_fail(VmOperation::ResetVcpu) { + return Err(ResetVcpuError::Hypervisor(Self::injected_error())); + } + self.inner_mut().reset_vcpu() + } + + #[cfg(target_os = "windows")] + fn partition_handle(&self) -> windows::Win32::System::Hypervisor::WHV_PARTITION_HANDLE { + self.inner().partition_handle() + } +} + +impl HyperlightVm { + pub(crate) fn inject_vm_faults( + &mut self, + operations: impl IntoIterator, + ) -> VmFaultPlan { + let placeholder = Box::new(FaultInjectingVirtualMachine::placeholder()); + let inner = std::mem::replace(&mut self.vm, placeholder); + let (vm, fault_plan) = FaultInjectingVirtualMachine::new(inner, operations); + self.vm = Box::new(vm); + fault_plan + } + + #[allow(clippy::type_complexity, reason = "test-only mapping state")] + pub(crate) fn base_mapping_state(&self) -> (Option<(usize, usize)>, Option<(usize, usize)>) { + let snapshot = self + .snapshot_memory + .as_ref() + .map(|memory| (memory.base_addr(), memory.mem_size())); + let scratch = self + .scratch_memory + .as_ref() + .map(|memory| (memory.base_addr(), memory.mem_size())); + (snapshot, scratch) + } +} diff --git a/src/hyperlight_host/src/lib.rs b/src/hyperlight_host/src/lib.rs index 162d0420f..cae18b1bc 100644 --- a/src/hyperlight_host/src/lib.rs +++ b/src/hyperlight_host/src/lib.rs @@ -89,6 +89,8 @@ pub use hypervisor::virtual_machine::is_hypervisor_present; /// A sandbox that can call be used to make multiple calls to guest functions, /// and otherwise reused multiple times pub use sandbox::MultiUseSandbox; +/// The lifecycle state of a [`MultiUseSandbox`]. +pub use sandbox::SandboxStatus; /// The re-export for the `UninitializedSandbox` type pub use sandbox::UninitializedSandbox; /// A collection of host functions that can be supplied to a sandbox diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index f455dffa5..781b2e8d1 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -42,6 +42,34 @@ use crate::metrics::{ }; use crate::{HyperlightError, Result, log_then_return}; +/// The lifecycle state of a [`MultiUseSandbox`]. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum SandboxStatus { + /// The sandbox can execute guest operations. + Ready, + /// The sandbox requires a successful restore before further use. + Poisoned, + /// The sandbox must be discarded. + Unrecoverable, +} + +impl SandboxStatus { + /// Returns whether the sandbox can execute guest operations. + pub const fn is_ready(self) -> bool { + matches!(self, Self::Ready) + } + + /// Returns whether the sandbox requires a successful restore. + pub const fn is_poisoned(self) -> bool { + matches!(self, Self::Poisoned) + } + + /// Returns whether the sandbox must be discarded. + pub const fn is_unrecoverable(self) -> bool { + matches!(self, Self::Unrecoverable) + } +} + /// A fully initialized sandbox that can execute guest functions multiple times. /// /// Guest functions can be called repeatedly while maintaining state between calls. @@ -78,11 +106,12 @@ use crate::{HyperlightError, Result, log_then_return}; /// ### Recovery /// /// Use [`restore()`](Self::restore) with a snapshot taken before poisoning occurred. -/// This is the **only safe way** to recover - it completely replaces all memory state, +/// This completely replaces all memory state, /// eliminating any inconsistencies. See [`restore()`](Self::restore) for details. +/// A sandbox becomes [`SandboxStatus::Unrecoverable`] when restore cannot establish +/// a usable VM mapping state. It must be discarded. pub struct MultiUseSandbox { - /// Whether this sandbox is poisoned - poisoned: bool, + status: SandboxStatus, pub(crate) host_funcs: Arc>, pub(crate) mem_mgr: SandboxMemoryManager, vm: HyperlightVm, @@ -108,6 +137,20 @@ pub struct MultiUseSandbox { pub type PtRootFinder = Box Vec + Send>; impl MultiUseSandbox { + fn ensure_usable(&self) -> Result<()> { + match self.status { + SandboxStatus::Ready => Ok(()), + SandboxStatus::Poisoned => Err(HyperlightError::PoisonedSandbox), + SandboxStatus::Unrecoverable => Err(HyperlightError::UnrecoverableSandbox), + } + } + + fn poison(&mut self) { + if self.status.is_ready() { + self.status = SandboxStatus::Poisoned; + } + } + /// Move an `UninitializedSandbox` into a new `MultiUseSandbox` instance. /// /// This function is not equivalent to doing an `evolve` from uninitialized @@ -120,7 +163,7 @@ impl MultiUseSandbox { vm: HyperlightVm, ) -> MultiUseSandbox { Self { - poisoned: false, + status: SandboxStatus::Ready, host_funcs, mem_mgr: mgr, vm, @@ -346,9 +389,7 @@ impl MultiUseSandbox { /// ``` #[instrument(err(Debug), skip_all, parent = Span::current())] pub fn snapshot(&mut self) -> Result> { - if self.poisoned { - return Err(crate::HyperlightError::PoisonedSandbox); - } + self.ensure_usable()?; if let Some(snapshot) = &self.snapshot { return Ok(snapshot.clone()); @@ -403,6 +444,21 @@ impl MultiUseSandbox { Ok(snapshot) } + fn restore_memory_and_mappings(&mut self, snapshot: &Snapshot) -> Result<()> { + let (snapshot_mem, scratch_mem) = self.mem_mgr.restore_snapshot(snapshot)?; + if let Some(snapshot_mem) = snapshot_mem { + self.vm + .update_snapshot_mapping(snapshot_mem) + .map_err(HyperlightVmError::UpdateRegion)?; + } + if let Some(scratch_mem) = scratch_mem { + self.vm + .update_scratch_mapping(scratch_mem) + .map_err(HyperlightVmError::UpdateRegion)?; + } + Ok(()) + } + /// Restores the sandbox's memory to a previously captured snapshot state. /// /// The snapshot's memory layout must be structurally compatible @@ -425,6 +481,9 @@ impl MultiUseSandbox { /// declare every MSR the snapshot saved, or the restore poisons with an MSR /// mismatch. /// + /// A failure while updating the sandbox's base VM mappings leaves the sandbox + /// [`Unrecoverable`](SandboxStatus::Unrecoverable). It must be discarded. + /// /// ## Poison State Recovery /// /// This method automatically clears any poison state when successful. This is safe because: @@ -482,10 +541,10 @@ impl MultiUseSandbox { /// // This might poison the sandbox (guest not run to completion) /// let result = sandbox.call::<()>("guest_panic", ()); /// if result.is_err() { - /// if sandbox.poisoned() { + /// if sandbox.status().is_poisoned() { /// // Restore from snapshot to clear poison /// sandbox.restore(snapshot.clone())?; - /// assert!(!sandbox.poisoned()); + /// assert!(sandbox.status().is_ready()); /// /// // Sandbox is now usable again /// sandbox.call::("Echo", "hello".to_string())?; @@ -496,6 +555,10 @@ impl MultiUseSandbox { /// ``` #[instrument(err(Debug), skip_all, parent = Span::current())] pub fn restore(&mut self, snapshot: Arc) -> Result<()> { + if self.status.is_unrecoverable() { + return Err(HyperlightError::UnrecoverableSandbox); + } + // Currently, we do not try to optimise restore to the // most-current snapshot. This is because the most-current // snapshot, while it must have identical virtual memory @@ -527,36 +590,28 @@ impl MultiUseSandbox { snapshot.validate_compatibility(&self.mem_mgr.layout, &host_funcs)?; } - let (gsnapshot, gscratch) = self.mem_mgr.restore_snapshot(&snapshot)?; - if let Some(gsnapshot) = gsnapshot { - self.vm - .update_snapshot_mapping(gsnapshot) - .map_err(|e| HyperlightError::HyperlightVmError(e.into()))?; - } - if let Some(gscratch) = gscratch { - self.vm - .update_scratch_mapping(gscratch) - .map_err(|e| HyperlightError::HyperlightVmError(e.into()))?; - } - let sregs = snapshot.sregs().ok_or_else(|| { HyperlightError::Error("snapshot from running sandbox should have sregs".to_string()) })?; - // TODO (ludfjig): Go through the rest of possible errors in this `MultiUseSandbox::restore` function - // and determine if they should also poison the sandbox. + + if let Err(error) = self.restore_memory_and_mappings(&snapshot) { + self.status = SandboxStatus::Unrecoverable; + self.snapshot = None; + return Err(error); + } + + self.status = SandboxStatus::Poisoned; + self.snapshot = None; + self.vm .reset_vcpu(snapshot.root_pt_gpa(), sregs) - .map_err(|e| { - self.poisoned = true; - HyperlightVmError::Restore(e) - })?; + .map_err(HyperlightVmError::Restore)?; // Restore captured MSR state. #[cfg(target_arch = "x86_64")] - self.vm.restore_msrs(snapshot.msrs()).map_err(|e| { - self.poisoned = true; - HyperlightVmError::Restore(e) - })?; + self.vm + .restore_msrs(snapshot.msrs()) + .map_err(HyperlightVmError::Restore)?; self.vm.set_stack_top(snapshot.stack_top_gva()); self.vm.set_next_action(snapshot.next_action()); @@ -585,7 +640,7 @@ impl MultiUseSandbox { // - All leaked heap allocations (memory is restored to snapshot state) // - All corrupted data structures (overwritten with consistent snapshot data) // - All inconsistent global state (reset to snapshot values) - self.poisoned = false; + self.status = SandboxStatus::Ready; Ok(()) } @@ -637,9 +692,7 @@ impl MultiUseSandbox { func_name: &str, args: impl ParameterTuple, ) -> Result { - if self.poisoned { - return Err(crate::HyperlightError::PoisonedSandbox); - } + self.ensure_usable()?; let snapshot = self.snapshot()?; let res = self.call(func_name, args); self.restore(snapshot)?; @@ -660,7 +713,7 @@ impl MultiUseSandbox { /// /// If this method returns an error, the sandbox may be poisoned if the guest was not run /// to completion (due to panic, abort, memory violation, stack/heap exhaustion, or forced - /// termination). Use [`poisoned()`](Self::poisoned) to check the poison state and + /// termination). Use [`status()`](Self::status) to check the sandbox state and /// [`restore()`](Self::restore) to recover if needed. /// /// If this method returns `Ok`, the sandbox is guaranteed to **not** be poisoned - the guest @@ -714,7 +767,7 @@ impl MultiUseSandbox { /// if let Err(e) = result { /// eprintln!("Guest function failed: {}", e); /// - /// if sandbox.poisoned() { + /// if sandbox.status().is_poisoned() { /// eprintln!("Sandbox was poisoned, restoring from snapshot"); /// sandbox.restore(snapshot.clone())?; /// } @@ -728,9 +781,7 @@ impl MultiUseSandbox { func_name: &str, args: impl ParameterTuple, ) -> Result { - if self.poisoned { - return Err(crate::HyperlightError::PoisonedSandbox); - } + self.ensure_usable()?; // Reset snapshot since we are mutating the sandbox state self.snapshot = None; maybe_time_and_emit_guest_call(func_name, || { @@ -763,9 +814,7 @@ impl MultiUseSandbox { /// for the lifetime of `self`. #[instrument(err(Debug), skip(self, rgn), parent = Span::current())] pub unsafe fn map_region(&mut self, rgn: &MemoryRegion) -> Result<()> { - if self.poisoned { - return Err(crate::HyperlightError::PoisonedSandbox); - } + self.ensure_usable()?; if rgn.flags.contains(MemoryRegionFlags::WRITE) { // TODO: Implement support for writable mappings, which // need to be registered with the memory manager so that @@ -789,9 +838,7 @@ impl MultiUseSandbox { /// is currently poisoned. Use [`restore()`](Self::restore) to recover from a poisoned state. #[instrument(err(Debug), skip(self, file_path, guest_base), parent = Span::current())] pub fn map_file_cow(&mut self, file_path: &Path, guest_base: u64) -> Result { - if self.poisoned { - return Err(crate::HyperlightError::PoisonedSandbox); - } + self.ensure_usable()?; // Phase 1: host-side OS work (open file, create mapping) let mut prepared = prepare_file_cow(file_path, guest_base)?; @@ -857,9 +904,7 @@ impl MultiUseSandbox { ret_type: ReturnType, args: Vec, ) -> Result { - if self.poisoned { - return Err(crate::HyperlightError::PoisonedSandbox); - } + self.ensure_usable()?; // Reset snapshot since we are mutating the sandbox state self.snapshot = None; maybe_time_and_emit_guest_call(func_name, || { @@ -873,9 +918,7 @@ impl MultiUseSandbox { return_type: ReturnType, args: Vec, ) -> Result { - if self.poisoned { - return Err(crate::HyperlightError::PoisonedSandbox); - } + self.ensure_usable()?; // ===== KILL() TIMING POINT 1 ===== // Clear any stale cancellation from a previous guest function call or if kill() was called too early. // Any kill() that completed (even partially) BEFORE this line has NO effect on this call. @@ -904,7 +947,9 @@ impl MultiUseSandbox { // but first determine if sandbox should be poisoned if let Err(e) = dispatch_res { let (error, should_poison) = e.promote(); - self.poisoned |= should_poison; + if should_poison { + self.poison(); + } return Err(error); } @@ -939,7 +984,9 @@ impl MultiUseSandbox { self.mem_mgr.clear_io_buffers(); // Determine if we should poison the sandbox. - self.poisoned |= e.is_poison_error(); + if e.is_poison_error() { + self.poison(); + } } // Note: clear_call_active() is automatically called when _guard is dropped here @@ -1037,10 +1084,9 @@ impl MultiUseSandbox { ) } - /// Returns whether the sandbox is currently poisoned. + /// Returns whether the sandbox is poisoned. /// - /// A poisoned sandbox is in an inconsistent state due to the guest not running to completion. - /// All operations will be rejected until the sandbox is restored from a non-poisoned snapshot. + /// Use [`status()`](Self::status) to distinguish every lifecycle state. /// /// ## Causes of Poisoning /// @@ -1059,22 +1105,27 @@ impl MultiUseSandbox { /// # Examples /// /// ```no_run - /// # use hyperlight_host::{MultiUseSandbox, UninitializedSandbox, GuestBinary}; + /// # use hyperlight_host::{MultiUseSandbox, UninitializedSandbox, GuestBinary, SandboxStatus}; /// # fn example() -> Result<(), Box> { /// let mut sandbox: MultiUseSandbox = UninitializedSandbox::new( /// GuestBinary::FilePath("guest.bin".into()), /// None /// )?.evolve()?; /// - /// // Check if sandbox is poisoned - /// if sandbox.poisoned() { - /// println!("Sandbox is poisoned and needs attention"); + /// if sandbox.status().is_poisoned() { + /// println!("Sandbox is poisoned"); /// } /// # Ok(()) /// # } /// ``` + #[deprecated(since = "0.16.0", note = "use status().is_poisoned()")] pub fn poisoned(&self) -> bool { - self.poisoned + self.status.is_poisoned() + } + + /// Returns whether the sandbox is ready, poisoned, or unrecoverable. + pub fn status(&self) -> SandboxStatus { + self.status } } @@ -1084,9 +1135,7 @@ impl Callable for MultiUseSandbox { func_name: &str, args: impl ParameterTuple, ) -> Result { - if self.poisoned { - return Err(crate::HyperlightError::PoisonedSandbox); - } + self.ensure_usable()?; self.call(func_name, args) } } @@ -1148,10 +1197,29 @@ mod tests { use hyperlight_testing::sandbox_sizes::{LARGE_HEAP_SIZE, MEDIUM_HEAP_SIZE, SMALL_HEAP_SIZE}; use hyperlight_testing::simple_guest_as_pathbuf; + #[cfg(not(gdb))] + use crate::hypervisor::hyperlight_vm::test_support::VmOperation; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags, MemoryRegionType}; use crate::mem::shared_mem::{ExclusiveSharedMemory, GuestSharedMemory, SharedMemory as _}; use crate::sandbox::SandboxConfiguration; - use crate::{GuestBinary, HyperlightError, MultiUseSandbox, Result, UninitializedSandbox}; + use crate::{ + GuestBinary, HyperlightError, MultiUseSandbox, Result, SandboxStatus, UninitializedSandbox, + }; + + #[test] + fn sandbox_status_predicates() { + assert!(SandboxStatus::Ready.is_ready()); + assert!(!SandboxStatus::Ready.is_poisoned()); + assert!(!SandboxStatus::Ready.is_unrecoverable()); + + assert!(!SandboxStatus::Poisoned.is_ready()); + assert!(SandboxStatus::Poisoned.is_poisoned()); + assert!(!SandboxStatus::Poisoned.is_unrecoverable()); + + assert!(!SandboxStatus::Unrecoverable.is_ready()); + assert!(!SandboxStatus::Unrecoverable.is_poisoned()); + assert!(SandboxStatus::Unrecoverable.is_unrecoverable()); + } #[test] fn poison() { @@ -1170,7 +1238,7 @@ mod tests { assert!( matches!(res, HyperlightError::GuestAborted(code, context) if code == ErrorCode::UnknownError as u8 && context.contains("hello")) ); - assert!(sbox.poisoned()); + assert!(sbox.status().is_poisoned()); // guest calls should fail when poisoned let res = sbox @@ -1180,7 +1248,7 @@ mod tests { // snapshot should fail when poisoned if let Err(e) = sbox.snapshot() { - assert!(sbox.poisoned()); + assert!(sbox.status().is_poisoned()); assert!(matches!(e, HyperlightError::PoisonedSandbox)); } else { panic!("Snapshot should fail"); @@ -1212,12 +1280,12 @@ mod tests { // restore to non-poisoned snapshot should work and clear poison sbox.restore(snapshot.clone()).unwrap(); - assert!(!sbox.poisoned()); + assert_eq!(sbox.status(), SandboxStatus::Ready); // guest calls should work again after restore let res = sbox.call::("Echo", "hello2".to_string()).unwrap(); assert_eq!(res, "hello2".to_string()); - assert!(!sbox.poisoned()); + assert_eq!(sbox.status(), SandboxStatus::Ready); // re-poison on purpose let res = sbox @@ -1226,16 +1294,16 @@ mod tests { assert!( matches!(res, HyperlightError::GuestAborted(code, context) if code == ErrorCode::UnknownError as u8 && context.contains("hello")) ); - assert!(sbox.poisoned()); + assert!(sbox.status().is_poisoned()); // restore to non-poisoned snapshot should work again sbox.restore(snapshot.clone()).unwrap(); - assert!(!sbox.poisoned()); + assert_eq!(sbox.status(), SandboxStatus::Ready); // guest calls should work again let res = sbox.call::("Echo", "hello3".to_string()).unwrap(); assert_eq!(res, "hello3".to_string()); - assert!(!sbox.poisoned()); + assert_eq!(sbox.status(), SandboxStatus::Ready); // snapshot should work again let _ = sbox.snapshot().unwrap(); @@ -1710,6 +1778,281 @@ mod tests { assert_eq!(sandbox2.call::("GetStatic", ()).unwrap(), 42); } + #[test] + #[cfg(not(gdb))] + fn snapshot_restore_keeps_current_base_mappings() { + let path = simple_guest_as_pathbuf(); + let mut sandbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + let snapshot = sandbox.snapshot().unwrap(); + sandbox.restore(snapshot.clone()).unwrap(); + sandbox.call::("AddToStatic", 42i32).unwrap(); + let mappings = sandbox.vm.base_mapping_state(); + let fault_plan = sandbox + .vm + .inject_vm_faults([VmOperation::Unmap(MemoryRegionType::Snapshot)]); + + sandbox.restore(snapshot).unwrap(); + + assert_eq!(sandbox.status(), SandboxStatus::Ready); + assert_eq!(sandbox.vm.base_mapping_state(), mappings); + assert!(!fault_plan.is_consumed()); + assert_eq!(sandbox.call::("GetStatic", ()).unwrap(), 0); + } + + #[test] + #[cfg(not(gdb))] + fn snapshot_restore_mapping_failure_is_unrecoverable() { + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + source.call::("AddToStatic", 42i32).unwrap(); + let snapshot = source.snapshot().unwrap(); + + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + let mappings = target.vm.base_mapping_state(); + let fault_plan = target + .vm + .inject_vm_faults([VmOperation::Map(MemoryRegionType::Snapshot)]); + + let error = target.restore(snapshot.clone()).unwrap_err(); + assert!(matches!(error, HyperlightError::HyperlightVmError(_))); + assert_eq!(target.status(), SandboxStatus::Unrecoverable); + assert_eq!(target.vm.base_mapping_state(), mappings); + assert!(fault_plan.is_consumed()); + + assert!(matches!( + target.restore(snapshot), + Err(HyperlightError::UnrecoverableSandbox) + )); + assert!(matches!( + target.call::("GetStatic", ()), + Err(HyperlightError::UnrecoverableSandbox) + )); + assert!(matches!( + target.snapshot(), + Err(HyperlightError::UnrecoverableSandbox) + )); + + let map_mem = allocate_guest_memory(); + let region = region_for_memory(&map_mem, 0x200000000_usize, MemoryRegionFlags::READ); + assert!(matches!( + unsafe { target.map_region(®ion) }, + Err(HyperlightError::UnrecoverableSandbox) + )); + } + + #[test] + #[cfg(not(gdb))] + fn snapshot_restore_unmapping_failure_is_unrecoverable() { + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + source.call::("AddToStatic", 42i32).unwrap(); + let snapshot = source.snapshot().unwrap(); + + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + let mappings = target.vm.base_mapping_state(); + let fault_plan = target + .vm + .inject_vm_faults([VmOperation::Unmap(MemoryRegionType::Snapshot)]); + + let error = target.restore(snapshot).unwrap_err(); + assert!(matches!(error, HyperlightError::HyperlightVmError(_))); + assert_eq!(target.status(), SandboxStatus::Unrecoverable); + assert_eq!(target.vm.base_mapping_state(), mappings); + assert!(fault_plan.is_consumed()); + } + + #[test] + #[cfg(not(gdb))] + fn snapshot_restore_dynamic_unmapping_failure_is_recoverable() { + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + source.call::("AddToStatic", 42i32).unwrap(); + let snapshot = source.snapshot().unwrap(); + + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + let map_mem = allocate_guest_memory(); + let region = region_for_memory(&map_mem, 0x200000000_usize, MemoryRegionFlags::READ); + unsafe { target.map_region(®ion).unwrap() }; + let fault_plan = target + .vm + .inject_vm_faults([VmOperation::Unmap(MemoryRegionType::Heap)]); + + let error = target.restore(snapshot.clone()).unwrap_err(); + assert!(matches!(error, HyperlightError::HyperlightVmError(_))); + assert!(target.status().is_poisoned()); + assert_eq!(target.vm.get_mapped_regions().count(), 1); + assert!(fault_plan.is_consumed()); + + target.restore(snapshot).unwrap(); + assert_eq!(target.status(), SandboxStatus::Ready); + assert_eq!(target.vm.get_mapped_regions().count(), 0); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 42); + } + + #[test] + #[cfg(not(gdb))] + fn snapshot_restore_partial_dynamic_unmapping_failure_is_recoverable() { + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + source.call::("AddToStatic", 42i32).unwrap(); + let snapshot = source.snapshot().unwrap(); + + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + let first_mem = allocate_guest_memory(); + let first_region = + region_for_memory(&first_mem, 0x200000000_usize, MemoryRegionFlags::READ); + let second_mem = allocate_guest_memory(); + let mut second_region = + region_for_memory(&second_mem, 0x300000000_usize, MemoryRegionFlags::READ); + second_region.region_type = MemoryRegionType::MappedFile; + unsafe { + target.map_region(&first_region).unwrap(); + target.map_region(&second_region).unwrap(); + } + let fault_plan = target + .vm + .inject_vm_faults([VmOperation::Unmap(MemoryRegionType::MappedFile)]); + + let error = target.restore(snapshot.clone()).unwrap_err(); + assert!(matches!(error, HyperlightError::HyperlightVmError(_))); + assert!(target.status().is_poisoned()); + assert_eq!( + target.vm.get_mapped_regions().collect::>(), + vec![&second_region] + ); + assert!(fault_plan.is_consumed()); + + target.restore(snapshot).unwrap(); + assert_eq!(target.status(), SandboxStatus::Ready); + assert_eq!(target.vm.get_mapped_regions().count(), 0); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 42); + } + + #[test] + #[cfg(not(gdb))] + fn snapshot_restore_vcpu_reset_failure_is_recoverable() { + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + source.call::("AddToStatic", 42i32).unwrap(); + let snapshot = source.snapshot().unwrap(); + + #[cfg(target_arch = "x86_64")] + let reset_operations = [ + VmOperation::SetRegs, + VmOperation::SetDebugRegs, + VmOperation::ResetXsave, + VmOperation::SetSregs, + ]; + #[cfg(target_arch = "aarch64")] + let reset_operations = [VmOperation::ResetVcpu]; + + for reset_operation in reset_operations { + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + let fault_plan = target.vm.inject_vm_faults([reset_operation]); + + let error = target.restore(snapshot.clone()).unwrap_err(); + assert!(matches!(error, HyperlightError::HyperlightVmError(_))); + assert!(target.status().is_poisoned()); + assert!(fault_plan.is_consumed()); + assert_eq!( + target.vm.base_mapping_state(), + ( + Some(( + target.mem_mgr.shared_mem.base_addr(), + target.mem_mgr.shared_mem.mem_size(), + )), + Some(( + target.mem_mgr.scratch_mem.base_addr(), + target.mem_mgr.scratch_mem.mem_size(), + )), + ) + ); + + target.restore(snapshot.clone()).unwrap(); + assert_eq!(target.status(), SandboxStatus::Ready); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 42); + } + } + + #[test] + #[cfg(all(target_arch = "x86_64", not(gdb)))] + fn snapshot_restore_msr_failure_is_recoverable() { + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + source.call::("AddToStatic", 42i32).unwrap(); + let snapshot = source.snapshot().unwrap(); + + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + let fault_plan = target.vm.inject_vm_faults([VmOperation::SetMsrs]); + + let error = target.restore(snapshot.clone()).unwrap_err(); + assert!(matches!(error, HyperlightError::HyperlightVmError(_))); + assert!(target.status().is_poisoned()); + assert!(fault_plan.is_consumed()); + assert_eq!( + target.vm.base_mapping_state(), + ( + Some(( + target.mem_mgr.shared_mem.base_addr(), + target.mem_mgr.shared_mem.mem_size(), + )), + Some(( + target.mem_mgr.scratch_mem.base_addr(), + target.mem_mgr.scratch_mem.mem_size(), + )), + ) + ); + + target.restore(snapshot).unwrap(); + assert_eq!(target.status(), SandboxStatus::Ready); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 42); + } + #[test] fn snapshot_restore_rejects_incompatible_layout() { let mut sandbox = { @@ -1724,6 +2067,7 @@ mod tests { let path = simple_guest_as_pathbuf(); let mut cfg = SandboxConfiguration::default(); cfg.set_heap_size(0x20_000); + cfg.set_scratch_size(0x60_000); let u_sbox = UninitializedSandbox::new(GuestBinary::FilePath(path), Some(cfg)).unwrap(); u_sbox.evolve().unwrap() }; @@ -2194,7 +2538,7 @@ mod tests { let _ = sbox .call::<()>("guest_panic", "hello".to_string()) .unwrap_err(); - assert!(sbox.poisoned()); + assert!(sbox.status().is_poisoned()); // map_file_cow should fail with PoisonedSandbox let err = sbox.map_file_cow(&path, 0x1_0000_0000).unwrap_err(); @@ -2202,7 +2546,7 @@ mod tests { // Restore and verify map_file_cow works again sbox.restore(snapshot).unwrap(); - assert!(!sbox.poisoned()); + assert_eq!(sbox.status(), SandboxStatus::Ready); let result = sbox.map_file_cow(&path, 0x1_0000_0000); assert!(result.is_ok()); @@ -3053,7 +3397,7 @@ mod tests { .restore(snapshot.clone()) .expect_err("restore must reject an unrestorable snapshot MSR"); assert_snapshot_msr_index_invalid(&err); - assert!(target.poisoned()); + assert!(target.status().is_poisoned()); assert!(matches!( target.call::("Echo", "hi".to_string()), Err(HyperlightError::PoisonedSandbox) @@ -3114,16 +3458,16 @@ mod tests { matches!(result, Err(HyperlightError::GuestAborted(_, _))), "guest enabled x2APIC through APIC_BASE: {result:?}" ); - assert!(sandbox.poisoned()); + assert!(sandbox.status().is_poisoned()); sandbox.restore(snapshot).unwrap(); - assert!(!sandbox.poisoned()); + assert!(!sandbox.status().is_poisoned()); let result = sandbox.call::<()>("WriteMSR", (MSR_X2APIC_BASE, 1u64)); assert!( matches!(result, Err(HyperlightError::GuestAborted(_, _))), "x2APIC MSR access succeeded after restore: {result:?}" ); - assert!(sandbox.poisoned()); + assert!(sandbox.status().is_poisoned()); } #[test] @@ -3154,7 +3498,7 @@ mod tests { msr_index, result ); - assert!(sbox.poisoned()); + assert!(sbox.status().is_poisoned()); sbox.restore(snapshot.clone()).unwrap(); @@ -3165,7 +3509,7 @@ mod tests { msr_index, result ); - assert!(sbox.poisoned()); + assert!(sbox.status().is_poisoned()); } #[test] @@ -3231,7 +3575,7 @@ mod tests { matches!(result, Err(HyperlightError::GuestAborted(_, _))), "guest entered VMX operation via CR4.VMXE: {result:?}" ); - assert!(sandbox.poisoned()); + assert!(sandbox.status().is_poisoned()); } /// Executing a VM-enter (`VMLAUNCH`) in the guest faults. The guest is @@ -3252,7 +3596,7 @@ mod tests { matches!(result, Err(HyperlightError::GuestAborted(_, _))), "guest executed VMLAUNCH without faulting: {result:?}" ); - assert!(sandbox.poisoned()); + assert!(sandbox.status().is_poisoned()); } /// x2APIC is denied at the MSR level and Hyperlight keeps the APIC in @@ -3417,7 +3761,7 @@ mod tests { "WRMSR 0x{msr_index:X}: expected direct #GP, got: {result:?}" ); assert!( - sbox.poisoned(), + sbox.status().is_poisoned(), "sandbox should be poisoned after a denied WRMSR to 0x{msr_index:X}" ); } @@ -3546,7 +3890,10 @@ mod tests { let original: u64 = match sbox.call("ReadMSR", index) { Ok(value) => value, Err(_) => { - assert!(sbox.poisoned(), "0x{index:X}: fault did not poison sandbox"); + assert!( + sbox.status().is_poisoned(), + "0x{index:X}: fault did not poison sandbox" + ); sbox.restore(baseline).unwrap(); return; } @@ -3559,7 +3906,10 @@ mod tests { continue; } if sbox.call::<()>("WriteMSR", (index, candidate)).is_err() { - assert!(sbox.poisoned(), "0x{index:X}: fault did not poison sandbox"); + assert!( + sbox.status().is_poisoned(), + "0x{index:X}: fault did not poison sandbox" + ); sbox.restore(baseline.clone()).unwrap(); continue; } @@ -3692,7 +4042,7 @@ mod tests { Ok(v) => v, Err(_) => { assert!( - sbox.poisoned(), + sbox.status().is_poisoned(), "0x{msr:X}: a faulting RDMSR should poison the sandbox" ); sbox.restore(baseline).unwrap(); @@ -3706,7 +4056,7 @@ mod tests { if sbox.call::<()>("WriteMSR", (msr, sentinel)).is_err() { assert!( - sbox.poisoned(), + sbox.status().is_poisoned(), "0x{msr:X}: a faulting WRMSR should poison the sandbox" ); sbox.restore(baseline).unwrap(); diff --git a/src/hyperlight_host/src/sandbox/mod.rs b/src/hyperlight_host/src/sandbox/mod.rs index 822b1e388..2e0fe5923 100644 --- a/src/hyperlight_host/src/sandbox/mod.rs +++ b/src/hyperlight_host/src/sandbox/mod.rs @@ -46,7 +46,7 @@ pub use callable::Callable; /// Re-export for `SandboxConfiguration` type pub use config::SandboxConfiguration; /// Re-export for the `MultiUseSandbox` type -pub use initialized_multi_use::{MultiUseSandbox, PtRootFinder}; +pub use initialized_multi_use::{MultiUseSandbox, PtRootFinder, SandboxStatus}; /// Re-export for `GuestBinary` type pub use uninitialized::GuestBinary; /// Re-export for `UninitializedSandbox` type diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index 4e0604c86..9787debe6 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -363,7 +363,7 @@ fn disk_snapshot_non_superset_guest_msrs_rejected() { format!("{err:?}").contains("InvalidSnapshotMsrIndex"), "expected an MSR reset-set mismatch, got: {err:?}" ); - assert!(target.poisoned()); + assert!(target.status().is_poisoned()); } #[test] diff --git a/src/hyperlight_host/tests/integration_test.rs b/src/hyperlight_host/tests/integration_test.rs index 24db9134a..ecd964516 100644 --- a/src/hyperlight_host/tests/integration_test.rs +++ b/src/hyperlight_host/tests/integration_test.rs @@ -21,7 +21,7 @@ use std::time::Duration; use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode; use hyperlight_common::log_level::GuestLogFilter; use hyperlight_host::sandbox::SandboxConfiguration; -use hyperlight_host::{HyperlightError, MultiUseSandbox}; +use hyperlight_host::{HyperlightError, MultiUseSandbox, SandboxStatus}; use hyperlight_testing::simplelogger::{LOGGER, SimpleLogger}; use serial_test::serial; use tracing_core::LevelFilter; @@ -65,11 +65,11 @@ fn interrupt_host_call() { matches!(&result, HyperlightError::ExecutionCanceledByHost()), "unexpected error: {result:?}" ); - assert!(sandbox.poisoned()); + assert!(sandbox.status().is_poisoned()); // Restore from snapshot to clear poison sandbox.restore(snapshot.clone()).unwrap(); - assert!(!sandbox.poisoned()); + assert_eq!(sandbox.status(), SandboxStatus::Ready); thread.join().unwrap(); }); @@ -99,11 +99,11 @@ fn interrupt_in_progress_guest_call() { matches!(&res, HyperlightError::ExecutionCanceledByHost()), "unexpected error: {res:?}" ); - assert!(sbox1.poisoned()); + assert!(sbox1.status().is_poisoned()); // Restore from snapshot to clear poison sbox1.restore(snapshot.clone()).unwrap(); - assert!(!sbox1.poisoned()); + assert_eq!(sbox1.status(), SandboxStatus::Ready); barrier.wait(); // Make sure we can still call guest functions after the VM was interrupted @@ -196,7 +196,7 @@ fn interrupt_same_thread() { Ok(_) | Err(HyperlightError::ExecutionCanceledByHost()) => {} _ => panic!("Unexpected return"), }; - if sbox2.poisoned() { + if sbox2.status().is_poisoned() { sbox2.restore(snapshot2.clone()).unwrap(); } sbox3 @@ -243,7 +243,7 @@ fn interrupt_same_thread_no_barrier() { Ok(_) | Err(HyperlightError::ExecutionCanceledByHost()) => {} other => panic!("Unexpected return: {:?}", other), }; - if sbox2.poisoned() { + if sbox2.status().is_poisoned() { sbox2.restore(snapshot2.clone()).unwrap(); } sbox3 @@ -275,9 +275,9 @@ fn interrupt_moved_sandbox() { matches!(&res, HyperlightError::ExecutionCanceledByHost()), "unexpected error: {res:?}" ); - assert!(sbox1.poisoned()); + assert!(sbox1.status().is_poisoned()); sbox1.restore(snapshot1.clone()).unwrap(); - assert!(!sbox1.poisoned()); + assert_eq!(sbox1.status(), SandboxStatus::Ready); }); let thread2 = thread::spawn(move || { @@ -333,11 +333,11 @@ fn interrupt_custom_signal_no_and_retry_delay() { matches!(&res, HyperlightError::ExecutionCanceledByHost()), "unexpected error: {res:?}" ); - assert!(sbox1.poisoned()); + assert!(sbox1.status().is_poisoned()); // immediately reenter another guest function call after having being cancelled, // so that the vcpu is running again before the interruptor-thread has a chance to see that the vcpu is not running sbox1.restore(snapshot1.clone()).unwrap(); - assert!(!sbox1.poisoned()); + assert_eq!(sbox1.status(), SandboxStatus::Ready); } thread.join().expect("Thread should finish"); }); @@ -572,7 +572,7 @@ fn guest_outb_with_invalid_port_poisons_sandbox() { // The sandbox should be poisoned because the guest didn't complete normally assert!( - sbox.poisoned(), + sbox.status().is_poisoned(), "Sandbox should be poisoned after invalid OUT" ); }); @@ -1143,7 +1143,7 @@ fn interrupt_random_kill_stress_test() { let sandbox_wrapper = guard.sandbox_with_snapshot.as_mut().unwrap(); // Make sure the sandbox is poisoned - assert!(sandbox_wrapper.sandbox.poisoned()); + assert!(sandbox_wrapper.sandbox.status().is_poisoned()); // Try to restore the snapshot if let Err(e) = sandbox_wrapper diff --git a/src/hyperlight_host/tests/sandbox_host_tests.rs b/src/hyperlight_host/tests/sandbox_host_tests.rs index b1a1a9918..6c17e94f9 100644 --- a/src/hyperlight_host/tests/sandbox_host_tests.rs +++ b/src/hyperlight_host/tests/sandbox_host_tests.rs @@ -366,7 +366,7 @@ fn host_function_error() { res ); // C guest panics in rust guest lib when host function returns error, which will poison the sandbox - if init_sandbox.poisoned() { + if init_sandbox.status().is_poisoned() { init_sandbox.restore(snapshot.clone()).unwrap(); } }