Skip to content

Commit 5907be8

Browse files
committed
Add test for creating sandbox from snapshot from sandbox
Previously, this code path was not tested, and would fail because `UninitializedSandbox::evolve()` unconditionally tried to run the guest initialisation function, which has already been run in this case. This simply ignores the initialisation stage if the snapshot has no `preinitialise` entrypoint. All that `UninitializedSandbox` is really good for at this point is registering host functions, so it might make sense to further revisit the API. Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com>
1 parent 6104cbc commit 5907be8

3 files changed

Lines changed: 53 additions & 14 deletions

File tree

src/hyperlight_host/src/hypervisor/hyperlight_vm.rs

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ pub(crate) struct HyperlightVm {
8585
#[cfg(not(gdb))]
8686
vm: Box<dyn VirtualMachine>,
8787
page_size: usize,
88-
entrypoint: u64,
88+
entrypoint: Option<u64>, // only present if this vm has not yet been initialised
8989
orig_rsp: GuestPtr,
9090
interrupt_handle: Arc<dyn InterruptHandleImpl>,
9191

@@ -350,7 +350,7 @@ impl HyperlightVm {
350350
snapshot_mem: GuestSharedMemory,
351351
scratch_mem: GuestSharedMemory,
352352
_pml4_addr: u64,
353-
entrypoint: u64,
353+
entrypoint: Option<u64>,
354354
rsp: u64,
355355
#[cfg_attr(target_os = "windows", allow(unused_variables))] config: &SandboxConfiguration,
356356
#[cfg(gdb)] gdb_conn: Option<DebugCommChannel<DebugResponse, DebugMsg>>,
@@ -450,11 +450,13 @@ impl HyperlightVm {
450450
#[cfg(gdb)]
451451
if ret.gdb_conn.is_some() {
452452
ret.send_dbg_msg(DebugResponse::InterruptHandle(ret.interrupt_handle.clone()))?;
453-
// Add breakpoint to the entry point address
453+
// Add breakpoint to the entry point address, if we are going to initialise
454454
ret.vm.set_debug(true).map_err(VmError::Debug)?;
455-
ret.vm
456-
.add_hw_breakpoint(entrypoint)
457-
.map_err(CreateHyperlightVmError::AddHwBreakpoint)?;
455+
if let Some(entrypoint) = entrypoint {
456+
ret.vm
457+
.add_hw_breakpoint(entrypoint)
458+
.map_err(CreateHyperlightVmError::AddHwBreakpoint)?;
459+
}
458460
}
459461

460462
Ok(ret)
@@ -474,6 +476,10 @@ impl HyperlightVm {
474476
guest_max_log_level: Option<LevelFilter>,
475477
#[cfg(gdb)] dbg_mem_access_fn: Arc<Mutex<SandboxMemoryManager<HostSharedMemory>>>,
476478
) -> std::result::Result<(), InitializeError> {
479+
let Some(entrypoint) = self.entrypoint else {
480+
return Ok(());
481+
};
482+
477483
self.page_size = page_size as usize;
478484

479485
let guest_max_log_level: u64 = match guest_max_log_level {
@@ -482,7 +488,7 @@ impl HyperlightVm {
482488
};
483489

484490
let regs = CommonRegisters {
485-
rip: self.entrypoint,
491+
rip: entrypoint,
486492
rsp: self
487493
.orig_rsp
488494
.absolute()
@@ -740,8 +746,12 @@ impl HyperlightVm {
740746
#[cfg(gdb)]
741747
Ok(VmExit::Debug { dr6, exception }) => {
742748
// Handle debug event (breakpoints)
743-
let stop_reason =
744-
arch::vcpu_stop_reason(self.vm.as_mut(), dr6, self.entrypoint, exception)?;
749+
let stop_reason = arch::vcpu_stop_reason(
750+
self.vm.as_mut(),
751+
dr6,
752+
self.entrypoint.unwrap_or(0),
753+
exception,
754+
)?;
745755
if let Err(e) = self.handle_debug(dbg_mem_access_fn.clone(), stop_reason) {
746756
break Err(e.into());
747757
}
@@ -1088,7 +1098,7 @@ impl HyperlightVm {
10881098
regions,
10891099
regs,
10901100
xsave.to_vec(),
1091-
self.entrypoint,
1101+
self.entrypoint.unwrap_or(0),
10921102
self.rt_cfg.binary_path.clone(),
10931103
filename,
10941104
)))

src/hyperlight_host/src/sandbox/uninitialized.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1258,5 +1258,33 @@ mod tests {
12581258

12591259
let _evolved: MultiUseSandbox = sandbox.evolve().expect("Failed to evolve sandbox");
12601260
}
1261+
1262+
// Test 9: Create snapshot from existing sandbox
1263+
{
1264+
let env = GuestEnvironment::new(GuestBinary::FilePath(binary_path.clone()), None);
1265+
let orig_snapshot = Arc::new(
1266+
Snapshot::from_env(env, Default::default())
1267+
.expect("Failed to create snapshot with default config")
1268+
);
1269+
let orig_sandbox = UninitializedSandbox::from_snapshot(
1270+
orig_snapshot,
1271+
None,
1272+
#[cfg(crashdump)]
1273+
Some(binary_path.clone()),
1274+
)
1275+
.expect("Failed to create orig_sandbox");
1276+
let mut initialized_sandbox = orig_sandbox
1277+
.evolve()
1278+
.expect("Failed to evolve orig_sandbox");
1279+
let new_snapshot = initialized_sandbox.snapshot().expect("Failed to create new_snapshot");
1280+
let new_sandbox = UninitializedSandbox::from_snapshot(
1281+
new_snapshot,
1282+
None,
1283+
#[cfg(crashdump)]
1284+
Some(binary_path.clone()),
1285+
)
1286+
.expect("Failed to create new_sandbox");
1287+
let _evolved = new_sandbox.evolve().expect("Failed to evolve new_sandbox");
1288+
}
12611289
}
12621290
}

src/hyperlight_host/src/sandbox/uninitialized_evolve.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,12 @@ pub(crate) fn set_up_hypervisor_partition(
119119
};
120120
let entrypoint_ptr = mgr
121121
.entrypoint_offset
122-
.ok_or_else(|| new_error!("Entrypoint offset is None"))
123-
.and_then(|x| {
122+
.map(|x| {
124123
let entrypoint_total_offset = mgr.load_addr.clone() + x;
125124
GuestPtr::try_from(entrypoint_total_offset)
126-
})?;
125+
.and_then(|x| x.absolute())
126+
})
127+
.transpose()?;
127128

128129
// Create gdb thread if gdb is enabled and the configuration is provided
129130
#[cfg(gdb)]
@@ -153,7 +154,7 @@ pub(crate) fn set_up_hypervisor_partition(
153154
mgr.shared_mem,
154155
mgr.scratch_mem,
155156
pml4_ptr.absolute()?,
156-
entrypoint_ptr.absolute()?,
157+
entrypoint_ptr,
157158
rsp_ptr.absolute()?,
158159
config,
159160
#[cfg(gdb)]

0 commit comments

Comments
 (0)