Skip to content

Commit f929ea4

Browse files
committed
Remove SandboxRuntimeConfig
Each field moves to the place that already owns its data: * debug_info and guest_core_dump are read from SandboxConfiguration. * The crashdump entry point is read from the memory manager, which restore already updates. * The guest binary path is dropped. Core dumps report <unknown> as the process name for every sandbox, as snapshot sandboxes already did. Sandbox setup and restore no longer need crashdump or gdb plumbing. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
1 parent 3f2ed9b commit f929ea4

10 files changed

Lines changed: 50 additions & 287 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
77
### Added
88

99
### Changed
10+
* Core dumps no longer record the guest binary path. The process name and command line are always `<unknown>`, as they already were for sandboxes created from snapshots by @ludfjig
1011

1112
### Removed
1213

‎docs/how-to-debug-a-hyperlight-guest.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ After the core dump has been created, to inspect the state of the guest, load th
267267

268268
#### Using gdb
269269

270-
Load the core dump alongside the guest binary that was running when the crash occurred:
270+
Load the core dump alongside the guest binary that was running when the crash occurred. Core dumps do not record which guest binary that was, so keep track of the guest each sandbox runs:
271271

272272
```bash
273273
gdb <guest-binary> -c <core-dump-file>

‎src/hyperlight_host/src/hypervisor/crashdump.rs‎

Lines changed: 8 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ const CORE_DUMP_PID: i32 = 1;
3030
/// The page size of the core dump
3131
const CORE_DUMP_PAGE_SIZE: usize = 0x1000;
3232

33+
/// Process name and command line in the core dump. Sandboxes do not
34+
/// record which guest binary they run.
35+
const UNKNOWN_PROCESS: &str = "<unknown>";
36+
3337
/// Structure to hold the crash dump context
3438
/// This structure contains the information needed to create a core dump
3539
#[derive(Debug)]
@@ -38,7 +42,6 @@ pub(crate) struct CrashDumpContext {
3842
regs: [u64; 27],
3943
xsave: Vec<u8>,
4044
entry: u64,
41-
binary: Option<PathBuf>,
4245
}
4346

4447
impl CrashDumpContext {
@@ -47,14 +50,12 @@ impl CrashDumpContext {
4750
regs: [u64; 27],
4851
xsave: Vec<u8>,
4952
entry: u64,
50-
binary: Option<PathBuf>,
5153
) -> Self {
5254
Self {
5355
regions,
5456
regs,
5557
xsave,
5658
entry,
57-
binary,
5859
}
5960
}
6061
}
@@ -88,21 +89,6 @@ impl GuestView {
8889
})
8990
.collect();
9091

91-
let filename = ctx
92-
.binary
93-
.as_deref()
94-
.and_then(|path| path.file_name())
95-
.map_or("<unknown>".to_string(), |name| {
96-
name.to_string_lossy().into_owned()
97-
});
98-
99-
let cmd = ctx
100-
.binary
101-
.as_deref()
102-
.map_or("<unknown>".to_string(), |path| {
103-
path.to_string_lossy().into_owned()
104-
});
105-
10692
// The xsave state is checked as it can be empty
10793
let mut components = vec![];
10894
if !ctx.xsave.is_empty() {
@@ -122,7 +108,7 @@ impl GuestView {
122108
tid: 1,
123109
uid: 0, // User ID
124110
gid: 0, // Group ID
125-
comm: filename,
111+
comm: UNKNOWN_PROCESS.to_string(),
126112
ppid: 0, // Parent PID
127113
pgrp: 0, // Process group ID
128114
nice: 0, // Nice value
@@ -135,7 +121,7 @@ impl GuestView {
135121
session: 0, // Session ID of the process
136122
sighold: 0, // Blocked signal
137123
sigpend: 0, // Pending signal
138-
cmd_line: cmd,
124+
cmd_line: UNKNOWN_PROCESS.to_string(),
139125

140126
arch_state: Box::new(ArchState {
141127
gpr_state: ctx.regs.to_vec(),
@@ -435,13 +421,7 @@ mod test {
435421
#[test]
436422
fn test_crashdump_write_fails_when_no_regions() {
437423
// Create a dummy context
438-
let ctx = CrashDumpContext::new(
439-
vec![],
440-
[0; 27],
441-
vec![],
442-
0,
443-
Some(PathBuf::from("dummy_binary")),
444-
);
424+
let ctx = CrashDumpContext::new(vec![], [0; 27], vec![], 0);
445425

446426
let get_writer = || Ok(Box::new(std::io::empty()) as Box<dyn Write>);
447427

@@ -466,13 +446,7 @@ mod test {
466446
region_type: crate::mem::memory_region::MemoryRegionType::Code,
467447
}];
468448
// Create a dummy context
469-
let ctx = CrashDumpContext::new(
470-
regions,
471-
[0; 27],
472-
vec![],
473-
0x1000,
474-
Some(PathBuf::from("dummy_binary")),
475-
);
449+
let ctx = CrashDumpContext::new(regions, [0; 27], vec![], 0x1000);
476450

477451
let get_writer = || Ok(Box::new(std::io::empty()) as Box<dyn Write>);
478452

‎src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ use crate::sandbox::host_funcs::FunctionRegistry;
3737
use crate::sandbox::snapshot::NextAction;
3838
#[cfg(feature = "mem_profile")]
3939
use crate::sandbox::trace::MemTraceInfo;
40-
#[cfg(crashdump)]
41-
use crate::sandbox::uninitialized::SandboxRuntimeConfig;
4240

4341
impl HyperlightVm {
4442
#[allow(clippy::too_many_arguments)]
@@ -52,7 +50,6 @@ impl HyperlightVm {
5250
page_size: usize,
5351
#[cfg_attr(target_os = "windows", allow(unused_variables))] config: &SandboxConfiguration,
5452
#[cfg(gdb)] _gdb_conn: Option<DebugCommChannel<DebugResponse, DebugMsg>>,
55-
#[cfg(crashdump)] _rt_cfg: SandboxRuntimeConfig,
5653
#[cfg(feature = "mem_profile")] _trace_info: MemTraceInfo,
5754
) -> std::result::Result<Self, CreateHyperlightVmError> {
5855
// TODO: support gdb on aarch64

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

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ use crate::sandbox::outb::{HandleOutbError, handle_outb};
4444
use crate::sandbox::snapshot::NextAction;
4545
#[cfg(feature = "mem_profile")]
4646
use crate::sandbox::trace::MemTraceInfo;
47-
#[cfg(crashdump)]
48-
use crate::sandbox::uninitialized::SandboxRuntimeConfig;
4947

5048
/// Get the logging level filter to pass to the guest entrypoint
5149
///
@@ -399,7 +397,7 @@ pub(crate) struct HyperlightVm {
399397
#[cfg(feature = "mem_profile")]
400398
pub(super) trace_info: MemTraceInfo,
401399
#[cfg(crashdump)]
402-
pub(super) rt_cfg: SandboxRuntimeConfig,
400+
pub(super) guest_core_dump: bool,
403401
/// MSRs restored on snapshot restore.
404402
#[cfg(target_arch = "x86_64")]
405403
pub(super) msr_reset: crate::hypervisor::regs::MsrResetState,
@@ -576,18 +574,6 @@ impl HyperlightVm {
576574
self.next_action = next_action
577575
}
578576

579-
/// Set the guest ELF entry point used to fill `AT_ENTRY` in
580-
/// crashdumps.
581-
#[cfg(crashdump)]
582-
pub(crate) fn set_crashdump_entry_point(&mut self, entry_point: u64) {
583-
self.rt_cfg.entry_point = Some(entry_point);
584-
}
585-
586-
#[cfg(crashdump)]
587-
pub(crate) fn clear_crashdump_binary_path(&mut self) {
588-
self.rt_cfg.binary_path = None;
589-
}
590-
591577
pub(crate) fn interrupt_handle(&self) -> Arc<dyn InterruptHandle> {
592578
self.interrupt_handle.clone()
593579
}
@@ -786,7 +772,7 @@ impl HyperlightVm {
786772
}
787773
Err(e) => {
788774
#[cfg(crashdump)]
789-
if self.rt_cfg.guest_core_dump {
775+
if self.guest_core_dump {
790776
crate::hypervisor::crashdump::generate_crashdump(self, mem_mgr, None)
791777
.map_err(|e| RunVmError::CrashdumpGeneration(Box::new(e)))?;
792778
}

‎src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ use crate::sandbox::host_funcs::FunctionRegistry;
4444
use crate::sandbox::snapshot::NextAction;
4545
#[cfg(feature = "mem_profile")]
4646
use crate::sandbox::trace::MemTraceInfo;
47-
#[cfg(crashdump)]
48-
use crate::sandbox::uninitialized::SandboxRuntimeConfig;
4947

5048
#[cfg(gdb)]
5149
type BoxedVm = Box<dyn DebuggableVm>;
@@ -65,7 +63,6 @@ impl HyperlightVm {
6563
page_size: usize,
6664
#[cfg_attr(target_os = "windows", allow(unused_variables))] config: &SandboxConfiguration,
6765
#[cfg(gdb)] gdb_conn: Option<DebugCommChannel<DebugResponse, DebugMsg>>,
68-
#[cfg(crashdump)] rt_cfg: SandboxRuntimeConfig,
6966
#[cfg(feature = "mem_profile")] trace_info: MemTraceInfo,
7067
) -> std::result::Result<Self, CreateHyperlightVmError> {
7168
let mut vm: BoxedVm = match get_available_hypervisor() {
@@ -136,7 +133,7 @@ impl HyperlightVm {
136133
#[cfg(feature = "mem_profile")]
137134
trace_info,
138135
#[cfg(crashdump)]
139-
rt_cfg,
136+
guest_core_dump: config.get_guest_core_dump(),
140137
msr_reset,
141138
};
142139

@@ -521,7 +518,7 @@ impl HyperlightVm {
521518
mem_mgr: &mut SandboxMemoryManager<HostSharedMemory>,
522519
) -> std::result::Result<Option<crate::hypervisor::crashdump::CrashDumpContext>, CrashDumpError>
523520
{
524-
if self.rt_cfg.guest_core_dump {
521+
if self.guest_core_dump {
525522
let mut regs = [0; 27];
526523

527524
let vcpu_regs = self.vm.regs()?;
@@ -557,17 +554,12 @@ impl HyperlightVm {
557554
regs[25] = sregs.fs.selector as u64; // fs
558555
regs[26] = sregs.gs.selector as u64; // gs
559556

560-
// Use the stored entry point address from the runtime config.
561-
// This is the original entry point (load_addr + ELF entry offset)
562-
// which GDB needs for AT_ENTRY to compute the PIE load offset.
563-
// We cannot use self.next_action here because it transitions from
564-
// Initialise(addr) to Call(dispatch_addr) after guest init.
565-
let initialise = self.rt_cfg.entry_point.unwrap_or_else(|| {
566-
tracing::warn!(
567-
"entry_point was never set in SandboxRuntimeConfig; AT_ENTRY will be 0"
568-
);
569-
0
570-
});
557+
// GDB derives the PIE load offset from AT_ENTRY. `next_action`
558+
// holds the dispatch address after init, so use the ELF entry.
559+
let entry = mem_mgr.original_entrypoint;
560+
if entry == 0 {
561+
tracing::warn!("Guest entry point is unknown; AT_ENTRY will be 0");
562+
}
571563
let mmap_regions: Vec<MemoryRegion> = self.get_mapped_regions().cloned().collect();
572564
let root_pt = self.get_root_pt()?;
573565

@@ -579,8 +571,7 @@ impl HyperlightVm {
579571
regions,
580572
regs,
581573
xsave.to_vec(),
582-
initialise,
583-
self.rt_cfg.binary_path.clone(),
574+
entry,
584575
)))
585576
} else {
586577
Ok(None)
@@ -880,8 +871,6 @@ mod tests {
880871
use crate::mem::shared_mem::{ExclusiveSharedMemory, ReadonlySharedMemory};
881872
use crate::sandbox::SandboxConfiguration;
882873
use crate::sandbox::host_funcs::FunctionRegistry;
883-
#[cfg(any(crashdump, gdb))]
884-
use crate::sandbox::uninitialized::SandboxRuntimeConfig;
885874
use crate::sandbox::uninitialized_evolve::set_up_hypervisor_partition;
886875

887876
/// Test context holding an initialized VM with memory manager.
@@ -1472,8 +1461,6 @@ mod tests {
14721461
/// both `hyperlight_vm()` and `create_test_vm_context()`.
14731462
fn create_test_vm_context(code: &[u8]) -> TestVmContext {
14741463
let config: SandboxConfiguration = Default::default();
1475-
#[cfg(any(crashdump, gdb))]
1476-
let rt_cfg: SandboxRuntimeConfig = Default::default();
14771464

14781465
let (layout, pt_bytes) = create_test_layout(code.len());
14791466

@@ -1506,8 +1493,6 @@ mod tests {
15061493
&config,
15071494
stack_top_gva,
15081495
page_size::get(),
1509-
#[cfg(any(crashdump, gdb))]
1510-
rt_cfg,
15111496
crate::mem::exe::LoadInfo::dummy(),
15121497
)
15131498
.unwrap();

‎src/hyperlight_host/src/hypervisor/mod.rs‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -470,8 +470,6 @@ pub(crate) mod tests {
470470
use hyperlight_testing::dummy_guest_as_pathbuf;
471471

472472
use crate::sandbox::uninitialized::GuestBinary;
473-
#[cfg(any(crashdump, gdb))]
474-
use crate::sandbox::uninitialized::SandboxRuntimeConfig;
475473
use crate::sandbox::uninitialized_evolve::set_up_hypervisor_partition;
476474
use crate::sandbox::{SandboxConfiguration, UninitializedSandbox};
477475
use crate::{Result, is_hypervisor_present};
@@ -489,8 +487,6 @@ pub(crate) mod tests {
489487
let filename = dummy_guest_as_pathbuf();
490488

491489
let config: SandboxConfiguration = Default::default();
492-
#[cfg(any(crashdump, gdb))]
493-
let rt_cfg: SandboxRuntimeConfig = Default::default();
494490
let sandbox =
495491
UninitializedSandbox::new(GuestBinary::FilePath(filename.clone()), Some(config))?;
496492
let (mut mem_mgr, gshm) = sandbox.mgr.build().unwrap();
@@ -502,8 +498,6 @@ pub(crate) mod tests {
502498
&config,
503499
exn_stack_top_gva,
504500
page_size::get(),
505-
#[cfg(any(crashdump, gdb))]
506-
rt_cfg,
507501
sandbox.load_info,
508502
)?;
509503

‎src/hyperlight_host/src/sandbox/initialized.rs‎

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -287,32 +287,8 @@ impl Sandbox {
287287
#[cfg(target_os = "linux")]
288288
crate::signal_handlers::setup_signal_handlers(&config)?;
289289

290-
// Runtime config for the restored sandbox. `guest_core_dump`
291-
// (crashdump) and `guest_debug_info` (gdb) come from the caller's
292-
// config. `binary_path` stays `None`. `set_up_hypervisor_partition`
293-
// fills `entry_point` from the manager's entry point so crashdumps
294-
// carry the correct `AT_ENTRY`.
295-
#[cfg(any(crashdump, gdb))]
296-
let rt_cfg = crate::sandbox::uninitialized::SandboxRuntimeConfig {
297-
#[cfg(crashdump)]
298-
binary_path: None,
299-
#[cfg(gdb)]
300-
debug_info: config.get_guest_debug_info(),
301-
#[cfg(crashdump)]
302-
guest_core_dump: config.get_guest_core_dump(),
303-
#[cfg(crashdump)]
304-
entry_point: None,
305-
};
306-
307-
let mut vm = set_up_hypervisor_partition(
308-
gshm,
309-
&config,
310-
stack_top_gva,
311-
page_size,
312-
#[cfg(any(crashdump, gdb))]
313-
rt_cfg,
314-
load_info,
315-
)?;
290+
let mut vm =
291+
set_up_hypervisor_partition(gshm, &config, stack_top_gva, page_size, load_info)?;
316292

317293
let seed = {
318294
let mut rng = rand::rng();
@@ -670,14 +646,6 @@ impl Sandbox {
670646

671647
self.vm.set_stack_top(snapshot.stack_top_gva());
672648
self.vm.set_next_action(snapshot.next_action());
673-
// Carry the guest ELF entry point across restore so a later
674-
// crashdump fills `AT_ENTRY` from the restored image.
675-
#[cfg(crashdump)]
676-
{
677-
self.vm
678-
.set_crashdump_entry_point(snapshot.original_entrypoint());
679-
self.vm.clear_crashdump_binary_path();
680-
}
681649

682650
self.mem_mgr
683651
.request_libc_rng_reseed(rand::random::<u32>())?;

0 commit comments

Comments
 (0)