Skip to content

Commit f857c5d

Browse files
committed
Ensure sandboxes can only restore to snapshots taken from the same sandbox
Signed-off-by: Ludvig Liljenberg <[email protected]>
1 parent 93612a2 commit f857c5d

File tree

4 files changed

+69
-4
lines changed

4 files changed

+69
-4
lines changed

src/hyperlight_host/src/error.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,10 @@ pub enum HyperlightError {
221221
#[cfg(all(feature = "seccomp", target_os = "linux"))]
222222
SeccompFilterError(#[from] seccompiler::Error),
223223

224+
/// Tried to restore snapshot to a sandbox that is not the same as the one the snapshot was taken from
225+
#[error("Snapshot was taken from a different sandbox")]
226+
SnapshotSandboxMismatch,
227+
224228
/// SystemTimeError
225229
#[error("SystemTimeError {0:?}")]
226230
SystemTimeError(#[from] SystemTimeError),

src/hyperlight_host/src/mem/mgr.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,9 +263,10 @@ where
263263
/// Create a snapshot with the given mapped regions
264264
pub(crate) fn snapshot(
265265
&mut self,
266+
sandbox_id: u64,
266267
mapped_regions: Vec<MemoryRegion>,
267268
) -> Result<SharedMemorySnapshot> {
268-
SharedMemorySnapshot::new(&mut self.shared_mem, mapped_regions)
269+
SharedMemorySnapshot::new(&mut self.shared_mem, sandbox_id, mapped_regions)
269270
}
270271

271272
/// This function restores a memory snapshot from a given snapshot.

src/hyperlight_host/src/mem/shared_mem_snapshot.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ use crate::Result;
2424
/// of the memory therein
2525
#[derive(Clone)]
2626
pub(crate) struct SharedMemorySnapshot {
27+
// Unique ID of the sandbox this snapshot was taken from
28+
sandbox_id: u64,
29+
// Memory of the sandbox at the time this snapshot was taken
2730
snapshot: Vec<u8>,
2831
/// The memory regions that were mapped when this snapshot was taken (excluding initial sandbox regions)
2932
regions: Vec<MemoryRegion>,
@@ -35,11 +38,16 @@ impl SharedMemorySnapshot {
3538
#[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")]
3639
pub(super) fn new<S: SharedMemory>(
3740
shared_mem: &mut S,
41+
sandbox_id: u64,
3842
regions: Vec<MemoryRegion>,
3943
) -> Result<Self> {
4044
// TODO: Track dirty pages instead of copying entire memory
4145
let snapshot = shared_mem.with_exclusivity(|e| e.copy_all_to_vec())??;
42-
Ok(Self { snapshot, regions })
46+
Ok(Self {
47+
sandbox_id,
48+
snapshot,
49+
regions,
50+
})
4351
}
4452

4553
/// Take another snapshot of the internally-stored `SharedMemory`,
@@ -59,6 +67,11 @@ impl SharedMemorySnapshot {
5967
Ok(())
6068
}
6169

70+
/// The id of the sandbox this snapshot was taken from.
71+
pub(crate) fn sandbox_id(&self) -> u64 {
72+
self.sandbox_id
73+
}
74+
6275
/// Get the mapped regions from this snapshot
6376
pub(crate) fn regions(&self) -> &[MemoryRegion] {
6477
&self.regions
@@ -84,7 +97,7 @@ mod tests {
8497
let data2 = data1.iter().map(|b| b + 1).collect::<Vec<u8>>();
8598
let mut gm = ExclusiveSharedMemory::new(PAGE_SIZE_USIZE).unwrap();
8699
gm.copy_from_slice(data1.as_slice(), 0).unwrap();
87-
let mut snap = super::SharedMemorySnapshot::new(&mut gm, Vec::new()).unwrap();
100+
let mut snap = super::SharedMemorySnapshot::new(&mut gm, 0, Vec::new()).unwrap();
88101
{
89102
// after the first snapshot is taken, make sure gm has the equivalent
90103
// of data1

src/hyperlight_host/src/sandbox/initialized_multi_use.rs

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use std::os::fd::AsRawFd;
2020
#[cfg(unix)]
2121
use std::os::linux::fs::MetadataExt;
2222
use std::path::Path;
23+
use std::sync::atomic::{AtomicU64, Ordering};
2324
use std::sync::{Arc, Mutex};
2425

2526
use hyperlight_common::flatbuffer_wrappers::function_call::{FunctionCall, FunctionCallType};
@@ -31,6 +32,7 @@ use tracing::{Span, instrument};
3132
use super::host_funcs::FunctionRegistry;
3233
use super::snapshot::Snapshot;
3334
use super::{Callable, MemMgrWrapper, WrapperGetter};
35+
use crate::HyperlightError::SnapshotSandboxMismatch;
3436
use crate::func::guest_err::check_for_guest_error;
3537
use crate::func::{ParameterTuple, SupportedReturnType};
3638
#[cfg(gdb)]
@@ -44,6 +46,9 @@ use crate::mem::shared_mem::HostSharedMemory;
4446
use crate::metrics::maybe_time_and_emit_guest_call;
4547
use crate::{HyperlightError, Result, log_then_return};
4648

49+
/// Global counter for assigning unique IDs to sandboxes
50+
static SANDBOX_ID_COUNTER: AtomicU64 = AtomicU64::new(0);
51+
4752
/// A sandbox that supports being used Multiple times.
4853
/// The implication of being used multiple times is two-fold:
4954
///
@@ -53,6 +58,8 @@ use crate::{HyperlightError, Result, log_then_return};
5358
/// 2. A MultiUseGuestCallContext can be created from the sandbox and used to make multiple guest function calls to the Sandbox.
5459
/// in this case the state of the sandbox is not reset until the context is finished and the `MultiUseSandbox` is returned.
5560
pub struct MultiUseSandbox {
61+
/// Unique identifier for this sandbox instance
62+
id: u64,
5663
// We need to keep a reference to the host functions, even if the compiler marks it as unused. The compiler cannot detect our dynamic usages of the host function in `HyperlightFunction::call`.
5764
pub(super) _host_funcs: Arc<Mutex<FunctionRegistry>>,
5865
pub(crate) mem_mgr: MemMgrWrapper<HostSharedMemory>,
@@ -77,6 +84,7 @@ impl MultiUseSandbox {
7784
#[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper,
7885
) -> MultiUseSandbox {
7986
Self {
87+
id: SANDBOX_ID_COUNTER.fetch_add(1, Ordering::Relaxed),
8088
_host_funcs: host_funcs,
8189
mem_mgr: mgr,
8290
vm,
@@ -91,7 +99,10 @@ impl MultiUseSandbox {
9199
pub fn snapshot(&mut self) -> Result<Snapshot> {
92100
let mapped_regions_iter = self.vm.get_mapped_regions();
93101
let mapped_regions_vec: Vec<MemoryRegion> = mapped_regions_iter.cloned().collect();
94-
let memory_snapshot = self.mem_mgr.unwrap_mgr_mut().snapshot(mapped_regions_vec)?;
102+
let memory_snapshot = self
103+
.mem_mgr
104+
.unwrap_mgr_mut()
105+
.snapshot(self.id, mapped_regions_vec)?;
95106
Ok(Snapshot {
96107
inner: memory_snapshot,
97108
})
@@ -100,6 +111,10 @@ impl MultiUseSandbox {
100111
/// Restore the sandbox's memory to the state captured in the given snapshot.
101112
#[instrument(err(Debug), skip_all, parent = Span::current())]
102113
pub fn restore(&mut self, snapshot: &Snapshot) -> Result<()> {
114+
if self.id != snapshot.inner.sandbox_id() {
115+
return Err(SnapshotSandboxMismatch);
116+
}
117+
103118
self.mem_mgr
104119
.unwrap_mgr_mut()
105120
.restore_snapshot(&snapshot.inner)?;
@@ -708,4 +723,36 @@ mod tests {
708723
err
709724
);
710725
}
726+
727+
#[test]
728+
fn snapshot_different_sandbox() {
729+
let mut sandbox = {
730+
let path = simple_guest_as_string().unwrap();
731+
let u_sbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap();
732+
u_sbox.evolve().unwrap()
733+
};
734+
735+
let mut sandbox2 = {
736+
let path = simple_guest_as_string().unwrap();
737+
let u_sbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap();
738+
u_sbox.evolve().unwrap()
739+
};
740+
assert_ne!(sandbox.id, sandbox2.id);
741+
742+
let snapshot = sandbox.snapshot().unwrap();
743+
let err = sandbox2.restore(&snapshot);
744+
assert!(matches!(err, Err(HyperlightError::SnapshotSandboxMismatch)));
745+
746+
let sandbox_id = sandbox.id;
747+
drop(sandbox);
748+
drop(sandbox2);
749+
drop(snapshot);
750+
751+
let sandbox3 = {
752+
let path = simple_guest_as_string().unwrap();
753+
let u_sbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap();
754+
u_sbox.evolve().unwrap()
755+
};
756+
assert_ne!(sandbox3.id, sandbox_id);
757+
}
711758
}

0 commit comments

Comments
 (0)