Relax layout requirements for MultiUseSandbox::restore

CHANGELOG.md

@@ -15,6 +15,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<PathBuf>` instead of `Into<String>`. 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

src/hyperlight_host/src/error.rs

@@ -224,12 +224,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(
@@ -380,7 +374,6 @@ impl HyperlightError {
             | HyperlightError::RefCellBorrowFailed(_)
             | HyperlightError::RefCellMutBorrowFailed(_)
             | HyperlightError::ReturnValueConversionFailure(_, _)
-            | HyperlightError::SnapshotLayoutMismatch
             | HyperlightError::SnapshotHostFunctionMismatch { .. }
             | HyperlightError::SystemTimeError(_)
             | HyperlightError::TryFromSliceError(_)

src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs

@@ -592,6 +592,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<dyn InterruptHandle> {
         self.interrupt_handle.clone()
     }

src/hyperlight_host/src/mem/layout.rs

@@ -318,40 +318,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
@@ -797,58 +763,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