diff --git a/CHANGELOG.md b/CHANGELOG.md index a7c7c6ae7..7e634063b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Removed ### Fixed +* Fix symbol resolution in guest core dumps for sandboxes created from snapshots by @ludfjig in https://github.com/hyperlight-dev/hyperlight/pull/1618 ## [v0.16.0] - 2026-06-26 diff --git a/src/hyperlight_host/examples/crashdump/main.rs b/src/hyperlight_host/examples/crashdump/main.rs index 7b50459d0..26a99cd26 100644 --- a/src/hyperlight_host/examples/crashdump/main.rs +++ b/src/hyperlight_host/examples/crashdump/main.rs @@ -342,7 +342,7 @@ mod tests { use std::process::Command; use hyperlight_host::sandbox::SandboxConfiguration; - use hyperlight_host::{GuestBinary, MultiUseSandbox, UninitializedSandbox}; + use hyperlight_host::{GuestBinary, HostFunctions, MultiUseSandbox, UninitializedSandbox}; use serial_test::serial; #[cfg(not(windows))] @@ -431,8 +431,9 @@ mod tests { sbox.generate_crashdump_to_dir(dump_dir.to_string_lossy()) .expect("generate_crashdump should succeed"); - // Find the generated hl_core_*.elf file - let mut elf_files: Vec = fs::read_dir(dump_dir) + // Find the generated hl_core_*.elf file. The dump dir is a fresh + // per-test tempdir, so exactly one core must be present. + let elf_files: Vec = fs::read_dir(dump_dir) .unwrap() .filter_map(|e| e.ok()) .map(|e| e.path()) @@ -443,15 +444,161 @@ mod tests { }) .collect(); + assert_eq!( + elf_files.len(), + 1, + "Expected exactly one core dump file (hl_core_*.elf) in {}, found {}", + dump_dir.display(), + elf_files.len() + ); + + elf_files.into_iter().next().unwrap() + } + + /// Snapshot an initialized sandbox, build a fresh sandbox from that + /// snapshot, trigger a crash on it, and return the path to the generated + /// ELF core dump. Used to check that crash dumps from snapshot-created + /// sandboxes resolve symbols the same way as directly-evolved ones. + fn generate_crashdump_from_snapshot(dump_dir: &Path) -> PathBuf { + let guest_path = + hyperlight_testing::simple_guest_as_string().expect("Cannot find simpleguest binary"); + let mut cfg = SandboxConfiguration::default(); + cfg.set_guest_core_dump(true); + let u_sbox = + UninitializedSandbox::new(GuestBinary::FilePath(guest_path), Some(cfg)).unwrap(); + let mut sbox: MultiUseSandbox = u_sbox.evolve().unwrap(); + + let snapshot = sbox.snapshot().expect("snapshot"); + + let mut cfg2 = SandboxConfiguration::default(); + cfg2.set_guest_core_dump(true); + let mut sbox2 = + MultiUseSandbox::from_snapshot(snapshot, HostFunctions::default(), Some(cfg2)).unwrap(); + + let result = sbox2.call::<()>("TriggerException", ()); + assert!(result.is_err(), "TriggerException should return an error"); + + sbox2 + .generate_crashdump_to_dir(dump_dir.to_string_lossy()) + .expect("generate_crashdump should succeed"); + + // The dump dir is a fresh per-test tempdir, so exactly one core + // must be present. + let elf_files: Vec = fs::read_dir(dump_dir) + .unwrap() + .filter_map(|e| e.ok()) + .map(|e| e.path()) + .filter(|p| { + p.file_name() + .and_then(|n| n.to_str()) + .map_or(false, |n| n.starts_with("hl_core_") && n.ends_with(".elf")) + }) + .collect(); + + assert_eq!( + elf_files.len(), + 1, + "Expected exactly one core dump file (hl_core_*.elf) in {}, found {}", + dump_dir.display(), + elf_files.len() + ); + + elf_files.into_iter().next().unwrap() + } + + /// Load `core_path` in GDB against the guest binary and assert that + /// `info symbol $pc` resolves to the guest function that raised the abort. + /// Resolving to the correct symbol only works when the core's `AT_ENTRY` + /// conveys the right PIE load bias. A wrong bias still resolves `$pc` to + /// *some* symbol, just the wrong one, so the expected name is asserted. + /// + /// As an independent check on the underlying mechanism, this also reads + /// the auxiliary vector (`info auxv`) and asserts the `AT_ENTRY` entry is + /// present and non-zero. GDB derives the PIE load bias from `AT_ENTRY`, so + /// a zero (or missing) value is the exact defect a broken entry point + /// produces, independent of GDB's symbol-relocation heuristics. + fn assert_gdb_resolves_pc_symbol(dump_dir: &Path, core_path: &Path) { + // The crash path is deterministic: TriggerException raises a CPU + // exception, and the guest handler reports it to the host via the + // abort/`outb` path, so `$pc` sits in that path when the dump is + // taken. Which exact leaf `$pc` lands on depends on inlining + // (`hyperlight_guest::exit::write_abort` when inlined, the + // `hyperlight_guest::exit::arch::out32` leaf in a non-inlined debug + // build), so we assert on the shared `hyperlight_guest::exit::` + // module prefix rather than one specific function. + const EXPECTED_SYMBOL: &str = "hyperlight_guest::exit::"; + + let guest_path = hyperlight_testing::simple_guest_as_string().expect("simpleguest binary"); + + let cmd_file = dump_dir.join("gdb_sym_cmds.txt"); + let out_file = dump_dir.join("gdb_sym_output.txt"); + + let cmds = format!( + "\ +set pagination off +set logging file {out} +set logging enabled on +file {binary} +core-file {core} +echo === SYMBOL ===\\n +info symbol $pc +echo === AUXV ===\\n +info auxv +echo === DONE ===\\n +set logging enabled off +quit +", + out = out_file.display(), + binary = guest_path, + core = core_path.display(), + ); + + let gdb_output = run_gdb_batch(&cmd_file, &out_file, &cmds); + println!("GDB symbol output:\n{gdb_output}"); + + assert!( + gdb_output.contains("=== SYMBOL ==="), + "GDB should have printed the SYMBOL marker.\nOutput:\n{gdb_output}" + ); + assert!( + !gdb_output.contains("No symbol matches $pc"), + "GDB failed to resolve $pc to a symbol — this indicates the core's \ + AT_ENTRY does not convey the correct PIE load bias.\nOutput:\n{gdb_output}" + ); + assert!( + gdb_output.contains(EXPECTED_SYMBOL), + "GDB resolved $pc to the wrong symbol — the core's AT_ENTRY conveys \ + an incorrect PIE load bias. Expected `{EXPECTED_SYMBOL}`.\nOutput:\n{gdb_output}" + ); + + // Independent mechanism check: AT_ENTRY must be present and non-zero. + // `info auxv` prints one entry per line, e.g. + // `9 AT_ENTRY Entry point of program 0x200000da0` + // The value is the last whitespace-separated token on the line. + let at_entry = gdb_output + .lines() + .find(|l| l.contains("AT_ENTRY")) + .unwrap_or_else(|| panic!("info auxv did not report AT_ENTRY.\nOutput:\n{gdb_output}")); + let value_tok = at_entry + .split_whitespace() + .next_back() + .expect("AT_ENTRY line has a value token"); + let value = value_tok + .strip_prefix("0x") + .and_then(|h| u64::from_str_radix(h, 16).ok()) + .unwrap_or_else(|| { + panic!("could not parse AT_ENTRY value {value_tok:?}.\nOutput:\n{gdb_output}") + }); assert!( - !elf_files.is_empty(), - "No core dump file (hl_core_*.elf) found in {}", - dump_dir.display() + value != 0, + "AT_ENTRY is zero — the core does not convey the guest's entry \ + point, so GDB cannot compute the PIE load bias.\nOutput:\n{gdb_output}" ); - // Return the newest one (lexicographic sort by timestamp works) - elf_files.sort(); - elf_files.pop().unwrap() + assert!( + gdb_output.contains("=== DONE ==="), + "GDB should have completed successfully.\nOutput:\n{gdb_output}" + ); } /// Write GDB batch commands to `cmd_path`, run GDB, and return the @@ -591,4 +738,40 @@ quit "GDB should have completed successfully.\nOutput:\n{gdb_output}" ); } + + /// Verify that GDB can resolve a guest symbol from the crash dump. + /// + /// Symbol resolution for the PIE guest binary only works if the core's + /// `AT_ENTRY` auxv entry conveys the correct load bias: GDB computes the + /// bias as `AT_ENTRY - e_entry` and applies it to the binary's symbols. + /// If `AT_ENTRY` is wrong (e.g. zero), GDB cannot match `$pc` to any + /// symbol, so this test guards that the entry point is reported correctly. + #[test] + #[serial] + fn test_crashdump_gdb_symbols() { + if !gdb_is_available() { + eprintln!("Skipping test: {GDB_COMMAND} not found on PATH"); + return; + } + + let dump_dir = tempfile::tempdir().expect("create temp dir"); + let core_path = generate_crashdump_with_content(dump_dir.path()); + assert_gdb_resolves_pc_symbol(dump_dir.path(), &core_path); + } + + /// Same symbol-resolution guarantee as [`test_crashdump_gdb_symbols`], but + /// for a sandbox created from a snapshot. The crash dump must convey the + /// same `AT_ENTRY` so symbols resolve identically. + #[test] + #[serial] + fn test_crashdump_gdb_symbols_from_snapshot() { + if !gdb_is_available() { + eprintln!("Skipping test: {GDB_COMMAND} not found on PATH"); + return; + } + + let dump_dir = tempfile::tempdir().expect("create temp dir"); + let core_path = generate_crashdump_from_snapshot(dump_dir.path()); + assert_gdb_resolves_pc_symbol(dump_dir.path(), &core_path); + } } diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs index 4419b60f8..770959c14 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs @@ -51,7 +51,7 @@ impl HyperlightVm { snapshot_mem: SnapshotSharedMemory, scratch_mem: GuestSharedMemory, root_pt_addr: u64, - entrypoint: NextAction, + next_action: NextAction, rsp_gva: u64, page_size: usize, config: &SandboxConfiguration, @@ -84,7 +84,7 @@ impl HyperlightVm { let vm_can_reset_vcpu = vm.can_reset_vcpu(); let mut ret = Self { vm, - entrypoint, + next_action, rsp_gva, interrupt_handle, page_size, @@ -119,7 +119,7 @@ impl HyperlightVm { std::sync::Mutex>, >, ) -> Result<(), InitializeError> { - let NextAction::Initialise(initialise) = self.entrypoint else { + let NextAction::Initialise(initialise) = self.next_action else { return Ok(()); }; let mut x: [u64; 31] = [0; 31]; @@ -149,7 +149,7 @@ impl HyperlightVm { return Err(InitializeError::InvalidStackPointer(regs.sp)); } self.rsp_gva = regs.sp; - self.entrypoint = NextAction::Call(regs.x[0]); + self.next_action = NextAction::Call(regs.x[0]); Ok(()) } @@ -162,7 +162,7 @@ impl HyperlightVm { std::sync::Mutex>, >, ) -> Result<(), DispatchGuestCallError> { - let NextAction::Call(dispatch_func_addr) = self.entrypoint else { + let NextAction::Call(dispatch_func_addr) = self.next_action else { return Err(DispatchGuestCallError::Uninitialized); }; let mut regs = CommonRegisters { diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs index 56f7d635d..2e545aba7 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs @@ -372,7 +372,7 @@ pub(crate) struct HyperlightVm { #[cfg(not(gdb))] pub(super) vm: Box, pub(super) page_size: usize, - pub(super) entrypoint: NextAction, // only present if this vm has not yet been initialised + pub(super) next_action: NextAction, // `Initialise` before the guest has run, `Call` afterwards pub(super) rsp_gva: u64, pub(super) interrupt_handle: Arc, @@ -565,14 +565,21 @@ impl HyperlightVm { self.rsp_gva = gva; } - /// Get the current entrypoint action - pub(crate) fn get_entrypoint(&self) -> NextAction { - self.entrypoint + /// Get the next action to perform when the sandbox resumes + pub(crate) fn get_next_action(&self) -> NextAction { + self.next_action } - /// Set the current entrypoint action - pub(crate) fn set_entrypoint(&mut self, entrypoint: NextAction) { - self.entrypoint = entrypoint + /// Set the next action to perform when the sandbox resumes + pub(crate) fn set_next_action(&mut self, next_action: NextAction) { + self.next_action = next_action + } + + /// Set the guest ELF entry point used to fill `AT_ENTRY` in + /// crashdumps. + #[cfg(crashdump)] + pub(crate) fn set_crashdump_entry_point(&mut self, entry_point: u64) { + self.rt_cfg.entry_point = Some(entry_point); } pub(crate) fn interrupt_handle(&self) -> Arc { diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs index 82206f787..ae17e100b 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -77,7 +77,7 @@ impl HyperlightVm { snapshot_mem: SnapshotSharedMemory, scratch_mem: GuestSharedMemory, _root_pt_addr: u64, - entrypoint: NextAction, + next_action: NextAction, rsp_gva: u64, page_size: usize, #[cfg_attr(target_os = "windows", allow(unused_variables))] config: &SandboxConfiguration, @@ -141,7 +141,7 @@ impl HyperlightVm { #[cfg_attr(not(gdb), allow(unused_mut))] let mut ret = Self { vm, - entrypoint, + next_action, rsp_gva, interrupt_handle, page_size, @@ -183,7 +183,7 @@ impl HyperlightVm { // `one_shot_entry_bp` so it does not interfere with later // user-installed breakpoints at the same address. ret.vm.set_debug(true).map_err(VmError::Debug)?; - let entry_addr = match entrypoint { + let entry_addr = match next_action { NextAction::Initialise(addr) | NextAction::Call(addr) => Some(addr), #[cfg(test)] NextAction::None => None, @@ -212,7 +212,7 @@ impl HyperlightVm { guest_max_log_level: Option, #[cfg(gdb)] dbg_mem_access_fn: Arc>>, ) -> std::result::Result<(), InitializeError> { - let NextAction::Initialise(initialise) = self.entrypoint else { + let NextAction::Initialise(initialise) = self.next_action else { return Ok(()); }; @@ -251,7 +251,7 @@ impl HyperlightVm { return Err(InitializeError::InvalidStackPointer(regs.rsp)); } self.rsp_gva = regs.rsp; - self.entrypoint = NextAction::Call(regs.rax); + self.next_action = NextAction::Call(regs.rax); Ok(()) } @@ -284,7 +284,7 @@ impl HyperlightVm { host_funcs: &Arc>, #[cfg(gdb)] dbg_mem_access_fn: Arc>>, ) -> std::result::Result<(), DispatchGuestCallError> { - let NextAction::Call(dispatch_func_addr) = self.entrypoint else { + let NextAction::Call(dispatch_func_addr) = self.next_action else { return Err(DispatchGuestCallError::Uninitialized); }; let mut rflags = 1 << 1; // RFLAGS.1 is RES1 @@ -567,7 +567,7 @@ impl HyperlightVm { // Use the stored entry point address from the runtime config. // This is the original entry point (load_addr + ELF entry offset) // which GDB needs for AT_ENTRY to compute the PIE load offset. - // We cannot use self.entrypoint here because it transitions from + // We cannot use self.next_action here because it transitions from // Initialise(addr) to Call(dispatch_addr) after guest init. let initialise = self.rt_cfg.entry_point.unwrap_or_else(|| { tracing::warn!( @@ -2060,7 +2060,7 @@ mod tests { // Re-run from entrypoint (flag=1 means guest skips dirty phase, just does FXSAVE) // Use stack_top - 8 to match initialise()'s behavior (simulates call pushing return addr) - let NextAction::Call(rip) = ctx.ctx.vm.entrypoint else { + let NextAction::Call(rip) = ctx.ctx.vm.next_action else { panic!("entrypoint should be call"); }; let regs = CommonRegisters { diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index 1804d1c9f..7c74fea91 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -140,8 +140,13 @@ pub(crate) struct SandboxMemoryManager { pub(crate) scratch_mem: S, /// The memory layout of the underlying shared memory pub(crate) layout: SandboxMemoryLayout, - /// Offset for the execution entrypoint from `load_addr` - pub(crate) entrypoint: NextAction, + /// The next action to perform when this sandbox resumes: + /// `Initialise` before the guest has run, `Call` afterwards. + pub(crate) next_action: NextAction, + /// Guest virtual address of the guest binary's ELF entry point, + /// preserved across the `Initialise` -> `Call` transition so it + /// can fill `AT_ENTRY` in guest core dumps. 0 if unknown. + pub(crate) original_entrypoint: u64, /// Buffer for accumulating guest abort messages pub(crate) abort_buffer: Vec, /// Generation counter: how many snapshots have been taken from @@ -275,13 +280,14 @@ where layout: SandboxMemoryLayout, shared_mem: SnapshotSharedMemory, scratch_mem: S, - entrypoint: NextAction, + next_action: NextAction, ) -> Self { Self { layout, shared_mem, scratch_mem, - entrypoint, + next_action, + original_entrypoint: 0, abort_buffer: Vec::new(), snapshot_count: 0, } @@ -300,7 +306,7 @@ where root_pt_gpas: &[u64], rsp_gva: u64, sregs: CommonSpecialRegisters, - entrypoint: NextAction, + next_action: NextAction, host_functions: HostFunctionDetails, ) -> Result { self.snapshot_count += 1; @@ -313,7 +319,8 @@ where root_pt_gpas, rsp_gva, sregs, - entrypoint, + next_action, + self.original_entrypoint, self.snapshot_count, host_functions, ) @@ -325,8 +332,9 @@ impl SandboxMemoryManager { let layout = *s.layout(); let shared_mem = s.memory().to_mgr_snapshot_mem()?; let scratch_mem = ExclusiveSharedMemory::new(s.layout().get_scratch_size())?; - let entrypoint = s.entrypoint(); - let mut mgr = Self::new(layout, shared_mem, scratch_mem, entrypoint); + 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 @@ -357,7 +365,8 @@ impl SandboxMemoryManager { shared_mem: hshm, scratch_mem: hscratch, layout: self.layout, - entrypoint: self.entrypoint, + next_action: self.next_action, + original_entrypoint: self.original_entrypoint, abort_buffer: self.abort_buffer, snapshot_count: self.snapshot_count, }; @@ -365,7 +374,8 @@ impl SandboxMemoryManager { shared_mem: gshm, scratch_mem: gscratch, layout: self.layout, - entrypoint: self.entrypoint, + next_action: self.next_action, + original_entrypoint: self.original_entrypoint, abort_buffer: Vec::new(), // Guest doesn't need abort buffer snapshot_count: self.snapshot_count, }; @@ -503,6 +513,9 @@ impl SandboxMemoryManager { // 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)) diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 6f9e87e17..8e60c68e7 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -243,11 +243,11 @@ impl MultiUseSandbox { #[cfg(target_os = "linux")] crate::signal_handlers::setup_signal_handlers(&config)?; - // Build the runtime config from the caller's `SandboxConfiguration` - // so that `guest_core_dump` (crashdump) and `guest_debug_info` (gdb) - // take effect just like they do in the normal evolve path. - // `binary_path` and `entry_point` are not available from a snapshot - // and are left unset. This only affects metadata in core dumps. + // Runtime config for the restored sandbox. `guest_core_dump` + // (crashdump) and `guest_debug_info` (gdb) come from the caller's + // config. `binary_path` stays `None`. `set_up_hypervisor_partition` + // fills `entry_point` from the manager's entry point so crashdumps + // carry the correct `AT_ENTRY`. #[cfg(any(crashdump, gdb))] let rt_cfg = crate::sandbox::uninitialized::SandboxRuntimeConfig { #[cfg(crashdump)] @@ -294,7 +294,7 @@ impl MultiUseSandbox { // If the snapshot was taken from an already-initialized guest // (NextAction::Call), apply the captured special registers so // the guest resumes in the correct CPU state. - if matches!(snapshot.entrypoint(), super::snapshot::NextAction::Call(_)) { + if matches!(snapshot.next_action(), super::snapshot::NextAction::Call(_)) { let sregs = snapshot.sregs().ok_or_else(|| { crate::new_error!("snapshot with NextAction::Call must have captured sregs") })?; @@ -385,7 +385,7 @@ impl MultiUseSandbox { .vm .get_snapshot_sregs() .map_err(|e| HyperlightError::HyperlightVmError(e.into()))?; - let entrypoint = self.vm.get_entrypoint(); + let next_action = self.vm.get_next_action(); let host_functions = (&*self.host_funcs.try_lock().map_err(|e| { crate::new_error!("Error locking host_funcs at {}:{}: {}", file!(), line!(), e) })?) @@ -396,7 +396,7 @@ impl MultiUseSandbox { &root_pt_gpas, stack_top_gpa, sregs, - entrypoint, + next_action, host_functions, )?; let snapshot = Arc::new(memory_snapshot); @@ -544,7 +544,12 @@ impl MultiUseSandbox { })?; self.vm.set_stack_top(snapshot.stack_top_gva()); - self.vm.set_entrypoint(snapshot.entrypoint()); + self.vm.set_next_action(snapshot.next_action()); + // 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 { diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs index 9b8d0792c..5e9fe6c02 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs @@ -177,6 +177,14 @@ pub(super) struct OciSnapshotConfig { pub(super) stack_top_gva: u64, /// Guest virtual address the loader resumes the paused call at. pub(super) entrypoint_addr: u64, + /// Guest virtual address of the ELF entry point + /// (`load_addr + e_entry - base_va`), preserved across the + /// Initialise->Call transition. Fills `AT_ENTRY` in core dumps so + /// gdb resolves PIE symbols. Optional: snapshots written before + /// this field existed deserialize to `0`, which core-dump code + /// treats as unknown. + #[serde(default)] + pub(super) original_entrypoint_addr: u64, /// Special registers captured from the paused vCPU, restored /// verbatim when resuming the call. pub(super) sregs: CommonSpecialRegisters, @@ -480,6 +488,20 @@ impl OciSnapshotConfig { )); } + // ELF entry point GVA for `AT_ENTRY` in core dumps. 0 means + // unknown. Any other value must point inside the snapshot + // region, like `entrypoint_addr`. + if self.original_entrypoint_addr != 0 + && (self.original_entrypoint_addr < snap_lo || self.original_entrypoint_addr >= snap_hi) + { + return Err(crate::new_error!( + "snapshot original entrypoint addr {:#x} is outside the snapshot region [{:#x}, {:#x})", + self.original_entrypoint_addr, + snap_lo, + snap_hi + )); + } + // `stack_top_gva` is restored into the guest stack pointer. // Bound it to the architecture's addressable GVA range so a // malformed config cannot install an out-of-range stack @@ -675,6 +697,7 @@ mod tests { cpu_vendor: CpuVendor::current(), stack_top_gva: 0x2000, entrypoint_addr: SandboxMemoryLayout::BASE_ADDRESS as u64, + original_entrypoint_addr: 0, sregs: distinct_sregs(), layout: MemoryLayout { input_data_size: 0, @@ -746,6 +769,7 @@ mod schema_pin { "cpu_vendor": "intel", "stack_top_gva": 3735928559, "entrypoint_addr": 8192, + "original_entrypoint_addr": 0, "sregs": { "cs": { "base": 1, @@ -921,6 +945,7 @@ mod schema_pin { "cpu_vendor": "intel", "stack_top_gva": 3735928559, "entrypoint_addr": 8192, + "original_entrypoint_addr": 0, "sregs": { "tcr_el1": 1, "mair_el1": 2, diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs index 59fcd6372..6e13085da 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs @@ -549,7 +549,7 @@ impl Snapshot { } fn build_config(&self) -> crate::Result { - let (entrypoint_addr, sregs) = match (self.entrypoint, self.sregs.as_ref()) { + let (entrypoint_addr, sregs) = match (self.next_action, self.sregs.as_ref()) { (NextAction::Call(addr), Some(sregs)) => (addr, sregs), (NextAction::Call(_), None) => { return Err(crate::new_error!( @@ -584,6 +584,7 @@ impl Snapshot { cpu_vendor: CpuVendor::current(), stack_top_gva: self.stack_top_gva, entrypoint_addr, + original_entrypoint_addr: self.original_entrypoint, sregs: *sregs, layout: MemoryLayout { input_data_size: l.input_data_size(), @@ -847,8 +848,8 @@ impl Snapshot { )); } - // 8. Build entrypoint + sregs back from the config. - let entrypoint = NextAction::Call(cfg.entrypoint_addr); + // 8. Build the next action + sregs back from the config. + let next_action = NextAction::Call(cfg.entrypoint_addr); // 9. Reconstitute host_functions metadata. let snapshot_generation = cfg.snapshot_generation; @@ -871,7 +872,8 @@ impl Snapshot { load_info: crate::mem::exe::LoadInfo::dummy(), stack_top_gva: cfg.stack_top_gva, sregs: Some(cfg.sregs), - entrypoint, + next_action, + original_entrypoint: cfg.original_entrypoint_addr, snapshot_generation, host_functions, }) diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index 691f6f920..e93c38763 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -1765,6 +1765,28 @@ fn entrypoint_addr_below_base_address_rejected() { assert_err_contains(err, "entrypoint addr"); } +#[test] +fn original_entrypoint_addr_outside_snapshot_region_rejected() { + let (_dir, path) = save_for_mutation(); + rewrite_config(&path, |cfg| { + cfg["original_entrypoint_addr"] = Value::from(0xDEAD_BEEF_0000u64); + }); + let err = unwrap_err_snapshot(Snapshot::checked_load( + &path, + OciTag::new("latest").unwrap(), + )); + assert_err_contains(err, "original entrypoint addr"); +} + +#[test] +fn original_entrypoint_addr_zero_accepted() { + let (_dir, path) = save_for_mutation(); + rewrite_config(&path, |cfg| { + cfg["original_entrypoint_addr"] = Value::from(0u64); + }); + Snapshot::checked_load(&path, OciTag::new("latest").unwrap()).unwrap(); +} + // `load`: skips blob digest verification, runs every // other validator. diff --git a/src/hyperlight_host/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index 9a638a98e..c062efad7 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -93,7 +93,16 @@ pub struct Snapshot { sregs: Option, /// The next action that should be performed on this snapshot - entrypoint: NextAction, + next_action: NextAction, + + /// Guest virtual address of the guest binary's ELF entry point + /// (`load_addr + e_entry - base_va`). Unlike `next_action`, which + /// transitions to `Call(dispatch_addr)` once the guest has run, + /// this preserves the original entry across that transition. Used + /// to fill `AT_ENTRY` in guest core dumps so a debugger can + /// compute the PIE load bias. 0 if unknown (e.g. an older + /// on-disk snapshot that predates this field). + original_entrypoint: u64, /// The generation number assigned to this snapshot when it was /// taken — i.e. "this is the Nth snapshot taken from the sandbox's @@ -375,13 +384,16 @@ impl Snapshot { - hyperlight_common::layout::SCRATCH_TOP_EXN_STACK_OFFSET + 1; + let entrypoint_gva = load_addr + entrypoint_va - base_va; + Ok(Self { memory: ReadonlySharedMemory::from_bytes(&memory, layout.snapshot_size())?, layout, load_info, stack_top_gva: exn_stack_top_gva, sregs: None, - entrypoint: NextAction::Initialise(load_addr + entrypoint_va - base_va), + next_action: NextAction::Initialise(entrypoint_gva), + original_entrypoint: entrypoint_gva, snapshot_generation: 0, host_functions: HostFunctionDetails { host_functions: None, @@ -407,7 +419,8 @@ impl Snapshot { root_pt_gpas: &[u64], stack_top_gva: u64, sregs: CommonSpecialRegisters, - entrypoint: NextAction, + next_action: NextAction, + original_entrypoint: u64, snapshot_generation: u64, host_functions: HostFunctionDetails, ) -> Result { @@ -560,7 +573,8 @@ impl Snapshot { load_info, stack_top_gva, sregs: Some(sregs), - entrypoint, + next_action, + original_entrypoint, snapshot_generation, host_functions, }) @@ -603,8 +617,15 @@ impl Snapshot { self.sregs.as_ref() } - pub(crate) fn entrypoint(&self) -> NextAction { - self.entrypoint + pub(crate) fn next_action(&self) -> NextAction { + self.next_action + } + + /// Guest virtual address of the guest binary's ELF entry point, + /// preserved across the `Initialise` -> `Call` transition. Used + /// to fill `AT_ENTRY` in guest core dumps. 0 if unknown. + pub(crate) fn original_entrypoint(&self) -> u64 { + self.original_entrypoint } /// Validate that `provided` is a superset of the host functions @@ -762,6 +783,7 @@ mod tests { 0, default_sregs(), super::NextAction::None, + 0, 1, HostFunctionDetails::default(), ) @@ -779,6 +801,7 @@ mod tests { 0, default_sregs(), super::NextAction::None, + 0, 2, HostFunctionDetails::default(), ) diff --git a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs index a38d14409..09e2761e3 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs @@ -151,14 +151,16 @@ pub(crate) fn set_up_hypervisor_partition( let trace_info = MemTraceInfo::new(_load_info.info)?; // Store the original entry point address in the runtime config for core dumps. - // This is needed because `entrypoint` transitions from `Initialise(addr)` to + // This is needed because `next_action` transitions from `Initialise(addr)` to // `Call(dispatch_addr)` after guest initialisation, losing the original value - // that GDB needs to compute the PIE binary's load offset. + // that GDB needs to compute the PIE binary's load offset. The manager carries + // it across that transition (and across snapshot save/restore), so it is + // correct for both the evolve path and `MultiUseSandbox::from_snapshot`. #[cfg(crashdump)] let rt_cfg = { let mut rt_cfg = rt_cfg; - if let crate::sandbox::snapshot::NextAction::Initialise(addr) = mgr.entrypoint { - rt_cfg.entry_point = Some(addr); + if mgr.original_entrypoint != 0 { + rt_cfg.entry_point = Some(mgr.original_entrypoint); } rt_cfg }; @@ -167,7 +169,7 @@ pub(crate) fn set_up_hypervisor_partition( mgr.shared_mem, mgr.scratch_mem, mgr.layout.get_pt_base_gpa(), - mgr.entrypoint, + mgr.next_action, stack_top_gva, page_size, config,