diff --git a/CHANGELOG.md b/CHANGELOG.md index 0546852eb..fd1dc5103 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 cannot recover 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. diff --git a/src/hyperlight_host/src/error.rs b/src/hyperlight_host/src/error.rs index c6738374d..cbd91c0a5 100644 --- a/src/hyperlight_host/src/error.rs +++ b/src/hyperlight_host/src/error.rs @@ -224,6 +224,22 @@ pub enum HyperlightError { #[error("The sandbox was poisoned")] PoisonedSandbox, + /// Snapshot restore could not establish a recoverable VM state. + #[error( + "Snapshot restore failed and the sandbox must be discarded. Mapping update failed: {update}. Mapping recovery failed: {recovery}" + )] + RestoreFailedUnrecoverably { + /// The mapping update failure. + #[source] + update: Box, + /// The failure encountered while recovering the prior mapping. + recovery: Box, + }, + + /// 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), @@ -401,6 +417,7 @@ impl HyperlightError { | HyperlightError::RefCellBorrowFailed(_) | HyperlightError::RefCellMutBorrowFailed(_) | HyperlightError::ReturnValueConversionFailure(_, _) + | HyperlightError::RestoreFailedUnrecoverably { .. } | HyperlightError::SnapshotLayoutMismatch | HyperlightError::SnapshotHostFunctionMismatch { .. } | HyperlightError::SystemTimeError(_) @@ -408,6 +425,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 d5411d80e..b26cbfce2 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; @@ -45,7 +47,7 @@ use crate::hypervisor::virtual_machine::{ }; use crate::hypervisor::{InterruptHandle, InterruptHandleImpl}; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags, MemoryRegionType}; -use crate::mem::mgr::{SandboxMemoryManager, SnapshotSharedMemory}; +use crate::mem::mgr::{BaseMappingUpdate, SandboxMemoryManager, SnapshotSharedMemory}; use crate::mem::shared_mem::{GuestSharedMemory, HostSharedMemory, SharedMemory}; use crate::metrics::{METRIC_ERRONEOUS_VCPU_KICKS, METRIC_GUEST_CANCELLATION}; use crate::sandbox::host_funcs::FunctionRegistry; @@ -270,6 +272,32 @@ pub enum UpdateRegionError { UnmapMemory(#[from] UnmapMemoryError), } +#[derive(Debug, thiserror::Error)] +pub(crate) enum BaseMappingUpdateError { + #[error("{0}")] + Recoverable(#[source] UpdateRegionError), + #[error("Mapping update failed: {update}. Restoring the prior mapping failed: {recovery}")] + Unrecoverable { + update: UpdateRegionError, + recovery: UpdateRegionError, + }, +} + +impl From for BaseMappingUpdateError { + fn from(error: UpdateRegionError) -> Self { + Self::Recoverable(error) + } +} + +impl BaseMappingUpdateError { + fn into_update_error(self) -> UpdateRegionError { + match self { + Self::Recoverable(error) => error, + Self::Unrecoverable { recovery, .. } => recovery, + } + } +} + /// Errors that can occur when accessing the root page table state #[derive(Debug, thiserror::Error)] pub enum AccessPageTableError { @@ -528,17 +556,43 @@ impl HyperlightVm { pub(crate) fn update_snapshot_mapping( &mut self, snapshot: SnapshotSharedMemory, - ) -> Result<(), UpdateRegionError> { + ) -> Result>, UpdateRegionError> { + self.update_snapshot_mapping_transactionally(snapshot) + .map_err(BaseMappingUpdateError::into_update_error) + } + + fn update_snapshot_mapping_transactionally( + &mut self, + snapshot: SnapshotSharedMemory, + ) -> Result>, BaseMappingUpdateError> { 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))?; + self.vm + .unmap_memory((self.snapshot_slot, &old_rgn)) + .map_err(UpdateRegionError::from)?; + } + // SAFETY: `snapshot` owns the mapped region and is stored in `self` on success. + let map_result = unsafe { self.vm.map_memory((self.snapshot_slot, &rgn)) }; + if let Err(err) = map_result { + let update = UpdateRegionError::from(err); + if let Some(old_snapshot) = self.snapshot_memory.as_ref() { + let old_rgn = old_snapshot.mapping_at(guest_base, MemoryRegionType::Snapshot); + // SAFETY: `old_snapshot` remains owned by `self` throughout rollback. + let recovery_result = unsafe { self.vm.map_memory((self.snapshot_slot, &old_rgn)) }; + if let Err(err) = recovery_result { + return Err(BaseMappingUpdateError::Unrecoverable { + update, + recovery: err.into(), + }); + } + } + return Err(BaseMappingUpdateError::Recoverable(update)); } - unsafe { self.vm.map_memory((self.snapshot_slot, &rgn))? }; - Ok(()) + Ok(self.snapshot_memory.replace(snapshot)) } /// Update the scratch mapping to point to a new GuestSharedMemory @@ -546,16 +600,93 @@ impl HyperlightVm { &mut self, scratch: GuestSharedMemory, ) -> Result<(), UpdateRegionError> { + self.update_scratch_mapping_transactionally(scratch) + .map_err(BaseMappingUpdateError::into_update_error) + } + + fn update_scratch_mapping_transactionally( + &mut self, + scratch: GuestSharedMemory, + ) -> Result<(), BaseMappingUpdateError> { 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))?; + self.vm + .unmap_memory((self.scratch_slot, &old_rgn)) + .map_err(UpdateRegionError::from)?; + } + // SAFETY: `scratch` owns the mapped region and is stored in `self` on success. + let map_result = unsafe { self.vm.map_memory((self.scratch_slot, &rgn)) }; + if let Err(err) = map_result { + let update = UpdateRegionError::from(err); + 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); + // SAFETY: `old_scratch` remains owned by `self` throughout rollback. + let recovery_result = unsafe { self.vm.map_memory((self.scratch_slot, &old_rgn)) }; + if let Err(err) = recovery_result { + return Err(BaseMappingUpdateError::Unrecoverable { + update, + recovery: err.into(), + }); + } + } + return Err(BaseMappingUpdateError::Recoverable(update)); } - unsafe { self.vm.map_memory((self.scratch_slot, &rgn))? }; + self.scratch_memory = Some(scratch); + + Ok(()) + } + pub(crate) fn update_base_mappings( + &mut self, + update: BaseMappingUpdate, + ) -> Result<(), BaseMappingUpdateError> { + match update { + BaseMappingUpdate::Keep => Ok(()), + BaseMappingUpdate::ReplaceSnapshot(snapshot) => self + .update_snapshot_mapping_transactionally(snapshot) + .map(|_| ()), + BaseMappingUpdate::ReplaceAll { snapshot, scratch } => { + self.replace_base_mappings(snapshot, scratch) + } + } + } + + fn replace_base_mappings( + &mut self, + snapshot: SnapshotSharedMemory, + scratch: GuestSharedMemory, + ) -> Result<(), BaseMappingUpdateError> { + let old_snapshot = self.update_snapshot_mapping_transactionally(snapshot)?; + if let Err(error) = self.update_scratch_mapping_transactionally(scratch) { + let update = match error { + BaseMappingUpdateError::Recoverable(update) => update, + error @ BaseMappingUpdateError::Unrecoverable { .. } => return Err(error), + }; + if let Some(old_snapshot) = old_snapshot + && let Err(error) = self.update_snapshot_mapping_transactionally(old_snapshot) + { + return Err(BaseMappingUpdateError::Unrecoverable { + update, + recovery: error.into_update_error(), + }); + } + return Err(BaseMappingUpdateError::Recoverable(update)); + } + Ok(()) + } + + #[cfg(gdb)] + pub(crate) fn clear_guest_debug_state(&mut self) -> Result<(), DebugError> { + self.sw_breakpoints.clear(); + if let Some(entry_addr) = self.one_shot_entry_bp { + self.vm.remove_hw_breakpoint(entry_addr)?; + self.one_shot_entry_bp = None; + } 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..d22c9e6e4 --- /dev/null +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/test_support.rs @@ -0,0 +1,281 @@ +/* +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::SandboxTraceContext, + ) -> 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(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 + } + + 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/hypervisor/virtual_machine/kvm/x86_64.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs index 9a2cc254f..0a48abfca 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs @@ -738,15 +738,18 @@ impl DebuggableVm for KvmVm { .position(|&a| a == addr) .ok_or(DebugError::HwBreakpointNotFound(addr))?; + let mut updated_debug_regs = self.debug_regs; + // Clear the address - self.debug_regs.arch.debugreg[index] = 0; + updated_debug_regs.arch.debugreg[index] = 0; // Disable LOCAL bit - self.debug_regs.arch.debugreg[7] &= !(1 << (index * 2)); + updated_debug_regs.arch.debugreg[7] &= !(1 << (index * 2)); self.vcpu_fd - .set_guest_debug(&self.debug_regs) + .set_guest_debug(&updated_debug_regs) .map_err(|e| RegisterError::SetDebugRegs(e.into()))?; + self.debug_regs = updated_debug_regs; Ok(()) } } 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/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index c93f1cac1..d4272d827 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -151,12 +151,57 @@ pub(crate) struct SandboxMemoryManager { pub(crate) abort_buffer: Vec, /// Generation counter: how many snapshots have been taken from /// this sandbox's execution path from init to here. Incremented - /// on each `snapshot` call; on `restore_snapshot` we inherit the + /// on each `snapshot` call; on restore we inherit the /// restored snapshot's own generation number so the guest-visible /// counter tracks which snapshot the sandbox is a clone of. pub(crate) snapshot_count: u64, } +pub(crate) enum BaseMappingUpdate { + /// Reuse both mappings. The target already maps this read-only snapshot, + /// and its scratch mapping has the required size. + Keep, + /// Replace only the snapshot mapping. The scratch mapping has the required size, + /// but the target maps another snapshot. Writable snapshots also use this state + /// because each restore creates a new writable copy. + ReplaceSnapshot(SnapshotSharedMemory), + /// Replace both mappings because the current scratch mapping has the wrong size. + /// Share the read-only snapshot memory or create a writable copy, as configured. + ReplaceAll { + snapshot: SnapshotSharedMemory, + scratch: GuestSharedMemory, + }, +} + +pub(crate) struct PreparedMemoryRestore { + manager: SandboxMemoryManager, + mapping_update: BaseMappingUpdate, +} + +impl PreparedMemoryRestore { + pub(crate) fn reset_reused_scratch(&mut self) -> Result<()> { + if !matches!(self.mapping_update, BaseMappingUpdate::ReplaceAll { .. }) { + self.manager.scratch_mem.zero()?; + self.manager.update_scratch_bookkeeping()?; + } + Ok(()) + } + + pub(crate) fn into_parts(self) -> (SandboxMemoryManager, BaseMappingUpdate) { + (self.manager, self.mapping_update) + } + + #[cfg(all(test, not(unshared_snapshot_mem)))] + pub(crate) fn keeps_mappings(&self) -> bool { + matches!(self.mapping_update, BaseMappingUpdate::Keep) + } + + #[cfg(test)] + pub(crate) fn replaces_snapshot(&self) -> bool { + matches!(self.mapping_update, BaseMappingUpdate::ReplaceSnapshot(_)) + } +} + /// Buffer for building guest page tables during snapshot creation. /// `TableAddr` is an absolute GPA (u64) so the same address space is /// used regardless of entry size. @@ -338,10 +383,8 @@ impl SandboxMemoryManager { let next_action = s.next_action(); let mut mgr = Self::new(layout, shared_mem, scratch_mem, next_action); mgr.original_entrypoint = s.original_entrypoint(); - // Inherit the snapshot's generation number for the same - // reason `restore_snapshot` does: the guest-visible counter - // reflects "which snapshot is the sandbox currently a clone - // of", not "how many snapshots this partition has taken". + // The guest-visible counter identifies the snapshot generation + // that this sandbox is a clone of. mgr.snapshot_count = s.snapshot_generation(); Ok(mgr) } @@ -388,6 +431,41 @@ impl SandboxMemoryManager { } impl SandboxMemoryManager { + pub(crate) fn prepare_restore(&self, snapshot: &Snapshot) -> Result { + let mut candidate = self.clone(); + let mapping_update = if snapshot.layout().get_scratch_size() != self.scratch_mem.mem_size() + { + let (snapshot_host, snapshot_guest) = snapshot.memory().to_mgr_snapshot_mem()?.build(); + let (scratch_host, scratch_guest) = + ExclusiveSharedMemory::new(snapshot.layout().get_scratch_size())?.build(); + candidate.shared_mem = snapshot_host; + candidate.scratch_mem = scratch_host; + BaseMappingUpdate::ReplaceAll { + snapshot: snapshot_guest, + scratch: scratch_guest, + } + } else if *snapshot.memory() == self.shared_mem { + BaseMappingUpdate::Keep + } else { + let (host, guest) = snapshot.memory().to_mgr_snapshot_mem()?.build(); + candidate.shared_mem = host; + BaseMappingUpdate::ReplaceSnapshot(guest) + }; + + candidate.layout = *snapshot.layout(); + candidate.next_action = snapshot.next_action(); + candidate.snapshot_count = snapshot.snapshot_generation(); + candidate.original_entrypoint = snapshot.original_entrypoint(); + if matches!(mapping_update, BaseMappingUpdate::ReplaceAll { .. }) { + candidate.update_scratch_bookkeeping()?; + } + + Ok(PreparedMemoryRestore { + manager: candidate, + mapping_update, + }) + } + /// Reads a host function call from memory #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn get_host_function_call(&mut self) -> Result { @@ -471,59 +549,6 @@ impl SandboxMemoryManager { } } - /// This function restores a memory snapshot from a given snapshot. - pub(crate) fn restore_snapshot( - &mut self, - snapshot: &Snapshot, - ) -> Result<( - Option>, - Option, - )> { - let gsnapshot = if *snapshot.memory() == self.shared_mem { - // If the snapshot memory is already the correct memory, - // which is readonly, don't bother with restoring it, - // since its contents must be the same. Note that in the - // #[cfg(unshared_snapshot_mem)] case, this condition will - // never be true, since even immediately after a restore, - // self.shared_mem is a (writable) copy, not the original - // shared_mem. - None - } else { - let new_snapshot_mem = snapshot.memory().to_mgr_snapshot_mem()?; - let (hsnapshot, gsnapshot) = new_snapshot_mem.build(); - self.shared_mem = hsnapshot; - Some(gsnapshot) - }; - let new_scratch_size = snapshot.layout().get_scratch_size(); - let gscratch = if new_scratch_size == self.scratch_mem.mem_size() { - self.scratch_mem.zero()?; - None - } else { - let new_scratch_mem = ExclusiveSharedMemory::new(new_scratch_size)?; - let (hscratch, gscratch) = new_scratch_mem.build(); - // Even though this destroys the reference to the host - // side of the old scratch mapping, the VM should still - // own the reference to the guest side of the old scratch - // mapping, so it won't actually be deallocated until it - // has been unmapped from the VM. - self.scratch_mem = hscratch; - - Some(gscratch) - }; - self.layout = *snapshot.layout(); - // Inherit the snapshot's own generation number — the - // guest-visible counter reflects "which snapshot is the - // sandbox currently a clone of", not "how many restores have - // happened into this (possibly-reused) partition". - self.snapshot_count = snapshot.snapshot_generation(); - // Carry the guest ELF entry point across restore so crashdumps - // report the restored image's entry. - self.original_entrypoint = snapshot.original_entrypoint(); - - self.update_scratch_bookkeeping()?; - Ok((gsnapshot, gscratch)) - } - #[inline] fn update_scratch_bookkeeping_item(&mut self, offset: u64, value: u64) -> Result<()> { let scratch_size = self.scratch_mem.mem_size(); diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 623c24667..b73c5cf95 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -33,7 +33,7 @@ use super::host_funcs::FunctionRegistry; use super::snapshot::Snapshot; use crate::func::{ParameterTuple, SupportedReturnType}; use crate::hypervisor::InterruptHandle; -use crate::hypervisor::hyperlight_vm::{HyperlightVm, HyperlightVmError}; +use crate::hypervisor::hyperlight_vm::{BaseMappingUpdateError, HyperlightVm, HyperlightVmError}; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::{HostSharedMemory, SharedMemory as _}; @@ -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, @@ -110,6 +139,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 @@ -123,7 +166,7 @@ impl MultiUseSandbox { #[cfg(gdb)] dbg_mem_access_fn: Arc>>, ) -> MultiUseSandbox { Self { - poisoned: false, + status: SandboxStatus::Ready, host_funcs, mem_mgr: mgr, vm, @@ -371,9 +414,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()); @@ -450,6 +491,10 @@ impl MultiUseSandbox { /// declare every MSR the snapshot saved, or the restore poisons with an MSR /// mismatch. /// + /// A restore that cannot recover a usable VM mapping state returns + /// [`RestoreFailedUnrecoverably`](crate::HyperlightError::RestoreFailedUnrecoverably). + /// The sandbox then rejects further operations and must be discarded. + /// /// ## Poison State Recovery /// /// This method automatically clears any poison state when successful. This is safe because: @@ -507,10 +552,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())?; @@ -521,6 +566,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 @@ -552,34 +601,78 @@ 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. - self.vm - .reset_vcpu(snapshot.root_pt_gpa(), sregs) - .map_err(|e| { - self.poisoned = true; - HyperlightVmError::Restore(e) - })?; + + let mut prepared_restore = self.mem_mgr.prepare_restore(&snapshot)?; + + #[cfg(gdb)] + let dbg_mem_access_fn = self.dbg_mem_access_fn.clone(); + #[cfg(gdb)] + let mut dbg_mem_access_fn = dbg_mem_access_fn + .lock() + .map_err(|err| crate::new_error!("failed to lock debug memory manager: {err}"))?; + + let restore_result = (|| -> Result<()> { + #[cfg(gdb)] + self.vm + .clear_guest_debug_state() + .map_err(|err| crate::new_error!("failed to clear guest debug state: {err}"))?; + + let current_regions: Vec = + self.vm.get_mapped_regions().cloned().collect(); + for region in ¤t_regions { + self.vm + .unmap_region(region) + .map_err(HyperlightVmError::UnmapRegion)?; + } + + prepared_restore.reset_reused_scratch()?; + let (candidate_mem_mgr, mapping_update) = prepared_restore.into_parts(); + if let Err(error) = self.vm.update_base_mappings(mapping_update) { + return match error { + BaseMappingUpdateError::Recoverable(error) => { + Err(HyperlightVmError::UpdateRegion(error).into()) + } + BaseMappingUpdateError::Unrecoverable { update, recovery } => { + Err(HyperlightError::RestoreFailedUnrecoverably { + update: Box::new(HyperlightVmError::UpdateRegion(update).into()), + recovery: Box::new(HyperlightVmError::UpdateRegion(recovery).into()), + }) + } + }; + } + + #[cfg(gdb)] + { + *dbg_mem_access_fn = candidate_mem_mgr.clone(); + } + self.mem_mgr = candidate_mem_mgr; + #[cfg(gdb)] + drop(dbg_mem_access_fn); + + self.vm + .reset_vcpu(snapshot.root_pt_gpa(), sregs) + .map_err(HyperlightVmError::Restore)?; + + Ok(()) + })(); + + if let Err(err) = restore_result { + self.status = if matches!(err, HyperlightError::RestoreFailedUnrecoverably { .. }) { + SandboxStatus::Unrecoverable + } else { + SandboxStatus::Poisoned + }; + self.snapshot = None; + return Err(err); + } // Restore captured MSR state. #[cfg(target_arch = "x86_64")] self.vm.restore_msrs(snapshot.msrs()).map_err(|e| { - self.poisoned = true; + self.status = SandboxStatus::Poisoned; HyperlightVmError::Restore(e) })?; @@ -591,13 +684,6 @@ impl MultiUseSandbox { self.vm .set_crashdump_entry_point(snapshot.original_entrypoint()); - let current_regions: Vec = self.vm.get_mapped_regions().cloned().collect(); - for region in ¤t_regions { - self.vm - .unmap_region(region) - .map_err(HyperlightVmError::UnmapRegion)?; - } - // The restored snapshot is now our most current snapshot self.snapshot = Some(snapshot.clone()); @@ -610,7 +696,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(()) } @@ -662,9 +748,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)?; @@ -685,7 +769,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 @@ -739,7 +823,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())?; /// } @@ -753,9 +837,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, || { @@ -788,9 +870,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 @@ -814,9 +894,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)?; @@ -882,9 +960,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, || { @@ -898,9 +974,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. @@ -932,7 +1006,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); } @@ -967,7 +1043,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 @@ -1065,10 +1143,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 /// @@ -1087,22 +1164,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 } } @@ -1112,9 +1194,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) } } @@ -1176,10 +1256,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::{HyperlightVmError, 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() { @@ -1198,7 +1297,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 @@ -1208,7 +1307,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"); @@ -1240,12 +1339,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 @@ -1254,16 +1353,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(); @@ -1738,6 +1837,533 @@ 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_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::Map(MemoryRegionType::Snapshot)]); + + let error = target.restore(snapshot.clone()).unwrap_err(); + assert!(matches!(error, HyperlightError::HyperlightVmError(_))); + assert!(target.status().is_poisoned()); + assert!(fault_plan.is_consumed()); + + target.restore(snapshot).unwrap(); + assert_eq!(target.status(), SandboxStatus::Ready); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 42); + } + + #[test] + #[cfg(not(gdb))] + fn snapshot_restore_mapping_recovery_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 fault_plan = target.vm.inject_vm_faults([ + VmOperation::Map(MemoryRegionType::Snapshot), + VmOperation::Map(MemoryRegionType::Snapshot), + ]); + + let error = target.restore(snapshot.clone()).unwrap_err(); + assert!(matches!( + error, + HyperlightError::RestoreFailedUnrecoverably { .. } + )); + assert_eq!(target.status(), SandboxStatus::Unrecoverable); + assert!(!target.status().is_poisoned()); + 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_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 old_mappings = target.vm.base_mapping_state(); + let fault_plan = target + .vm + .inject_vm_faults([VmOperation::Unmap(MemoryRegionType::Snapshot)]); + + let error = target.restore(snapshot.clone()).unwrap_err(); + assert!(matches!(error, HyperlightError::HyperlightVmError(_))); + assert!(target.status().is_poisoned()); + assert_eq!(target.vm.base_mapping_state(), old_mappings); + assert!(fault_plan.is_consumed()); + + target.restore(snapshot).unwrap(); + assert_eq!(target.status(), SandboxStatus::Ready); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 42); + } + + #[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] + #[cfg(not(gdb))] + fn replace_all_mapping_transaction_covers_success_and_failures() { + #[derive(Clone, Copy)] + enum Expected { + Success, + Recoverable, + Unrecoverable, + } + + struct Case { + name: &'static str, + operations: &'static [VmOperation], + expected: Expected, + } + + let cases = [ + Case { + name: "success", + operations: &[], + expected: Expected::Success, + }, + Case { + name: "snapshot unmap failure", + operations: &[VmOperation::Unmap(MemoryRegionType::Snapshot)], + expected: Expected::Recoverable, + }, + Case { + name: "snapshot map failure", + operations: &[VmOperation::Map(MemoryRegionType::Snapshot)], + expected: Expected::Recoverable, + }, + Case { + name: "snapshot recovery failure", + operations: &[ + VmOperation::Map(MemoryRegionType::Snapshot), + VmOperation::Map(MemoryRegionType::Snapshot), + ], + expected: Expected::Unrecoverable, + }, + Case { + name: "scratch unmap failure", + operations: &[VmOperation::Unmap(MemoryRegionType::Scratch)], + expected: Expected::Recoverable, + }, + Case { + name: "scratch map failure", + operations: &[VmOperation::Map(MemoryRegionType::Scratch)], + expected: Expected::Recoverable, + }, + Case { + name: "scratch rollback failure", + operations: &[ + VmOperation::Map(MemoryRegionType::Scratch), + VmOperation::Map(MemoryRegionType::Scratch), + ], + expected: Expected::Unrecoverable, + }, + Case { + name: "snapshot rollback unmap failure", + operations: &[ + VmOperation::Unmap(MemoryRegionType::Scratch), + VmOperation::Unmap(MemoryRegionType::Snapshot), + ], + expected: Expected::Unrecoverable, + }, + Case { + name: "snapshot rollback map failure", + operations: &[ + VmOperation::Unmap(MemoryRegionType::Scratch), + VmOperation::Map(MemoryRegionType::Snapshot), + ], + expected: Expected::Unrecoverable, + }, + Case { + name: "snapshot rollback recovery failure", + operations: &[ + VmOperation::Unmap(MemoryRegionType::Scratch), + VmOperation::Map(MemoryRegionType::Snapshot), + VmOperation::Map(MemoryRegionType::Snapshot), + ], + expected: Expected::Unrecoverable, + }, + ]; + + let path = simple_guest_as_pathbuf(); + let mut source_config = SandboxConfiguration::default(); + source_config.set_scratch_size(0x100000); + let mut source = + UninitializedSandbox::new(GuestBinary::FilePath(path), Some(source_config)) + .unwrap() + .evolve() + .unwrap(); + source.call::("AddToStatic", 42i32).unwrap(); + let snapshot = source.snapshot().unwrap(); + + for case in cases { + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + target.mem_mgr.layout = *snapshot.layout(); + let old_mappings = target.vm.base_mapping_state(); + let old_manager_mappings = ( + 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(), + )), + ); + assert_eq!(old_mappings, old_manager_mappings); + let fault_plan = target.vm.inject_vm_faults(case.operations.iter().copied()); + + let result = target.restore(snapshot.clone()); + match (case.expected, result) { + (Expected::Success, Ok(())) => { + 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(), + )), + ), + "{}", + case.name + ); + assert_eq!(target.status(), SandboxStatus::Ready, "{}", case.name); + assert_eq!( + target.call::("GetStatic", ()).unwrap(), + 42, + "{}", + case.name + ); + } + ( + Expected::Recoverable, + Err(HyperlightError::HyperlightVmError(HyperlightVmError::UpdateRegion(_))), + ) => { + assert_eq!( + target.vm.base_mapping_state(), + old_mappings, + "{}", + case.name + ); + assert_eq!( + ( + 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(), + )), + ), + old_manager_mappings, + "{}", + case.name + ); + assert!(target.status().is_poisoned(), "{}", case.name); + target.restore(snapshot.clone()).unwrap(); + assert_eq!(target.status(), SandboxStatus::Ready, "{}", case.name); + assert_eq!( + target.call::("GetStatic", ()).unwrap(), + 42, + "{}", + case.name + ); + } + ( + Expected::Unrecoverable, + Err(HyperlightError::RestoreFailedUnrecoverably { update, recovery }), + ) => { + assert!( + matches!( + update.as_ref(), + HyperlightError::HyperlightVmError(HyperlightVmError::UpdateRegion(_)) + ), + "{}", + case.name + ); + assert!( + matches!( + recovery.as_ref(), + HyperlightError::HyperlightVmError(HyperlightVmError::UpdateRegion(_)) + ), + "{}", + case.name + ); + assert_eq!( + target.status(), + SandboxStatus::Unrecoverable, + "{}", + case.name + ); + assert!(matches!( + target.restore(snapshot.clone()), + Err(HyperlightError::UnrecoverableSandbox) + )); + } + (_, result) => panic!("{} returned {result:?}", case.name), + } + assert!(fault_plan.is_consumed(), "{}", case.name); + } + } + #[test] fn snapshot_restore_rejects_incompatible_layout() { let mut sandbox = { @@ -1752,6 +2378,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() }; @@ -1908,6 +2535,86 @@ mod tests { assert_eq!(target.call::("GetStatic", ()).unwrap(), 7); } + #[test] + fn prepare_restore_replaces_snapshot_mapping_for_different_snapshot() { + let mut source = { + let path = simple_guest_as_pathbuf(); + UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap() + }; + let target = { + let path = simple_guest_as_pathbuf(); + UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap() + }; + + let snapshot = source.snapshot().unwrap(); + + assert!( + target + .mem_mgr + .prepare_restore(&snapshot) + .unwrap() + .replaces_snapshot() + ); + } + + #[cfg(not(unshared_snapshot_mem))] + #[test] + fn prepare_restore_keeps_mappings_for_current_snapshot() { + let mut sandbox = { + let path = simple_guest_as_pathbuf(); + UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap() + }; + + let snapshot = sandbox.snapshot().unwrap(); + let (candidate, _) = sandbox + .mem_mgr + .prepare_restore(&snapshot) + .unwrap() + .into_parts(); + + assert!( + candidate + .prepare_restore(&snapshot) + .unwrap() + .keeps_mappings() + ); + } + + #[cfg(unshared_snapshot_mem)] + #[test] + fn prepare_restore_replaces_writable_mapping_for_current_snapshot() { + let mut sandbox = { + let path = simple_guest_as_pathbuf(); + UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap() + }; + + let snapshot = sandbox.snapshot().unwrap(); + let (candidate, _) = sandbox + .mem_mgr + .prepare_restore(&snapshot) + .unwrap() + .into_parts(); + + assert!( + candidate + .prepare_restore(&snapshot) + .unwrap() + .replaces_snapshot() + ); + } + /// Test that snapshot restore properly resets vCPU debug registers. This test verifies /// that restore() calls reset_vcpu(). #[test] @@ -2198,7 +2905,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(); @@ -2206,7 +2913,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()); @@ -3057,7 +3764,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) @@ -3118,16 +3825,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] @@ -3158,7 +3865,7 @@ mod tests { msr_index, result ); - assert!(sbox.poisoned()); + assert!(sbox.status().is_poisoned()); sbox.restore(snapshot.clone()).unwrap(); @@ -3169,7 +3876,7 @@ mod tests { msr_index, result ); - assert!(sbox.poisoned()); + assert!(sbox.status().is_poisoned()); } #[test] @@ -3235,7 +3942,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 @@ -3256,7 +3963,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 @@ -3421,7 +4128,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}" ); } @@ -3550,7 +4257,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; } @@ -3563,7 +4273,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; } @@ -3696,7 +4409,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(); @@ -3710,7 +4423,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/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index e1578e2e7..54dd3c993 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -828,15 +828,23 @@ mod tests { ) .unwrap(); - // Restore snapshot A - mgr.restore_snapshot(&snapshot_a).unwrap(); - mgr.shared_mem + // Build a manager from snapshot A + let (mut mgr_a, _) = SandboxMemoryManager::from_snapshot(&snapshot_a) + .unwrap() + .build() + .unwrap(); + mgr_a + .shared_mem .with_contents(|contents| assert_eq!(&contents[0..pattern_a.len()], &pattern_a[..])) .unwrap(); - // Restore snapshot B - mgr.restore_snapshot(&snapshot_b).unwrap(); - mgr.shared_mem + // Build a manager from snapshot B + let (mut mgr_b, _) = SandboxMemoryManager::from_snapshot(&snapshot_b) + .unwrap() + .build() + .unwrap(); + mgr_b + .shared_mem .with_contents(|contents| assert_eq!(&contents[0..pattern_b.len()], &pattern_b[..])) .unwrap(); } diff --git a/src/hyperlight_host/tests/integration_test.rs b/src/hyperlight_host/tests/integration_test.rs index b449ea68d..20a04cc04 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(); } }