Skip to content

Commit d00d503

Browse files
authored
Add unrecoverable sandbox state (#1727)
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
1 parent 3e6a885 commit d00d503

11 files changed

Lines changed: 826 additions & 151 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
99
configuration, host functions, init data and memory mappings, then builds a
1010
`MultiUseSandbox` from a guest binary on disk, a guest binary in memory, or a
1111
snapshot by @jprendes in https://github.com/hyperlight-dev/hyperlight/pull/1725
12+
* Add `MultiUseSandbox::status()`, which returns `SandboxStatus` for inspecting sandbox lifecycle state (poisoned, unrecoverable).
1213

1314
### Changed
1415
* **Breaking:** Guest MSR state is now saved and restored across snapshots.
@@ -17,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
1718
resets to a clean default. On KVM the guest may only read or write declared
1819
MSRs, on MSHV and WHP this is not enforced. by @ludfjig in https://github.com/hyperlight-dev/hyperlight/pull/991
1920
* **Breaking:** Filesystem paths are now represented using `PathBuf`. `GuestBinary::FilePath` now stores a `PathBuf` instead of a `String`, and `MultiUseSandbox::generate_crashdump_to_dir` accepts `Into<PathBuf>` instead of `Into<String>`. Callers passing a `String` to `GuestBinary::FilePath` must convert it using `.into()`.
21+
* Deprecate `MultiUseSandbox::poisoned` in favor of `MultiUseSandbox::status().is_poisoned()`.
2022

2123
Certain fixed guest addresses were changed on AArch64 to more easily
2224
accommodate 16k pages without wasting memory. Snapshots taken from
@@ -26,6 +28,7 @@ hyperlight versions.
2628
### Removed
2729

2830
### Fixed
31+
* Mark a sandbox unrecoverable in rare cases when snapshot restore fails while updating its VM mappings.
2932
* Fix symbol resolution in guest core dumps for sandboxes created from snapshots by @ludfjig in https://github.com/hyperlight-dev/hyperlight/pull/1618
3033
* Reject malformed OCI snapshot metadata and non-regular artifact files during load.
3134
* Reset XCR0 during x86 snapshot restore.

‎src/hyperlight_host/src/error.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,10 @@ pub enum HyperlightError {
201201
#[error("The sandbox was poisoned")]
202202
PoisonedSandbox,
203203

204+
/// The sandbox cannot safely perform further operations and must be discarded.
205+
#[error("The sandbox is unrecoverable and must be discarded")]
206+
UnrecoverableSandbox,
207+
204208
/// Raw pointer is less than base address
205209
#[error("Raw pointer ({0:?}) was less than the base address ({1})")]
206210
RawPointerLessThanBaseAddress(RawPtr, u64),
@@ -383,6 +387,7 @@ impl HyperlightError {
383387
| HyperlightError::UnexpectedNoOfArguments(_, _)
384388
| HyperlightError::UnexpectedParameterValueType(_, _)
385389
| HyperlightError::UnexpectedReturnValueType(_, _)
390+
| HyperlightError::UnrecoverableSandbox
386391
| HyperlightError::UTF8StringConversionFailure(_)
387392
| HyperlightError::VectorCapacityIncorrect(_, _, _)
388393
| HyperlightError::SharedMemory(_) => false,

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ mod x86_64;
1919

2020
#[cfg(target_arch = "aarch64")]
2121
mod aarch64;
22+
23+
#[cfg(all(test, not(gdb)))]
24+
pub(crate) mod test_support;
25+
2226
#[cfg(gdb)]
2327
use std::collections::HashMap;
2428
use std::str::FromStr;
@@ -532,11 +536,14 @@ impl HyperlightVm {
532536
let guest_base = crate::mem::layout::SandboxMemoryLayout::BASE_ADDRESS as u64;
533537
let rgn = snapshot.mapping_at(guest_base, MemoryRegionType::Snapshot);
534538

535-
if let Some(old_snapshot) = self.snapshot_memory.replace(snapshot) {
539+
// Keep the backing memory alive until its VM mapping is removed.
540+
if let Some(old_snapshot) = self.snapshot_memory.as_ref() {
536541
let old_rgn = old_snapshot.mapping_at(guest_base, MemoryRegionType::Snapshot);
537542
self.vm.unmap_memory((self.snapshot_slot, &old_rgn))?;
538543
}
544+
self.snapshot_memory = None;
539545
unsafe { self.vm.map_memory((self.snapshot_slot, &rgn))? };
546+
self.snapshot_memory = Some(snapshot);
540547

541548
Ok(())
542549
}
@@ -549,12 +556,15 @@ impl HyperlightVm {
549556
let guest_base = hyperlight_common::layout::scratch_base_gpa(scratch.mem_size());
550557
let rgn = scratch.mapping_at(guest_base, MemoryRegionType::Scratch);
551558

552-
if let Some(old_scratch) = self.scratch_memory.replace(scratch) {
559+
// Keep the backing memory alive until its VM mapping is removed.
560+
if let Some(old_scratch) = self.scratch_memory.as_ref() {
553561
let old_base = hyperlight_common::layout::scratch_base_gpa(old_scratch.mem_size());
554562
let old_rgn = old_scratch.mapping_at(old_base, MemoryRegionType::Scratch);
555563
self.vm.unmap_memory((self.scratch_slot, &old_rgn))?;
556564
}
565+
self.scratch_memory = None;
557566
unsafe { self.vm.map_memory((self.scratch_slot, &rgn))? };
567+
self.scratch_memory = Some(scratch);
558568

559569
Ok(())
560570
}
Lines changed: 291 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,291 @@
1+
/*
2+
Copyright 2025 The Hyperlight Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
use std::collections::VecDeque;
18+
19+
use super::*;
20+
#[cfg(target_arch = "x86_64")]
21+
use crate::hypervisor::regs::MsrEntry;
22+
use crate::hypervisor::regs::{
23+
CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters,
24+
};
25+
#[cfg(target_arch = "x86_64")]
26+
use crate::hypervisor::virtual_machine::CreateVmError;
27+
use crate::hypervisor::virtual_machine::{HypervisorError, VirtualMachine};
28+
29+
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
30+
pub(crate) enum VmOperation {
31+
Map(MemoryRegionType),
32+
Unmap(MemoryRegionType),
33+
#[cfg(target_arch = "x86_64")]
34+
SetRegs,
35+
#[cfg(target_arch = "x86_64")]
36+
SetDebugRegs,
37+
#[cfg(target_arch = "x86_64")]
38+
ResetXsave,
39+
#[cfg(target_arch = "x86_64")]
40+
SetSregs,
41+
#[cfg(target_arch = "x86_64")]
42+
SetMsrs,
43+
#[cfg(target_arch = "aarch64")]
44+
ResetVcpu,
45+
}
46+
47+
#[derive(Clone, Debug)]
48+
pub(crate) struct VmFaultPlan {
49+
operations: Arc<Mutex<VecDeque<VmOperation>>>,
50+
}
51+
52+
impl VmFaultPlan {
53+
fn new(operations: impl IntoIterator<Item = VmOperation>) -> Self {
54+
Self {
55+
operations: Arc::new(Mutex::new(operations.into_iter().collect())),
56+
}
57+
}
58+
59+
pub(crate) fn is_consumed(&self) -> bool {
60+
self.operations.lock().unwrap().is_empty()
61+
}
62+
63+
fn should_fail(&self, operation: VmOperation) -> bool {
64+
let mut operations = self.operations.lock().unwrap();
65+
if operations.front() == Some(&operation) {
66+
operations.pop_front();
67+
true
68+
} else {
69+
false
70+
}
71+
}
72+
}
73+
74+
#[derive(Debug)]
75+
struct FaultInjectingVirtualMachine {
76+
inner: Option<Box<dyn VirtualMachine>>,
77+
fault_plan: VmFaultPlan,
78+
}
79+
80+
impl FaultInjectingVirtualMachine {
81+
fn new(
82+
inner: Box<dyn VirtualMachine>,
83+
operations: impl IntoIterator<Item = VmOperation>,
84+
) -> (Self, VmFaultPlan) {
85+
let fault_plan = VmFaultPlan::new(operations);
86+
(
87+
Self {
88+
inner: Some(inner),
89+
fault_plan: fault_plan.clone(),
90+
},
91+
fault_plan,
92+
)
93+
}
94+
95+
fn placeholder() -> Self {
96+
Self {
97+
inner: None,
98+
fault_plan: VmFaultPlan::new([]),
99+
}
100+
}
101+
102+
fn inner(&self) -> &dyn VirtualMachine {
103+
self.inner.as_deref().expect("placeholder VM was used")
104+
}
105+
106+
fn inner_mut(&mut self) -> &mut dyn VirtualMachine {
107+
self.inner.as_deref_mut().expect("placeholder VM was used")
108+
}
109+
110+
fn should_fail(&self, operation: VmOperation) -> bool {
111+
self.fault_plan.should_fail(operation)
112+
}
113+
114+
fn injected_error() -> HypervisorError {
115+
HypervisorError::Injected
116+
}
117+
}
118+
119+
impl VirtualMachine for FaultInjectingVirtualMachine {
120+
unsafe fn map_memory(
121+
&mut self,
122+
region: (u32, &MemoryRegion),
123+
) -> std::result::Result<(), MapMemoryError> {
124+
if self.should_fail(VmOperation::Map(region.1.region_type)) {
125+
return Err(MapMemoryError::Hypervisor(Self::injected_error()));
126+
}
127+
// SAFETY: The decorator forwards the caller's preconditions unchanged.
128+
unsafe { self.inner_mut().map_memory(region) }
129+
}
130+
131+
fn unmap_memory(
132+
&mut self,
133+
region: (u32, &MemoryRegion),
134+
) -> std::result::Result<(), UnmapMemoryError> {
135+
if self.should_fail(VmOperation::Unmap(region.1.region_type)) {
136+
return Err(UnmapMemoryError::Hypervisor(Self::injected_error()));
137+
}
138+
self.inner_mut().unmap_memory(region)
139+
}
140+
141+
fn run_vcpu(
142+
&mut self,
143+
#[cfg(feature = "trace_guest")] tc: &mut crate::sandbox::trace::TraceContext,
144+
) -> std::result::Result<VmExit, RunVcpuError> {
145+
self.inner_mut().run_vcpu(
146+
#[cfg(feature = "trace_guest")]
147+
tc,
148+
)
149+
}
150+
151+
fn regs(&self) -> std::result::Result<CommonRegisters, RegisterError> {
152+
self.inner().regs()
153+
}
154+
155+
fn set_regs(&mut self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> {
156+
#[cfg(target_arch = "x86_64")]
157+
if self.should_fail(VmOperation::SetRegs) {
158+
return Err(RegisterError::SetRegs(Self::injected_error()));
159+
}
160+
self.inner_mut().set_regs(regs)
161+
}
162+
163+
fn fpu(&self) -> std::result::Result<CommonFpu, RegisterError> {
164+
self.inner().fpu()
165+
}
166+
167+
fn set_fpu(&mut self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError> {
168+
self.inner_mut().set_fpu(fpu)
169+
}
170+
171+
fn sregs(&self) -> std::result::Result<CommonSpecialRegisters, RegisterError> {
172+
self.inner().sregs()
173+
}
174+
175+
fn set_sregs(
176+
&mut self,
177+
sregs: &CommonSpecialRegisters,
178+
) -> std::result::Result<(), RegisterError> {
179+
#[cfg(target_arch = "x86_64")]
180+
if self.should_fail(VmOperation::SetSregs) {
181+
return Err(RegisterError::SetSregs(Self::injected_error()));
182+
}
183+
self.inner_mut().set_sregs(sregs)
184+
}
185+
186+
fn debug_regs(&self) -> std::result::Result<CommonDebugRegs, RegisterError> {
187+
self.inner().debug_regs()
188+
}
189+
190+
fn set_debug_regs(&self, drs: &CommonDebugRegs) -> std::result::Result<(), RegisterError> {
191+
#[cfg(target_arch = "x86_64")]
192+
if self.should_fail(VmOperation::SetDebugRegs) {
193+
return Err(RegisterError::SetDebugRegs(Self::injected_error()));
194+
}
195+
self.inner().set_debug_regs(drs)
196+
}
197+
198+
#[cfg(target_arch = "x86_64")]
199+
fn msrs(&self, indices: &[u32]) -> std::result::Result<Vec<MsrEntry>, RegisterError> {
200+
self.inner().msrs(indices)
201+
}
202+
203+
#[cfg(target_arch = "x86_64")]
204+
fn set_msrs(&self, msrs: &[MsrEntry]) -> std::result::Result<(), RegisterError> {
205+
if self.should_fail(VmOperation::SetMsrs) {
206+
return Err(RegisterError::SetMsrs(Self::injected_error()));
207+
}
208+
self.inner().set_msrs(msrs)
209+
}
210+
211+
#[cfg(target_arch = "x86_64")]
212+
fn msr_reset_indices(
213+
&self,
214+
guest_msrs: &[u32],
215+
) -> std::result::Result<Vec<u32>, CreateVmError> {
216+
self.inner().msr_reset_indices(guest_msrs)
217+
}
218+
219+
#[cfg(not(target_arch = "aarch64"))]
220+
fn xsave(&self) -> std::result::Result<Vec<u8>, RegisterError> {
221+
self.inner().xsave()
222+
}
223+
224+
#[cfg(not(target_arch = "aarch64"))]
225+
fn reset_xsave(&self) -> std::result::Result<(), RegisterError> {
226+
#[cfg(target_arch = "x86_64")]
227+
if self.should_fail(VmOperation::ResetXsave) {
228+
return Err(RegisterError::SetXsave(Self::injected_error()));
229+
}
230+
self.inner().reset_xsave()
231+
}
232+
233+
#[cfg(not(target_arch = "aarch64"))]
234+
fn set_xsave(&self, xsave: &[u32]) -> std::result::Result<(), RegisterError> {
235+
self.inner().set_xsave(xsave)
236+
}
237+
238+
#[cfg(all(test, target_arch = "x86_64"))]
239+
fn xcr0(&self) -> std::result::Result<u64, RegisterError> {
240+
self.inner().xcr0()
241+
}
242+
243+
#[cfg(target_arch = "x86_64")]
244+
fn set_xcr0(&self, value: u64) -> std::result::Result<(), RegisterError> {
245+
self.inner().set_xcr0(value)
246+
}
247+
248+
#[cfg(target_arch = "aarch64")]
249+
fn can_reset_vcpu(&self) -> bool {
250+
self.inner().can_reset_vcpu()
251+
}
252+
253+
#[cfg(target_arch = "aarch64")]
254+
fn reset_vcpu(&mut self) -> std::result::Result<(), ResetVcpuError> {
255+
if self.should_fail(VmOperation::ResetVcpu) {
256+
return Err(ResetVcpuError::Hypervisor(Self::injected_error()));
257+
}
258+
self.inner_mut().reset_vcpu()
259+
}
260+
261+
#[cfg(target_os = "windows")]
262+
fn partition_handle(&self) -> windows::Win32::System::Hypervisor::WHV_PARTITION_HANDLE {
263+
self.inner().partition_handle()
264+
}
265+
}
266+
267+
impl HyperlightVm {
268+
pub(crate) fn inject_vm_faults(
269+
&mut self,
270+
operations: impl IntoIterator<Item = VmOperation>,
271+
) -> VmFaultPlan {
272+
let placeholder = Box::new(FaultInjectingVirtualMachine::placeholder());
273+
let inner = std::mem::replace(&mut self.vm, placeholder);
274+
let (vm, fault_plan) = FaultInjectingVirtualMachine::new(inner, operations);
275+
self.vm = Box::new(vm);
276+
fault_plan
277+
}
278+
279+
#[allow(clippy::type_complexity, reason = "test-only mapping state")]
280+
pub(crate) fn base_mapping_state(&self) -> (Option<(usize, usize)>, Option<(usize, usize)>) {
281+
let snapshot = self
282+
.snapshot_memory
283+
.as_ref()
284+
.map(|memory| (memory.base_addr(), memory.mem_size()));
285+
let scratch = self
286+
.scratch_memory
287+
.as_ref()
288+
.map(|memory| (memory.base_addr(), memory.mem_size()));
289+
(snapshot, scratch)
290+
}
291+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,9 @@ pub enum UnmapMemoryError {
390390
/// Implementation-specific Hypervisor error
391391
#[derive(Debug, Clone, thiserror::Error)]
392392
pub enum HypervisorError {
393+
#[cfg(test)]
394+
#[error("Injected hypervisor error")]
395+
Injected,
393396
#[cfg(kvm)]
394397
#[error("KVM error: {0}")]
395398
KvmError(#[from] kvm_ioctls::Error),

‎src/hyperlight_host/src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ pub use hypervisor::virtual_machine::is_hypervisor_present;
8989
/// A sandbox that can call be used to make multiple calls to guest functions,
9090
/// and otherwise reused multiple times
9191
pub use sandbox::MultiUseSandbox;
92+
/// The lifecycle state of a [`MultiUseSandbox`].
93+
pub use sandbox::SandboxStatus;
9294
/// The re-export for the `UninitializedSandbox` type
9395
pub use sandbox::UninitializedSandbox;
9496
/// The re-export for the `SandboxBuilder` type

0 commit comments

Comments
 (0)