Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions docs/snapshot-versioning.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 16 additions & 21 deletions src/hyperlight_host/src/sandbox/snapshot/file/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<MsrEntry>,
pub(super) layout: MemoryLayout,
/// Total size of the memory blob in bytes (including the guest
Expand Down Expand Up @@ -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(|| {
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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::<OciSnapshotConfig>(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
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
23 changes: 13 additions & 10 deletions src/hyperlight_host/src/sandbox/snapshot/file_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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::<i32>("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
Expand Down Expand Up @@ -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]
Expand Down
Loading