Skip to content

Commit 324b2fd

Browse files
committed
Remove obsolete snapshot compatibility paths
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
1 parent deb876d commit 324b2fd

3 files changed

Lines changed: 29 additions & 46 deletions

File tree

docs/snapshot-versioning.md

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -49,21 +49,6 @@ records it for diagnostics and does not gate loading on it.
4949
Record compatibility paths here when a future hard snapshot break can remove
5050
them.
5151

52-
### Original ELF entry point
53-
54-
The persisted `original_entrypoint_addr` field defaults to zero so snapshots
55-
made before it was added remain loadable. At the next hard break, make the
56-
field required, remove `serde(default)`, and reject zero as an invalid entry
57-
point rather than treating it as unknown.
58-
59-
### Missing MSR state
60-
61-
Configs written before MSR capture omit the `msrs` array. The loader defaults a
62-
missing `msrs` to an empty array, which restores the destination baseline.
63-
64-
At the next hard break, make `msrs` required and remove its `serde(default)`
65-
missing-field fallback.
66-
6752
## Enforcement
6853

6954
The format is large and easy to change by accident. Two mechanisms

src/hyperlight_host/src/sandbox/snapshot/file/config.rs

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -178,18 +178,14 @@ pub(super) struct OciSnapshotConfig {
178178
/// Guest virtual address of the ELF entry point
179179
/// (`load_addr + e_entry - base_va`), preserved across the
180180
/// Initialise->Call transition. Fills `AT_ENTRY` in core dumps so
181-
/// gdb resolves PIE symbols. Optional: snapshots written before
182-
/// this field existed deserialize to `0`, which core-dump code
183-
/// treats as unknown.
184-
#[serde(default)]
181+
/// gdb resolves PIE symbols.
185182
pub(super) original_entrypoint_addr: u64,
186183
/// Special registers captured from the paused vCPU, restored
187184
/// verbatim when resuming the call.
188185
pub(super) sregs: CommonSpecialRegisters,
189-
/// The MSRs saved in this snapshot. A missing or empty field restores the
190-
/// destination baseline.
186+
/// The MSRs saved in this snapshot. An empty field restores the destination
187+
/// baseline.
191188
#[cfg(target_arch = "x86_64")]
192-
#[serde(default)]
193189
pub(super) msrs: Vec<MsrEntry>,
194190
pub(super) layout: MemoryLayout,
195191
/// Total size of the memory blob in bytes (including the guest
@@ -507,9 +503,8 @@ impl OciSnapshotConfig {
507503
));
508504
}
509505

510-
// ELF entry point GVA for `AT_ENTRY` in core dumps. 0 means
511-
// unknown. Any other value must point inside the snapshot
512-
// region, like `entrypoint_addr`.
506+
// ELF entry point GVA for `AT_ENTRY` in core dumps. It must point
507+
// inside the snapshot region, like `entrypoint_addr`.
513508
let snapshot_hi = code_lo
514509
.checked_add(self.layout.snapshot_size as u64)
515510
.ok_or_else(|| {
@@ -518,10 +513,7 @@ impl OciSnapshotConfig {
518513
self.layout.snapshot_size
519514
)
520515
})?;
521-
if self.original_entrypoint_addr != 0
522-
&& (self.original_entrypoint_addr < code_lo
523-
|| self.original_entrypoint_addr >= snapshot_hi)
524-
{
516+
if self.original_entrypoint_addr < code_lo || self.original_entrypoint_addr >= snapshot_hi {
525517
return Err(crate::new_error!(
526518
"snapshot original entrypoint addr {:#x} is outside the snapshot region [{:#x}, {:#x})",
527519
self.original_entrypoint_addr,
@@ -666,10 +658,10 @@ mod tests {
666658
assert_eq!(restored.msrs, original.msrs);
667659
}
668660

669-
/// A config JSON with no MSR state deserializes to an empty set.
661+
/// A config JSON with no MSR state is rejected.
670662
#[cfg(target_arch = "x86_64")]
671663
#[test]
672-
fn config_without_msrs_deserializes_to_empty_set() {
664+
fn config_without_msrs_is_rejected() {
673665
let with = gating_config_with_msrs(Some(vec![MsrEntry {
674666
index: 0x10,
675667
value: 1,
@@ -678,8 +670,11 @@ mod tests {
678670
serde_json::from_slice(&serde_json::to_vec(&with).unwrap()).unwrap();
679671
assert!(json.as_object_mut().unwrap().remove("msrs").is_some());
680672

681-
let restored: OciSnapshotConfig = serde_json::from_value(json).unwrap();
682-
assert!(restored.msrs.is_empty());
673+
let err = serde_json::from_value::<OciSnapshotConfig>(json)
674+
.err()
675+
.expect("config without msrs should fail to deserialize")
676+
.to_string();
677+
assert!(err.contains("missing field `msrs`"), "got: {err}");
683678
}
684679

685680
/// Every `ParameterType` survives the round-trip through its serde
@@ -767,7 +762,7 @@ mod tests {
767762
cpu_vendor: CpuVendor::current(),
768763
stack_top_gva: 0x2000,
769764
entrypoint_addr: SandboxMemoryLayout::BASE_ADDRESS as u64,
770-
original_entrypoint_addr: 0,
765+
original_entrypoint_addr: SandboxMemoryLayout::BASE_ADDRESS as u64,
771766
sregs: distinct_sregs(),
772767
#[cfg(target_arch = "x86_64")]
773768
msrs: Vec::new(),
@@ -850,7 +845,7 @@ mod schema_pin {
850845
"cpu_vendor": "intel",
851846
"stack_top_gva": 3735928559,
852847
"entrypoint_addr": 8192,
853-
"original_entrypoint_addr": 0,
848+
"original_entrypoint_addr": 4096,
854849
"sregs": {
855850
"cs": {
856851
"base": 1,
@@ -1036,7 +1031,7 @@ mod schema_pin {
10361031
"cpu_vendor": "intel",
10371032
"stack_top_gva": 3735928559,
10381033
"entrypoint_addr": 8192,
1039-
"original_entrypoint_addr": 0,
1034+
"original_entrypoint_addr": 4096,
10401035
"sregs": {
10411036
"tcr_el1": 1,
10421037
"mair_el1": 2,

src/hyperlight_host/src/sandbox/snapshot/file_tests.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -243,10 +243,10 @@ fn msrs_round_trip_via_disk() {
243243
assert_eq!(loaded.msrs(), original.as_ref());
244244
}
245245

246-
/// A config with no `msrs` key loads and uses the destination reset set.
246+
/// A config with no `msrs` key is rejected.
247247
#[cfg(target_arch = "x86_64")]
248248
#[test]
249-
fn snapshot_without_msrs_key_loads_and_runs() {
249+
fn snapshot_without_msrs_key_is_rejected() {
250250
let (_dir, path) = save_for_mutation();
251251
rewrite_config(&path, |cfg| {
252252
let obj = cfg.as_object_mut().unwrap();
@@ -256,12 +256,11 @@ fn snapshot_without_msrs_key_loads_and_runs() {
256256
);
257257
});
258258

259-
let loaded = Snapshot::checked_load(&path, OciTag::new("latest").unwrap()).unwrap();
260-
assert_eq!(loaded.msrs(), Some(&Vec::new()));
261-
262-
let mut sbox =
263-
MultiUseSandbox::from_snapshot(Arc::new(loaded), HostFunctions::default(), None).unwrap();
264-
assert_eq!(sbox.call::<i32>("GetStatic", ()).unwrap(), 0);
259+
let err = unwrap_err_snapshot(Snapshot::checked_load(
260+
&path,
261+
OciTag::new("latest").unwrap(),
262+
));
263+
assert_err_contains(err, "missing field `msrs`");
265264
}
266265

267266
/// A snapshot whose reset set includes a declared guest MSR carries that
@@ -2042,12 +2041,16 @@ fn original_entrypoint_addr_outside_snapshot_region_rejected() {
20422041
}
20432042

20442043
#[test]
2045-
fn original_entrypoint_addr_zero_accepted() {
2044+
fn original_entrypoint_addr_zero_rejected() {
20462045
let (_dir, path) = save_for_mutation();
20472046
rewrite_config(&path, |cfg| {
20482047
cfg["original_entrypoint_addr"] = Value::from(0u64);
20492048
});
2050-
Snapshot::checked_load(&path, OciTag::new("latest").unwrap()).unwrap();
2049+
let err = unwrap_err_snapshot(Snapshot::checked_load(
2050+
&path,
2051+
OciTag::new("latest").unwrap(),
2052+
));
2053+
assert_err_contains(err, "original entrypoint addr");
20512054
}
20522055

20532056
#[test]

0 commit comments

Comments
 (0)