diff --git a/CHANGELOG.md b/CHANGELOG.md index 56a89482b..39985b9c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). 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()`. +* `MultiUseSandbox::restore` has been made more flexible and now accepts snapshots from any guest binary or memory layout when host functions are compatible. Certain fixed guest addresses were changed on AArch64 to more easily accommodate 16k pages without wasting memory. Snapshots taken from diff --git a/src/hyperlight_host/src/error.rs b/src/hyperlight_host/src/error.rs index 4a1bc26b3..fb6dcb395 100644 --- a/src/hyperlight_host/src/error.rs +++ b/src/hyperlight_host/src/error.rs @@ -211,12 +211,6 @@ pub enum HyperlightError { /// Error creating or operating on memory shared with the guest #[error("Failed to execute shared memory operation: {0}")] SharedMemory(#[from] crate::mem::shared_mem::SharedMemoryError), - - /// Tried to restore a snapshot into a sandbox whose memory - /// layout is not compatible with the snapshot's. - #[error("Snapshot memory layout is not compatible with this sandbox")] - SnapshotLayoutMismatch, - /// Tried to restore a snapshot into a sandbox whose registered /// host functions do not satisfy the snapshot's required set. #[error( @@ -367,7 +361,6 @@ impl HyperlightError { | HyperlightError::RefCellBorrowFailed(_) | HyperlightError::RefCellMutBorrowFailed(_) | HyperlightError::ReturnValueConversionFailure(_, _) - | HyperlightError::SnapshotLayoutMismatch | HyperlightError::SnapshotHostFunctionMismatch { .. } | HyperlightError::SystemTimeError(_) | HyperlightError::TryFromSliceError(_) diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs index 5aeba166f..9d6157364 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs @@ -583,6 +583,11 @@ impl HyperlightVm { self.rt_cfg.entry_point = Some(entry_point); } + #[cfg(crashdump)] + pub(crate) fn clear_crashdump_binary_path(&mut self) { + self.rt_cfg.binary_path = None; + } + pub(crate) fn interrupt_handle(&self) -> Arc { self.interrupt_handle.clone() } diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index 9e7e10b24..22b0abc37 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -305,40 +305,6 @@ impl Debug for SandboxMemoryLayout { } impl SandboxMemoryLayout { - /// Whether `other` has the same layout configuration as `self`, - /// i.e. the fields that come from the guest binary and the - /// `SandboxConfiguration`. `snapshot_size` and `pt_size` are - /// excluded because they are outputs of building a snapshot blob - /// (the compacted data size and the size of the rebuilt - /// page-table tail), not configuration inputs, so they differ - /// between the sandbox's live layout and any snapshot taken - /// from it. - /// - /// TODO: separate/remove snapshot_size and pt_size from this struct. - pub(crate) fn is_compatible_with(&self, other: &Self) -> bool { - // Exhaustive destructure so adding a field to - // `SandboxMemoryLayout` fails to compile here, forcing the - // author to decide whether it participates in compatibility. - let Self { - input_data_size, - output_data_size, - heap_size, - code_size, - init_data_size, - init_data_permissions, - scratch_size, - snapshot_size: _, - pt_size: _, - } = self; - *input_data_size == other.input_data_size - && *output_data_size == other.output_data_size - && *heap_size == other.heap_size - && *code_size == other.code_size - && *init_data_size == other.init_data_size - && *init_data_permissions == other.init_data_permissions - && *scratch_size == other.scratch_size - } - /// The maximum amount of memory a single sandbox will be allowed. /// /// Both the scratch region and the snapshot region are bounded by @@ -784,58 +750,6 @@ mod tests { assert!(matches!(layout.unwrap_err(), MemoryRequestTooBig(..))); } - #[test] - fn is_compatible_with_identical_layouts() { - let cfg = SandboxConfiguration::default(); - let a = SandboxMemoryLayout::new(cfg, 4096, 0, None).unwrap(); - let b = SandboxMemoryLayout::new(cfg, 4096, 0, None).unwrap(); - assert!(a.is_compatible_with(&b)); - assert!(b.is_compatible_with(&a)); - } - - #[test] - fn is_compatible_with_ignores_snapshot_size_and_pt_size() { - // `snapshot_size` and `pt_size` are outputs of building a - // snapshot blob, not configuration inputs, so flipping - // them must not break compatibility. - let cfg = SandboxConfiguration::default(); - let a = SandboxMemoryLayout::new(cfg, 4096, 0, None).unwrap(); - let mut b = a; - b.snapshot_size = a.snapshot_size + PAGE_SIZE; - b.set_pt_size(PAGE_SIZE).unwrap(); - assert!(a.is_compatible_with(&b)); - assert!(b.is_compatible_with(&a)); - } - - #[test] - fn is_compatible_with_rejects_each_configured_field() { - let cfg = SandboxConfiguration::default(); - let base = SandboxMemoryLayout::new(cfg, 4096, 0, None).unwrap(); - - // Each mutation must independently break compatibility. - let mutators: &[fn(&mut SandboxMemoryLayout)] = &[ - |l| l.input_data_size += PAGE_SIZE, - |l| l.output_data_size += PAGE_SIZE, - |l| l.heap_size += PAGE_SIZE, - |l| l.code_size += PAGE_SIZE, - |l| l.init_data_size += PAGE_SIZE, - |l| l.scratch_size += PAGE_SIZE, - |l| { - l.init_data_permissions = Some(MemoryRegionFlags::READ); - }, - ]; - for mutate in mutators { - let mut other = base; - mutate(&mut other); - assert!( - !base.is_compatible_with(&other), - "mutation should have broken compatibility: {:?} vs {:?}", - base, - other, - ); - } - } - /// Pinned region offsets. These methods place every region that a /// restored snapshot is interpreted against, so a change shifts /// where the loader reads captured bytes and breaks existing diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 8613546dc..3d976ed04 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -153,6 +153,8 @@ impl MultiUseSandbox { /// Set a callback that discovers page table roots from guest memory. /// The callback receives (snapshot_mem, scratch_mem, cr3) and returns /// the list of root GPAs to walk during snapshot creation. + /// + /// The callback must support every guest restored into this sandbox. pub fn set_pt_root_finder(&mut self, finder: PtRootFinder) { self.pt_root_finder = Some(finder); } @@ -331,10 +333,9 @@ impl MultiUseSandbox { /// Creates a snapshot of the sandbox's current memory state. /// /// The returned snapshot can be applied to any - /// [`MultiUseSandbox`] whose memory layout is structurally - /// compatible with this sandbox's layout and whose registered - /// host functions are a superset of those registered here at the - /// time of capture. See [`MultiUseSandbox::restore`] and + /// [`MultiUseSandbox`] whose registered host functions are a + /// superset of those registered here at the time of capture. See + /// [`MultiUseSandbox::restore`] and /// [`MultiUseSandbox::from_snapshot`] for the exact compatibility /// rules and the error variants returned on mismatch. /// @@ -437,10 +438,6 @@ impl MultiUseSandbox { /// Restores the sandbox's memory to a previously captured snapshot state. /// - /// The snapshot's memory layout must be structurally compatible - /// with this sandbox's layout, otherwise this returns - /// [`SnapshotLayoutMismatch`](crate::HyperlightError::SnapshotLayoutMismatch). - /// /// The sandbox's registered host functions must be a superset of /// those required by the snapshot (matched by name and /// signature). Extras on the sandbox are allowed. The registry @@ -561,23 +558,29 @@ impl MultiUseSandbox { .host_funcs .try_lock() .map_err(|e| crate::new_error!("Error locking host_funcs: {}", e))?; - snapshot.validate_compatibility(&self.mem_mgr.layout, &host_funcs)?; + snapshot.validate_host_functions(&host_funcs)?; } let sregs = snapshot.sregs().ok_or_else(|| { HyperlightError::Error("snapshot from running sandbox should have sregs".to_string()) })?; + // Errors below leave the sandbox poisoned unless base mapping updates make it unrecoverable. + self.status = SandboxStatus::Poisoned; + self.snapshot = None; + + 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)?; + } + if let Err(error) = self.restore_memory_and_mappings(&snapshot) { self.status = SandboxStatus::Unrecoverable; - self.snapshot = None; return Err(error); } - // Errors below here leave the sandbox poisoned (restore must be retried to unpoison). - self.status = SandboxStatus::Poisoned; - self.snapshot = None; - self.vm .reset_vcpu(snapshot.root_pt_gpa(), sregs) .map_err(HyperlightVmError::Restore)?; @@ -593,14 +596,10 @@ impl MultiUseSandbox { // Carry the guest ELF entry point across restore so a later // crashdump fills `AT_ENTRY` from the restored image. #[cfg(crashdump)] - 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)?; + .set_crashdump_entry_point(snapshot.original_entrypoint()); + self.vm.clear_crashdump_binary_path(); } self.mem_mgr @@ -1164,13 +1163,15 @@ mod tests { use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode; use hyperlight_testing::sandbox_sizes::{LARGE_HEAP_SIZE, MEDIUM_HEAP_SIZE, SMALL_HEAP_SIZE}; - use hyperlight_testing::simple_guest_as_pathbuf; + use hyperlight_testing::{c_simple_guest_as_pathbuf, simple_guest_as_pathbuf}; + use crate::func::host_functions::Registerable; #[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::sandbox::uninitialized::{GuestBlob, GuestEnvironment}; use crate::{ GuestBinary, HyperlightError, MultiUseSandbox, Result, SandboxBuilder, SandboxStatus, UninitializedSandbox, @@ -2021,41 +2022,439 @@ mod tests { } #[test] - fn snapshot_restore_rejects_incompatible_layout() { - let mut sandbox = SandboxBuilder::new() - .heap_size(0x10_000) - .build_from_file(simple_guest_as_pathbuf()) + fn snapshot_restore_accepts_different_configured_layout() { + type Configure = fn(&mut SandboxConfiguration); + type LayoutValue = fn(&crate::mem::layout::SandboxMemoryLayout) -> usize; + let cases: &[(&str, Configure, LayoutValue)] = &[ + ( + "input", + |cfg| cfg.set_input_data_size(0x8000), + |layout| layout.input_data_size(), + ), + ( + "output", + |cfg| cfg.set_output_data_size(0x8000), + |layout| layout.output_data_size(), + ), + ( + "heap", + |cfg| cfg.set_heap_size(0x40_000), + |layout| layout.heap_size(), + ), + ( + "scratch", + |cfg| cfg.set_scratch_size(0x90_000), + |layout| layout.get_scratch_size(), + ), + ]; + + for (name, configure, layout_value) in cases { + for incoming_is_larger in [true, false] { + let mut custom_cfg = SandboxConfiguration::default(); + configure(&mut custom_cfg); + let (source_cfg, target_cfg) = if incoming_is_larger { + (custom_cfg, SandboxConfiguration::default()) + } else { + (SandboxConfiguration::default(), custom_cfg) + }; + + let path = simple_guest_as_pathbuf(); + let mut source = + UninitializedSandbox::new(GuestBinary::FilePath(path), Some(source_cfg)) + .unwrap() + .evolve() + .unwrap(); + + let path = simple_guest_as_pathbuf(); + let mut target = + UninitializedSandbox::new(GuestBinary::FilePath(path), Some(target_cfg)) + .unwrap() + .evolve() + .unwrap(); + + let source_value = layout_value(&source.mem_mgr.layout); + assert_ne!(source_value, layout_value(&target.mem_mgr.layout)); + + source.call::("AddToStatic", 42i32).unwrap(); + target + .restore(source.snapshot().unwrap()) + .unwrap_or_else(|err| panic!("restore with different {name} layout: {err}")); + assert_eq!(layout_value(&target.mem_mgr.layout), source_value); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 42); + } + } + } + + #[test] + fn snapshot_restore_recovers_oom_with_larger_heap() { + let mut source_cfg = SandboxConfiguration::default(); + source_cfg.set_heap_size(0x20_000); + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), Some(source_cfg)) + .unwrap() + .evolve() .unwrap(); + let snapshot = source.snapshot().unwrap(); - let mut sandbox2 = SandboxBuilder::new() - .heap_size(0x20_000) - .build_from_file(simple_guest_as_pathbuf()) + let mut target_cfg = SandboxConfiguration::default(); + target_cfg.set_heap_size(0x8000); + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), Some(target_cfg)) + .unwrap() + .evolve() .unwrap(); - let snapshot = sandbox.snapshot().unwrap(); - let err = sandbox2.restore(snapshot); - assert!(matches!(err, Err(HyperlightError::SnapshotLayoutMismatch))); + assert!(target.call::<()>("ExhaustHeap", ()).is_err()); + assert!(target.status().is_poisoned()); + + target.restore(snapshot).unwrap(); + assert!(!target.status().is_poisoned()); + assert_eq!( + target.call::("CallMalloc", 0x10_000i32).unwrap(), + 0x10_000 + ); + } + + #[test] + fn snapshot_restore_applies_smaller_heap_limit() { + let mut source_cfg = SandboxConfiguration::default(); + source_cfg.set_heap_size(0x8000); + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), Some(source_cfg)) + .unwrap() + .evolve() + .unwrap(); + let snapshot = source.snapshot().unwrap(); + + let mut target_cfg = SandboxConfiguration::default(); + target_cfg.set_heap_size(0x20_000); + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), Some(target_cfg)) + .unwrap() + .evolve() + .unwrap(); + + assert_eq!( + target.call::("CallMalloc", 0x10_000i32).unwrap(), + 0x10_000 + ); + target.restore(snapshot).unwrap(); + assert_eq!(target.mem_mgr.layout.heap_size(), 0x8000); + assert!(target.call::("CallMalloc", 0x10_000i32).is_err()); + assert!(target.status().is_poisoned()); + } + + #[test] + fn snapshot_restore_applies_smaller_io_limits() { + let mut source_cfg = SandboxConfiguration::default(); + source_cfg.set_input_data_size(0x2000); + source_cfg.set_output_data_size(0x2000); + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), Some(source_cfg)) + .unwrap() + .evolve() + .unwrap(); + let snapshot = source.snapshot().unwrap(); + + let mut target_cfg = SandboxConfiguration::default(); + target_cfg.set_input_data_size(0x8000); + target_cfg.set_output_data_size(0x8000); + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), Some(target_cfg)) + .unwrap() + .evolve() + .unwrap(); + let large = "x".repeat(0x3000); + + assert_eq!(target.call::("Echo", large.clone()).unwrap(), large); + target.restore(snapshot).unwrap(); + assert_eq!(target.mem_mgr.layout.input_data_size(), 0x2000); + assert_eq!(target.mem_mgr.layout.output_data_size(), 0x2000); + assert!(target.call::("Echo", large).is_err()); + assert!(!target.status().is_poisoned()); + assert_eq!( + target.call::("Echo", "small".to_string()).unwrap(), + "small" + ); + } + + #[test] + fn snapshot_restore_alternates_different_layouts() { + let mut small_cfg = SandboxConfiguration::default(); + small_cfg.set_input_data_size(0x2000); + small_cfg.set_output_data_size(0x2000); + small_cfg.set_heap_size(0x8000); + let path = simple_guest_as_pathbuf(); + let mut small = UninitializedSandbox::new(GuestBinary::FilePath(path), Some(small_cfg)) + .unwrap() + .evolve() + .unwrap(); + small.call::("AddToStatic", 11i32).unwrap(); + let small_snapshot = small.snapshot().unwrap(); + + let mut large_cfg = SandboxConfiguration::default(); + large_cfg.set_input_data_size(0x8000); + large_cfg.set_output_data_size(0x8000); + large_cfg.set_heap_size(0x40_000); + large_cfg.set_scratch_size(0x90_000); + let path = simple_guest_as_pathbuf(); + let mut large = UninitializedSandbox::new(GuestBinary::FilePath(path), Some(large_cfg)) + .unwrap() + .evolve() + .unwrap(); + large.call::("AddToStatic", 22i32).unwrap(); + let large_snapshot = large.snapshot().unwrap(); + + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + + target.restore(small_snapshot.clone()).unwrap(); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 11); + assert_eq!(target.mem_mgr.layout.heap_size(), 0x8000); + + target.restore(large_snapshot).unwrap(); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 22); + assert_eq!(target.mem_mgr.layout.heap_size(), 0x40_000); + + target.restore(small_snapshot).unwrap(); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 11); + assert_eq!(target.mem_mgr.layout.heap_size(), 0x8000); + } + + #[test] + fn snapshot_restore_replaces_rust_guest_with_c_guest() { + let init_data = b"cross-layout-init-data"; + let source_env = GuestEnvironment { + guest_binary: GuestBinary::FilePath(c_simple_guest_as_pathbuf()), + init_data: Some(GuestBlob { + data: init_data, + permissions: MemoryRegionFlags::READ | MemoryRegionFlags::WRITE, + }), + }; + let mut source = UninitializedSandbox::new(source_env, None) + .unwrap() + .evolve() + .unwrap(); + let mut target = + UninitializedSandbox::new(GuestBinary::FilePath(simple_guest_as_pathbuf()), None) + .unwrap() + .evolve() + .unwrap(); + + assert_eq!(source.call::("StackAllocate", 256i32).unwrap(), 256); + assert_eq!(target.call::("AddToStatic", 17i32).unwrap(), 17); + target.set_pt_root_finder(Box::new(|_, _, root| vec![root])); + assert!(target.pt_root_finder.is_some()); + + assert_ne!( + source.mem_mgr.layout.code_size(), + target.mem_mgr.layout.code_size() + ); + assert_ne!( + source.mem_mgr.layout.init_data_size(), + target.mem_mgr.layout.init_data_size() + ); + assert_ne!( + source.mem_mgr.layout.init_data_permissions(), + target.mem_mgr.layout.init_data_permissions() + ); + + let snapshot = source.snapshot().unwrap(); + target.restore(snapshot).unwrap(); + assert_eq!(target.call::("StackAllocate", 512i32).unwrap(), 512); + assert!(matches!( + target.call::("GetStatic", ()), + Err(HyperlightError::GuestError( + ErrorCode::GuestFunctionNotFound, + name + )) if name == "GetStatic" + )); + } + + #[test] + fn snapshot_restore_replaces_c_guest_with_rust_guest() { + let mut source = + UninitializedSandbox::new(GuestBinary::FilePath(simple_guest_as_pathbuf()), None) + .unwrap() + .evolve() + .unwrap(); + assert_eq!(source.call::("AddToStatic", 42i32).unwrap(), 42); + let snapshot = source.snapshot().unwrap(); + + let mut target = + UninitializedSandbox::new(GuestBinary::FilePath(c_simple_guest_as_pathbuf()), None) + .unwrap() + .evolve() + .unwrap(); + assert_eq!(target.call::("StackAllocate", 256i32).unwrap(), 256); + + target.restore(snapshot).unwrap(); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 42); + assert!(matches!( + target.call::("StackAllocate", 512i32), + Err(HyperlightError::GuestError( + ErrorCode::GuestFunctionNotFound, + name + )) if name == "StackAllocate" + )); + } + + #[test] + fn snapshot_restore_alternates_c_and_rust_guests() { + let mut c_source = + UninitializedSandbox::new(GuestBinary::FilePath(c_simple_guest_as_pathbuf()), None) + .unwrap() + .evolve() + .unwrap(); + assert_eq!(c_source.call::("StackAllocate", 256i32).unwrap(), 256); + let c_snapshot = c_source.snapshot().unwrap(); + + let mut rust_source = + UninitializedSandbox::new(GuestBinary::FilePath(simple_guest_as_pathbuf()), None) + .unwrap() + .evolve() + .unwrap(); + rust_source.call::("AddToStatic", 42i32).unwrap(); + let rust_snapshot = rust_source.snapshot().unwrap(); + + let mut target = + UninitializedSandbox::new(GuestBinary::FilePath(c_simple_guest_as_pathbuf()), None) + .unwrap() + .evolve() + .unwrap(); + assert_eq!(target.call::("StackAllocate", 256i32).unwrap(), 256); + + target.restore(rust_snapshot).unwrap(); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 42); + assert!(matches!( + target.call::("StackAllocate", 512i32), + Err(HyperlightError::GuestError( + ErrorCode::GuestFunctionNotFound, + name + )) if name == "StackAllocate" + )); + + target.restore(c_snapshot).unwrap(); + assert_eq!(target.call::("StackAllocate", 512i32).unwrap(), 512); + assert!(matches!( + target.call::("GetStatic", ()), + Err(HyperlightError::GuestError( + ErrorCode::GuestFunctionNotFound, + name + )) if name == "GetStatic" + )); + } + + #[test] + fn snapshot_restore_keeps_target_host_function_implementation() { + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap(); + source + .register_host_function("Echo42", || Ok(1i64)) + .unwrap(); + let mut source = source.evolve().unwrap(); + let snapshot = source.snapshot().unwrap(); + + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap(); + target + .register_host_function("Echo42", || Ok(42i64)) + .unwrap(); + let mut target = target.evolve().unwrap(); + + target.restore(snapshot).unwrap(); + assert_eq!( + target + .call::( + "CallGivenParamlessHostFuncThatReturnsI64", + "Echo42".to_string(), + ) + .unwrap(), + 42 + ); + } + + #[test] + fn snapshot_restore_recovers_poison_with_different_guest() { + let mut source = + UninitializedSandbox::new(GuestBinary::FilePath(c_simple_guest_as_pathbuf()), None) + .unwrap() + .evolve() + .unwrap(); + let snapshot = source.snapshot().unwrap(); + + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + assert!(target.call::<()>("ExhaustHeap", ()).is_err()); + assert!(target.status().is_poisoned()); + + target.restore(snapshot).unwrap(); + assert!(!target.status().is_poisoned()); + assert_eq!(target.call::("StackAllocate", 512i32).unwrap(), 512); + assert!(matches!( + target.call::("GetStatic", ()), + Err(HyperlightError::GuestError( + ErrorCode::GuestFunctionNotFound, + name + )) if name == "GetStatic" + )); } /// Validation runs before any memory or vCPU mutation, so a /// rejected `restore` leaves the target usable. #[test] fn snapshot_restore_failure_leaves_target_usable() { - let mut source = SandboxBuilder::new() - .heap_size(0x10_000) - .build_from_file(simple_guest_as_pathbuf()) + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap(); + source + .register_host_function("Add", |a: i32, b: i32| Ok(a + b)) .unwrap(); + let mut source = source.evolve().unwrap(); - let mut target = SandboxBuilder::new() - .heap_size(0x20_000) - .build_from_file(simple_guest_as_pathbuf()) + let map_mem = allocate_guest_memory(); + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() .unwrap(); target.call::("AddToStatic", 5i32).unwrap(); + let guest_base = 0x200000000_usize; + let region = region_for_memory(&map_mem, guest_base, MemoryRegionFlags::READ); + // SAFETY: `map_mem` is page-aligned and outlives every use of `target`. + unsafe { target.map_region(®ion).unwrap() }; + target + .call::>( + "ReadMappedBuffer", + ( + guest_base as u64, + hyperlight_common::vmem::PAGE_SIZE as u64, + true, + ), + ) + .unwrap(); + let cached_snapshot = target.snapshot().unwrap(); let bad_snapshot = source.snapshot().unwrap(); let err = target.restore(bad_snapshot); - assert!(matches!(err, Err(HyperlightError::SnapshotLayoutMismatch))); + assert!(matches!( + err, + Err(HyperlightError::SnapshotHostFunctionMismatch { missing, .. }) + if missing.iter().any(|name| name == "Add") + )); + assert!(Arc::ptr_eq(&target.snapshot().unwrap(), &cached_snapshot)); + assert_eq!(target.vm.get_mapped_regions().count(), 1); + assert!( + target + .call::("CheckMapped", guest_base as u64) + .unwrap() + ); assert_eq!(target.call::("GetStatic", ()).unwrap(), 5); target.call::("AddToStatic", 3i32).unwrap(); assert_eq!(target.call::("GetStatic", ()).unwrap(), 8); @@ -2091,6 +2490,71 @@ mod tests { assert_eq!(target.call::("GetStatic", ()).unwrap(), 23); } + #[test] + fn snapshot_restore_unmaps_regions_overlapping_incoming_layout() { + let mut source_cfg = SandboxConfiguration::default(); + source_cfg.set_scratch_size(0x90_000); + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), Some(source_cfg)) + .unwrap() + .evolve() + .unwrap(); + source.call::("AddToStatic", 23i32).unwrap(); + let snapshot = source.snapshot().unwrap(); + + let path = simple_guest_as_pathbuf(); + let mut target = UninitializedSandbox::new(GuestBinary::FilePath(path), None) + .unwrap() + .evolve() + .unwrap(); + assert!(snapshot.memory().mem_size() > target.mem_mgr.shared_mem.mem_size()); + + let map_mem = allocate_guest_memory(); + let guest_base = crate::mem::layout::SandboxMemoryLayout::BASE_ADDRESS + + target.mem_mgr.shared_mem.mem_size(); + let region = region_for_memory(&map_mem, guest_base, MemoryRegionFlags::READ); + // SAFETY: `map_mem` is page-aligned and outlives every use of `target`. + unsafe { target.map_region(®ion).unwrap() }; + + target.restore(snapshot).unwrap(); + assert_eq!(target.vm.get_mapped_regions().count(), 0); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 23); + } + + #[test] + fn snapshot_restore_unmaps_region_overlapping_incoming_scratch() { + let incoming_scratch_size = 0x90_000; + let mut source_cfg = SandboxConfiguration::default(); + source_cfg.set_scratch_size(incoming_scratch_size); + let path = simple_guest_as_pathbuf(); + let mut source = UninitializedSandbox::new(GuestBinary::FilePath(path), Some(source_cfg)) + .unwrap() + .evolve() + .unwrap(); + source.call::("AddToStatic", 23i32).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 guest_base = + hyperlight_common::layout::scratch_base_gpa(incoming_scratch_size) as usize; + let target_scratch_base = + hyperlight_common::layout::scratch_base_gpa(SandboxConfiguration::DEFAULT_SCRATCH_SIZE) + as usize; + let map_mem = allocate_guest_memory(); + assert!(guest_base + map_mem.mem_size() <= target_scratch_base); + let region = region_for_memory(&map_mem, guest_base, MemoryRegionFlags::READ); + // SAFETY: `map_mem` is page-aligned and outlives every use of `target`. + unsafe { target.map_region(®ion).unwrap() }; + + target.restore(snapshot).unwrap(); + assert_eq!(target.vm.get_mapped_regions().count(), 0); + assert_eq!(target.call::("GetStatic", ()).unwrap(), 23); + } + /// Compacted snapshot data is reachable at the source's GVA even /// when the target had a different region mapped at a different /// GVA. diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index 7e200a50e..fdc532567 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -15,7 +15,7 @@ use crate::func::Registerable; use crate::mem::layout::SandboxMemoryLayout; use crate::mem::shared_mem::SharedMemory as _; use crate::sandbox::snapshot::{OciDigest, OciReference, OciTag, Snapshot}; -use crate::{GuestBinary, HostFunctions, MultiUseSandbox, SandboxBuilder}; +use crate::{GuestBinary, HostFunctions, MultiUseSandbox, SandboxBuilder, UninitializedSandbox}; fn create_test_sandbox() -> MultiUseSandbox { let path = simple_guest_as_pathbuf(); @@ -2783,6 +2783,50 @@ fn round_trip_preserves_non_default_scratch_size() { assert_eq!(loaded.layout().get_scratch_size(), custom_scratch); } +#[test] +fn persisted_non_default_layout_loads_and_runs() { + use crate::sandbox::SandboxConfiguration; + + let mut config = SandboxConfiguration::default(); + config.set_input_data_size(0x8000); + config.set_output_data_size(0x8000); + config.set_heap_size(0x40_000); + config.set_scratch_size(0x90_000); + let mut source = UninitializedSandbox::new( + GuestBinary::FilePath(simple_guest_as_pathbuf()), + Some(config), + ) + .unwrap() + .evolve() + .unwrap(); + source.call::("AddToStatic", 42i32).unwrap(); + let snapshot = source.snapshot().unwrap(); + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("layout"); + snapshot + .save(&path, &OciTag::new("latest").unwrap()) + .unwrap(); + let loaded = Arc::new(Snapshot::checked_load(&path, OciTag::new("latest").unwrap()).unwrap()); + assert_eq!(loaded.layout().input_data_size(), 0x8000); + assert_eq!(loaded.layout().output_data_size(), 0x8000); + assert_eq!(loaded.layout().heap_size(), 0x40_000); + assert_eq!(loaded.layout().get_scratch_size(), 0x90_000); + + let mut restored = + MultiUseSandbox::from_snapshot(loaded, HostFunctions::default(), None).unwrap(); + assert_eq!(restored.call::("GetStatic", ()).unwrap(), 42); + let large = "x".repeat(0x5000); + assert_eq!( + restored.call::("Echo", large.clone()).unwrap(), + large + ); + assert_eq!( + restored.call::("CallMalloc", 0x10_000i32).unwrap(), + 0x10_000 + ); +} + #[test] fn snapshot_config_records_entrypoint_and_sregs() { let snap = create_snapshot(); @@ -2845,9 +2889,8 @@ fn snapshot_with_no_host_functions_round_trips() { MultiUseSandbox::from_snapshot(Arc::new(loaded), HostFunctions::default(), None).unwrap(); } -// Snapshot lineage and restore semantics. `restore` accepts any -// snapshot whose memory layout and host-function set match the sandbox. -// Snapshots within a compatible set are interchangeable. +// Snapshot lineage and restore semantics. `restore` accepts snapshots +// whose required host functions match the sandbox. #[test] fn linear_chain_restore_in_order() { diff --git a/src/hyperlight_host/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index a4def5b7a..7f93e1998 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -697,26 +697,6 @@ impl Snapshot { signature_mismatches, }) } - - /// Validate that this snapshot can be applied to a sandbox with - /// the given memory layout and host-function registry. - /// - /// The layout must be structurally compatible with the snapshot's - /// layout (see - /// [`SandboxMemoryLayout::is_compatible_with`](crate::mem::layout::SandboxMemoryLayout::is_compatible_with)), - /// and the registry must be a superset of the host functions the - /// snapshot requires (see - /// [`validate_host_functions`](Self::validate_host_functions)). - pub(crate) fn validate_compatibility( - &self, - layout: &crate::mem::layout::SandboxMemoryLayout, - host_funcs: &crate::sandbox::host_funcs::FunctionRegistry, - ) -> Result<()> { - if !self.layout().is_compatible_with(layout) { - return Err(crate::HyperlightError::SnapshotLayoutMismatch); - } - self.validate_host_functions(host_funcs) - } } #[cfg(test)] diff --git a/src/hyperlight_host/tests/wit_test.rs b/src/hyperlight_host/tests/wit_test.rs index ad0d5af90..d19cfcbdb 100644 --- a/src/hyperlight_host/tests/wit_test.rs +++ b/src/hyperlight_host/tests/wit_test.rs @@ -1,12 +1,15 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2025 The Hyperlight Authors. +use std::path::PathBuf; use std::sync::{Arc, Mutex}; use hyperlight_common::component::{Negative, Positive}; use hyperlight_common::resource::BorrowedResourceGuard; use hyperlight_host::{GuestBinary, MultiUseSandbox, UninitializedSandbox}; -use hyperlight_testing::wit_guest_as_pathbuf; +use hyperlight_testing::{ + c_simple_guest_as_pathbuf, simple_guest_as_pathbuf, wit_guest_as_pathbuf, +}; extern crate alloc; mod bindings { @@ -273,19 +276,176 @@ impl test::wit::TestImports for Host { } fn sb() -> TestSandbox { - let path = wit_guest_as_pathbuf(); + sb_from_guest(wit_guest_as_pathbuf()) +} + +fn sb_from_guest(path: PathBuf) -> TestSandbox { let guest_path = GuestBinary::FilePath(path); let uninit = UninitializedSandbox::new(guest_path, None).unwrap(); test::wit::Test::instantiate(uninit, Host {}).unwrap() } mod wit_test { + use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode; + use hyperlight_host::HyperlightError; use proptest::prelude::*; use crate::bindings::test::wit::{ Failable, Roundtrip, TestExports, TestHostResource, roundtrip, }; - use crate::sb; + use crate::{ + GuestBinary, UninitializedSandbox, c_simple_guest_as_pathbuf, sb, sb_from_guest, + simple_guest_as_pathbuf, + }; + + #[test] + fn restore_wit_snapshot_replaces_rust_and_c_guests() { + let mut source = sb(); + assert_eq!( + source + .roundtrip() + .roundtrip_string("before snapshot".to_string()) + .unwrap(), + "before snapshot" + ); + let snapshot = source.sb.snapshot().unwrap(); + + let mut rust_target = sb_from_guest(simple_guest_as_pathbuf()); + assert_eq!( + rust_target.sb.call::("AddToStatic", 17i32).unwrap(), + 17 + ); + rust_target.sb.restore(snapshot.clone()).unwrap(); + assert_eq!( + rust_target + .roundtrip() + .roundtrip_string("restored over Rust".to_string()) + .unwrap(), + "restored over Rust" + ); + assert!(matches!( + rust_target.sb.call::("GetStatic", ()), + Err(HyperlightError::GuestError( + ErrorCode::GuestFunctionNotFound, + name + )) if name == "GetStatic" + )); + + let mut c_target = sb_from_guest(c_simple_guest_as_pathbuf()); + assert_eq!( + c_target.sb.call::("StackAllocate", 256i32).unwrap(), + 256 + ); + c_target.sb.restore(snapshot).unwrap(); + assert_eq!( + c_target + .roundtrip() + .roundtrip_string("restored over C".to_string()) + .unwrap(), + "restored over C" + ); + assert!(matches!( + c_target.sb.call::("StackAllocate", 512i32), + Err(HyperlightError::GuestError( + ErrorCode::GuestFunctionNotFound, + name + )) if name == "StackAllocate" + )); + } + + #[test] + fn restore_rust_and_c_snapshots_replace_wit_guest() { + let mut rust_source = + UninitializedSandbox::new(GuestBinary::FilePath(simple_guest_as_pathbuf()), None) + .unwrap() + .evolve() + .unwrap(); + assert_eq!(rust_source.call::("AddToStatic", 42i32).unwrap(), 42); + let rust_snapshot = rust_source.snapshot().unwrap(); + + let mut rust_target = sb(); + assert_eq!( + rust_target + .roundtrip() + .roundtrip_string("WIT before Rust".to_string()) + .unwrap(), + "WIT before Rust" + ); + rust_target.sb.restore(rust_snapshot).unwrap(); + assert_eq!(rust_target.sb.call::("GetStatic", ()).unwrap(), 42); + + let mut c_source = + UninitializedSandbox::new(GuestBinary::FilePath(c_simple_guest_as_pathbuf()), None) + .unwrap() + .evolve() + .unwrap(); + assert_eq!(c_source.call::("StackAllocate", 256i32).unwrap(), 256); + let c_snapshot = c_source.snapshot().unwrap(); + + let mut c_target = sb(); + assert_eq!( + c_target + .roundtrip() + .roundtrip_string("WIT before C".to_string()) + .unwrap(), + "WIT before C" + ); + c_target.sb.restore(c_snapshot).unwrap(); + assert_eq!( + c_target.sb.call::("StackAllocate", 512i32).unwrap(), + 512 + ); + } + + #[test] + fn restore_chain_replaces_each_guest() { + let mut rust_source = + UninitializedSandbox::new(GuestBinary::FilePath(simple_guest_as_pathbuf()), None) + .unwrap() + .evolve() + .unwrap(); + assert_eq!(rust_source.call::("AddToStatic", 42i32).unwrap(), 42); + let rust_snapshot = rust_source.snapshot().unwrap(); + + let mut wit_source = sb(); + assert_eq!( + wit_source + .roundtrip() + .roundtrip_string("WIT source".to_string()) + .unwrap(), + "WIT source" + ); + let wit_snapshot = wit_source.sb.snapshot().unwrap(); + + let mut target = sb_from_guest(c_simple_guest_as_pathbuf()); + assert_eq!(target.sb.call::("StackAllocate", 256i32).unwrap(), 256); + + target.sb.restore(rust_snapshot).unwrap(); + assert_eq!(target.sb.call::("GetStatic", ()).unwrap(), 42); + assert!(matches!( + target.sb.call::("StackAllocate", 512i32), + Err(HyperlightError::GuestError( + ErrorCode::GuestFunctionNotFound, + name + )) if name == "StackAllocate" + )); + + target.sb.restore(wit_snapshot).unwrap(); + assert_eq!( + target + .roundtrip() + .roundtrip_string("WIT restored".to_string()) + .unwrap(), + "WIT restored" + ); + assert!(matches!( + target.sb.call::("GetStatic", ()), + Err(HyperlightError::GuestError( + ErrorCode::GuestFunctionNotFound, + name + )) if name == "GetStatic" + )); + } prop_compose! { fn arb_testrecord()(contents in ".*", length in any::()) -> roundtrip::Testrecord {