From b9dd787717f56a204bd3430de87495973e714d29 Mon Sep 17 00:00:00 2001 From: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:34:35 -0700 Subject: [PATCH] Remove obsolete snapshot compatibility paths Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> --- docs/snapshot-versioning.md | 15 -------- .../src/sandbox/snapshot/file/config.rs | 37 ++++++++----------- .../src/sandbox/snapshot/file_tests.rs | 23 +++++++----- 3 files changed, 29 insertions(+), 46 deletions(-) diff --git a/docs/snapshot-versioning.md b/docs/snapshot-versioning.md index f855c1576..ddf2f8ce8 100644 --- a/docs/snapshot-versioning.md +++ b/docs/snapshot-versioning.md @@ -49,21 +49,6 @@ records it for diagnostics and does not gate loading on it. Record compatibility paths here when a future hard snapshot break can remove them. -### Original ELF entry point - -The persisted `original_entrypoint_addr` field defaults to zero so snapshots -made before it was added remain loadable. At the next hard break, make the -field required, remove `serde(default)`, and reject zero as an invalid entry -point rather than treating it as unknown. - -### Missing MSR state - -Configs written before MSR capture omit the `msrs` array. The loader defaults a -missing `msrs` to an empty array, which restores the destination baseline. - -At the next hard break, make `msrs` required and remove its `serde(default)` -missing-field fallback. - ## Enforcement The format is large and easy to change by accident. Two mechanisms diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs index 01c2f501b..7d839330c 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs @@ -182,18 +182,14 @@ pub(super) struct OciSnapshotConfig { /// 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)] + /// gdb resolves PIE symbols. pub(super) original_entrypoint_addr: u64, /// Special registers captured from the paused vCPU, restored /// verbatim when resuming the call. pub(super) sregs: CommonSpecialRegisters, - /// The MSRs saved in this snapshot. A missing or empty field restores the - /// destination baseline. + /// The MSRs saved in this snapshot. An empty field restores the destination + /// baseline. #[cfg(target_arch = "x86_64")] - #[serde(default)] pub(super) msrs: Vec, pub(super) layout: MemoryLayout, /// Total size of the memory blob in bytes (including the guest @@ -504,9 +500,8 @@ 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`. + // ELF entry point GVA for `AT_ENTRY` in core dumps. It must point + // inside the snapshot region, like `entrypoint_addr`. let snapshot_hi = code_lo .checked_add(self.layout.snapshot_size as u64) .ok_or_else(|| { @@ -515,10 +510,7 @@ impl OciSnapshotConfig { self.layout.snapshot_size ) })?; - if self.original_entrypoint_addr != 0 - && (self.original_entrypoint_addr < code_lo - || self.original_entrypoint_addr >= snapshot_hi) - { + if self.original_entrypoint_addr < code_lo || self.original_entrypoint_addr >= snapshot_hi { return Err(crate::new_error!( "snapshot original entrypoint addr {:#x} is outside the snapshot region [{:#x}, {:#x})", self.original_entrypoint_addr, @@ -663,10 +655,10 @@ mod tests { assert_eq!(restored.msrs, original.msrs); } - /// A config JSON with no MSR state deserializes to an empty set. + /// A config JSON with no MSR state is rejected. #[cfg(target_arch = "x86_64")] #[test] - fn config_without_msrs_deserializes_to_empty_set() { + fn config_without_msrs_is_rejected() { let with = gating_config_with_msrs(Some(vec![MsrEntry { index: 0x10, value: 1, @@ -675,8 +667,11 @@ mod tests { serde_json::from_slice(&serde_json::to_vec(&with).unwrap()).unwrap(); assert!(json.as_object_mut().unwrap().remove("msrs").is_some()); - let restored: OciSnapshotConfig = serde_json::from_value(json).unwrap(); - assert!(restored.msrs.is_empty()); + let err = serde_json::from_value::(json) + .err() + .expect("config without msrs should fail to deserialize") + .to_string(); + assert!(err.contains("missing field `msrs`"), "got: {err}"); } /// Every `ParameterType` survives the round-trip through its serde @@ -762,7 +757,7 @@ mod tests { cpu_vendor: CpuVendor::current(), stack_top_gva: 0x2000, entrypoint_addr: SandboxMemoryLayout::BASE_ADDRESS as u64, - original_entrypoint_addr: 0, + original_entrypoint_addr: SandboxMemoryLayout::BASE_ADDRESS as u64, sregs: distinct_sregs(), #[cfg(target_arch = "x86_64")] msrs: Vec::new(), @@ -845,7 +840,7 @@ mod schema_pin { "cpu_vendor": "intel", "stack_top_gva": 3735928559, "entrypoint_addr": 8192, - "original_entrypoint_addr": 0, + "original_entrypoint_addr": 4096, "sregs": { "cs": { "base": 1, @@ -1031,7 +1026,7 @@ mod schema_pin { "cpu_vendor": "intel", "stack_top_gva": 3735928559, "entrypoint_addr": 8192, - "original_entrypoint_addr": 0, + "original_entrypoint_addr": 4096, "sregs": { "tcr_el1": 1, "mair_el1": 2, diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index 4e0604c86..0f295d123 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -227,10 +227,10 @@ fn msrs_round_trip_via_disk() { assert_eq!(loaded.msrs(), original.as_ref()); } -/// A config with no `msrs` key loads and uses the destination reset set. +/// A config with no `msrs` key is rejected. #[cfg(target_arch = "x86_64")] #[test] -fn snapshot_without_msrs_key_loads_and_runs() { +fn snapshot_without_msrs_key_is_rejected() { let (_dir, path) = save_for_mutation(); rewrite_config(&path, |cfg| { let obj = cfg.as_object_mut().unwrap(); @@ -240,12 +240,11 @@ fn snapshot_without_msrs_key_loads_and_runs() { ); }); - let loaded = Snapshot::checked_load(&path, OciTag::new("latest").unwrap()).unwrap(); - assert_eq!(loaded.msrs(), Some(&Vec::new())); - - let mut sbox = - MultiUseSandbox::from_snapshot(Arc::new(loaded), HostFunctions::default(), None).unwrap(); - assert_eq!(sbox.call::("GetStatic", ()).unwrap(), 0); + let err = unwrap_err_snapshot(Snapshot::checked_load( + &path, + OciTag::new("latest").unwrap(), + )); + assert_err_contains(err, "missing field `msrs`"); } /// A snapshot whose reset set includes a declared guest MSR carries that @@ -2025,12 +2024,16 @@ fn original_entrypoint_addr_outside_snapshot_region_rejected() { } #[test] -fn original_entrypoint_addr_zero_accepted() { +fn original_entrypoint_addr_zero_rejected() { 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(); + let err = unwrap_err_snapshot(Snapshot::checked_load( + &path, + OciTag::new("latest").unwrap(), + )); + assert_err_contains(err, "original entrypoint addr"); } #[test]