From bf60b31d1213922cf533a354800629418ef222dc Mon Sep 17 00:00:00 2001 From: danbugs Date: Fri, 16 Oct 2026 16:27:25 +0000 Subject: [PATCH 1/2] feat!: place the transport arena at a fixed scratch address The guest builds its transport context once and keeps the ring addresses. For scratch to change size across a snapshot, the transport arena (virtqueue rings, pools and the checkpoint mailbox) needs an address that does not depend on the scratch size. The arena ends one guard page below the reserved top pages. The page tables start at the scratch base. The guard page is left unmapped, so an amd64 exception stack overrun faults there before it reaches the arena. aarch64 exception vectors store below SP before any check, so nested faults there can step past it. The guest's page allocator stops at the arena GPA the host publishes, and aborts if that is not between the scratch base and the end of the transport range. The guest's transport memory access is bound to the arena, which must start above the scratch base. scratch_allocator_limit_gpa is renamed scratch_reserved_top_gpa, so a guest that stops its allocator at the old limit fails to compile. The minimum scratch size grows by the guard page. This changes the snapshot layout: snapshot ABI 7, goldens v7.0. Existing snapshots must be regenerated, and guests rebuilt. Signed-off-by: danbugs --- CHANGELOG.md | 2 + docs/paging-development-notes.md | 25 ++-- docs/snapshot-versioning.md | 4 + docs/virtio-host-guest-communication.md | 25 ++-- src/hyperlight_common/src/layout.rs | 33 ++++- .../src/arch/aarch64/prim_alloc.rs | 4 +- .../src/arch/amd64/prim_alloc.rs | 4 +- src/hyperlight_guest/src/layout.rs | 27 ++++ src/hyperlight_guest/src/transport/context.rs | 64 +++++++-- src/hyperlight_guest/src/transport/mem.rs | 24 ++-- src/hyperlight_host/src/mem/layout.rs | 136 ++++++++++++------ src/hyperlight_host/src/mem/mgr.rs | 14 +- src/hyperlight_host/src/mem/virtq/mod.rs | 8 +- src/hyperlight_host/src/mem/virtq/tests.rs | 19 +-- .../src/sandbox/initialized.rs | 20 ++- .../src/sandbox/snapshot/file/media_types.rs | 2 +- .../src/sandbox/snapshot/file_tests.rs | 2 +- .../src/sandbox/snapshot/mod.rs | 52 +++++-- .../src/sandbox/snapshot/tripwires.rs | 2 +- .../tests/snapshot_goldens/goldens_version.rs | 2 +- 20 files changed, 330 insertions(+), 139 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2cbb54ef5..2f18394c84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). * **Breaking:** Generated libc bindings use `usize` for `size_t` arguments and return values, including `strlen` and `memcpy`. Rust callers that use `u64` or `c_ulong` need explicit conversions. The C ABI is unchanged. +* **Breaking:** The transport arena sits one unmapped guard page below the reserved scratch-top pages, at an address independent of the scratch size, and the page tables at the scratch base. The guest's page allocator stops at the arena GPA the host publishes in the scratch-top metadata, so guests must be rebuilt. `hyperlight_common::layout::scratch_allocator_limit_gpa` is renamed `scratch_reserved_top_gpa`, the start of the reserved pages. An allocator that stops there runs into the arena. The minimum scratch size grows by that guard page. Snapshot ABI 7: existing snapshots must be regenerated. + ### Removed ### Fixed diff --git a/docs/paging-development-notes.md b/docs/paging-development-notes.md index 7d2bbbdcd2..06be5e4671 100644 --- a/docs/paging-development-notes.md +++ b/docs/paging-development-notes.md @@ -64,7 +64,8 @@ original virtual address to point to the new page. The page table entries to do this will likely need to be copied themselves, and so a ready supply of already-mapped scratch pages to use for replacement page tables is needed. Currently, the guest accomplishes this by -keeping an identity mapping of the entire scratch memory around. +keeping an identity mapping of scratch memory around, except one guard +page below the reserved top pages. The host and the guest need to agree on the location of this mapping, so that (a) the host can create it when first setting up a blank guest @@ -139,14 +140,20 @@ calls, i.e. there may be no calls in flight at the time of snapshotting. This is not enforced, but odd things may happen if it is violated. -Host and guest calls use two virtqueues in a fixed transport arena at -the bottom of scratch. The arena contains both rings and their -fixed-slot buffer pools. Copied page tables follow the arena. Dynamic -scratch allocations begin after the copied page tables. - -The minimum scratch size is calculated by `min_scratch_size()` in the -architecture-specific layout modules under `hyperlight_common`; see -that function for the detailed breakdown of required overhead. +Host function calls and guest function calls use two virtqueues in a +fixed transport arena below the reserved scratch-top pages. One +unmapped guard page between them makes an amd64 exception stack +overrun fault before it reaches the arena. On aarch64 nested faults +lower SP past it, since the exception vectors store below SP before +any check. The arena's address does not depend on the scratch size. +The arena contains both rings and their fixed-slot buffer pools. +Copied page tables start at the scratch base. Dynamic scratch +allocations begin after the copied page tables and stop at the arena. + +The minimum scratch size is the architecture's fixed overhead +(`min_scratch_size()` in the layout modules under `hyperlight_common`, +which breaks it down) plus the transport arena and the guard page above +it (`SandboxMemoryLayout::min_scratch`), plus the copied page tables. ## Creating a fresh guest diff --git a/docs/snapshot-versioning.md b/docs/snapshot-versioning.md index b86f72ca00..6ac61c31a9 100644 --- a/docs/snapshot-versioning.md +++ b/docs/snapshot-versioning.md @@ -41,6 +41,10 @@ A snapshot carries four independently evolvable version markers: ABI 6 requires checkpoint readiness after guest initialization and permits H2G prefill to omit retained slots. Earlier snapshots must be regenerated. +ABI 7 places the transport arena below the reserved scratch-top pages, at an +address independent of the scratch size, and the page tables at the scratch +base. Earlier snapshots must be regenerated. + The `OCI_LAYOUT_VERSION` constant is pinned by the OCI image-layout spec at `1.0.0`. diff --git a/docs/virtio-host-guest-communication.md b/docs/virtio-host-guest-communication.md index 1b4b6a2e60..d2b7362138 100644 --- a/docs/virtio-host-guest-communication.md +++ b/docs/virtio-host-guest-communication.md @@ -42,16 +42,17 @@ optional writable response capacity. ## Transport arena -Both rings, the transport mailbox, and both pools occupy one fixed prefix of -guest scratch memory. +Both rings, the transport mailbox, and both pools occupy one fixed range of +guest scratch memory, below the reserved scratch-top pages with one unmapped +guard page between them. Its address does not depend on the scratch size. +Guest page allocations stop at its base. ```text - scratch base - | - v - +----------+-----+----------+-----+-----+-----+----------+----------+ - | G2H ring | pad | H2G ring | pad | mbx | pad | G2H pool | H2G pool | - +----------+-----+----------+-----+-----+-----+----------+----------+ + reserved top pages + | + +----------+-----+----------+-----+-----+-----+----------+----------+-------+ + | G2H ring | pad | H2G ring | pad | mbx | pad | G2H pool | H2G pool | guard | + +----------+-----+----------+-----+-----+-----+----------+----------+-------+ ``` The host derives this layout from `SandboxConfiguration`. Ring starts follow @@ -365,8 +366,8 @@ Guest `SlotPool` instances own all transport buffers. Pool clones share one allocation bitmap with each producer. Each producer pairs its backend with that pool. Completion leases carry the original slot addresses and full capacities. Backing owns alias allocation and -paging. The backend holds only scratch bounds. -Guest completion mapping checks scratch bounds. Allocation ownership and +paging. The backend holds only the transport arena's bounds. +Guest completion mapping checks those bounds. Allocation ownership and initialized lengths follow the `BufferMap` safety contract. ```text @@ -527,8 +528,8 @@ both consumers. ## Placement and relocation limitations The fixed-pool runtime places both rings, the mailbox, and both pools in one -host-owned arena at the scratch base. The guest reconstructs that layout from -host metadata. +host-owned arena below the reserved scratch-top pages. The guest reconstructs +that layout from host metadata. Descriptors, pool owners, and producer state contain absolute GVAs. Restore adopts the snapshot's scratch size, queue geometry, and transport addresses, diff --git a/src/hyperlight_common/src/layout.rs b/src/hyperlight_common/src/layout.rs index 3d4c23302a..7305a6accd 100644 --- a/src/hyperlight_common/src/layout.rs +++ b/src/hyperlight_common/src/layout.rs @@ -113,11 +113,35 @@ const _: () = { assert!((SCRATCH_TOP_GPA as u64) < VIRTQ_BUFFER_GVA_END - VIRTQ_BUFFER_GVA_START); }; -/// Exclusive upper GPA boundary for dynamic scratch allocations. -pub const fn scratch_allocator_limit_gpa() -> u64 { +/// Unmapped pages between the transport arena and the reserved top pages. +/// On amd64 an exception stack overrun faults in them before it reaches the +/// arena. aarch64 exception vectors lower SP before their first store, so +/// nested faults there can step past them. +pub const SCRATCH_TRANSPORT_GAP_PAGES: usize = 1; + +/// Start of the reserved pages at the top of scratch. The transport arena +/// sits [`SCRATCH_TRANSPORT_GAP_PAGES`] below it. A guest's page allocator +/// stops at the arena GPA the host publishes. +pub const fn scratch_reserved_top_gpa() -> u64 { (SCRATCH_TOP_GPA + 1 - SCRATCH_TOP_RESERVED_PAGES * crate::vmem::PAGE_SIZE) as u64 } +/// [`scratch_reserved_top_gpa`] in the scratch mapping. +pub const fn scratch_reserved_top_gva() -> u64 { + SCRATCH_TOP_GVA as u64 + 1 - (SCRATCH_TOP_RESERVED_PAGES * crate::vmem::PAGE_SIZE) as u64 +} + +/// Exclusive end of the transport arena: the reserved top pages, less the +/// guard pages between them. +pub const fn scratch_transport_end_gpa() -> u64 { + scratch_reserved_top_gpa() - (SCRATCH_TRANSPORT_GAP_PAGES * crate::vmem::PAGE_SIZE) as u64 +} + +/// [`scratch_transport_end_gpa`] in the scratch mapping. +pub const fn scratch_transport_end_gva() -> u64 { + scratch_reserved_top_gva() - (SCRATCH_TRANSPORT_GAP_PAGES * crate::vmem::PAGE_SIZE) as u64 +} + pub fn scratch_base_gpa(size: usize) -> u64 { (SCRATCH_TOP_GPA - size + 1) as u64 } @@ -127,8 +151,9 @@ pub const fn scratch_base_gva(size: usize) -> u64 { /// Compute fixed scratch overhead for a sandbox, saturating at [`usize::MAX`]. /// -/// `transport_len` covers rings and buffer pools. Snapshot page tables and -/// runtime allocations need additional scratch. +/// `transport_len` covers the transport arena (rings, mailbox and buffer +/// pools) and the [`SCRATCH_TRANSPORT_GAP_PAGES`] above it. Snapshot page +/// tables and runtime allocations need additional scratch. pub fn min_scratch_size(transport_len: usize) -> usize { arch::min_scratch_size() .and_then(|fixed| fixed.checked_add(transport_len)) diff --git a/src/hyperlight_guest/src/arch/aarch64/prim_alloc.rs b/src/hyperlight_guest/src/arch/aarch64/prim_alloc.rs index bc3147f73b..e69f6e1e8c 100644 --- a/src/hyperlight_guest/src/arch/aarch64/prim_alloc.rs +++ b/src/hyperlight_guest/src/arch/aarch64/prim_alloc.rs @@ -2,7 +2,7 @@ // Copyright 2025 The Hyperlight Authors. use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode; -use hyperlight_common::{layout, vmem}; +use hyperlight_common::vmem; // There are no notable architecture-specific safety considerations // here, and the general conditions are documented in the @@ -22,7 +22,7 @@ pub unsafe fn alloc_phys_pages(n: u64) -> u64 { prev_base = out(reg) prev_base, ); } - let limit = layout::scratch_allocator_limit_gpa(); + let limit = crate::layout::allocator_limit_gpa(); if prev_base.checked_add(nbytes).is_none_or(|end| end > limit) { unsafe { crate::exit::abort_with_code_and_message( diff --git a/src/hyperlight_guest/src/arch/amd64/prim_alloc.rs b/src/hyperlight_guest/src/arch/amd64/prim_alloc.rs index bc3bf6f97f..a75ace5925 100644 --- a/src/hyperlight_guest/src/arch/amd64/prim_alloc.rs +++ b/src/hyperlight_guest/src/arch/amd64/prim_alloc.rs @@ -2,7 +2,7 @@ // Copyright 2025 The Hyperlight Authors. use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode; -use hyperlight_common::{layout, vmem}; +use hyperlight_common::vmem; // There are no notable architecture-specific safety considerations // here, and the general conditions are documented in the @@ -19,7 +19,7 @@ pub unsafe fn alloc_phys_pages(n: u64) -> u64 { x = inout(reg) x ); } - let limit = layout::scratch_allocator_limit_gpa(); + let limit = crate::layout::allocator_limit_gpa(); if x.checked_add(nbytes).is_none_or(|end| end > limit) { unsafe { crate::exit::abort_with_code_and_message( diff --git a/src/hyperlight_guest/src/layout.rs b/src/hyperlight_guest/src/layout.rs index 0b340e2117..1686c4f2b6 100644 --- a/src/hyperlight_guest/src/layout.rs +++ b/src/hyperlight_guest/src/layout.rs @@ -52,3 +52,30 @@ pub fn guest_log_level_gva() -> *mut u64 { (SCRATCH_TOP_GVA as u64 - SCRATCH_TOP_GUEST_LOG_LEVEL_OFFSET + 1) as *mut u64 } pub use arch::{scratch_base_gpa, scratch_base_gva}; + +/// Exclusive upper GPA for the scratch page allocator: the transport arena, +/// which sits below the reserved top pages. +/// +/// Aborts if the host has not published an arena between the scratch base +/// and the end of the transport range, so allocations never reach the +/// rings. +pub fn allocator_limit_gpa() -> u64 { + // SAFETY: the host writes the arena GPA into scratch metadata before entry. + let arena = unsafe { transport_arena_gpa_gva().read_volatile() }; + if arena <= scratch_base_gpa() + || arena >= hyperlight_common::layout::scratch_transport_end_gpa() + { + // SAFETY: aborting is always sound. + unsafe { + crate::exit::abort_with_code_and_message( + &[ + hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode::MallocFailed + as u8, + ], + c"Transport arena not published between the scratch base and the transport end" + .as_ptr(), + ) + } + } + arena +} diff --git a/src/hyperlight_guest/src/transport/context.rs b/src/hyperlight_guest/src/transport/context.rs index 7794739cd5..7c56048c80 100644 --- a/src/hyperlight_guest/src/transport/context.rs +++ b/src/hyperlight_guest/src/transport/context.rs @@ -12,7 +12,7 @@ use hyperlight_common::flatbuffer_wrappers::function_types::{ FunctionCallResult, ParameterValue, ReturnType, ReturnValue, }; use hyperlight_common::flatbuffer_wrappers::util::estimate_flatbuffer_capacity; -use hyperlight_common::layout::SCRATCH_TOP_GVA; +use hyperlight_common::layout::scratch_transport_end_gva; use hyperlight_common::outb::OutBAction; use hyperlight_common::transport::{ EncodedMessage, ExternalValues, MailboxValue, MsgHeader, MsgKind, @@ -119,23 +119,28 @@ impl GuestContext { /// The caller must run in an initialized single-vCPU Hyperlight guest and /// exclusively own both queues, their pools, and the writable `u64` mailbox /// at `mbx_gva`. The mailbox must lie in scratch, disjoint from both queues - /// and pools. Scratch must remain mapped while the context or any returned - /// buffer views exist. + /// and pools. Both rings and pools must lie in the transport arena, which + /// starts at the G2H ring and ends below the reserved scratch-top pages, + /// with the pools after the rings. Scratch must remain mapped while the + /// context or any returned buffer views exist. pub unsafe fn new(g2h: QueueConfig, h2g: QueueConfig, mbx_gva: u64) -> Result { // SAFETY: Generation has been initialized by host. let generation = unsafe { layout::snapshot_generation_gva().read_volatile() }; let g2h_pool = g2h_pool(g2h.pool_gva, g2h.pool_pages, g2h.buffer_size) .with_context(|| "failed to create G2H pool")?; + // The G2H ring starts the transport arena. + let arena_gva = g2h.layout.desc_table_addr(); + let scratch_gva = layout::scratch_base_gva(); // SAFETY: The caller owns the pool's scratch pages. - let mem = unsafe { pool_mem(&g2h_pool) } + let mem = unsafe { pool_mem(&g2h_pool, arena_gva, scratch_gva) } .with_context(|| "failed to initialize G2H pool access")?; let g2h_producer = VirtqProducer::new(g2h.layout, mem, OutbNotifier, g2h_pool); let h2g_pool = h2g_pool(h2g.pool_gva, h2g.pool_pages, h2g.buffer_size) .with_context(|| "failed to create H2G slot pool")?; // SAFETY: The caller owns this disjoint pool. - let mem = unsafe { pool_mem(&h2g_pool) } + let mem = unsafe { pool_mem(&h2g_pool, arena_gva, scratch_gva) } .with_context(|| "failed to initialize H2G pool access")?; // H2G prefill supplies writable buffers for host-initiated messages. let h2g_producer = VirtqProducer::new(h2g.layout, mem, NoopNotifier, h2g_pool); @@ -548,24 +553,38 @@ fn pool_len(pages: usize) -> result::Result { .ok_or(AllocError::Overflow) } -/// Validate pool placement and bind scratch access. +/// Validate pool placement and bind access to the arena up to the pool's end. +/// +/// The bounds start at the arena, whose address does not depend on the +/// scratch size. The pools follow the rings, so a producer's rings and pool +/// lie in [arena, end of its pool). That range must start in scratch above +/// `scratch_gva`, where the copied page tables sit, and end below the guard +/// page under the reserved scratch-top pages. /// /// # Safety /// /// Run on the initialized guest vCPU and own the pool's scratch. -unsafe fn pool_mem(pool: &SlotPool) -> result::Result { - let scratch_gva = layout::scratch_base_gva(); - let scratch_end = SCRATCH_TOP_GVA as u64 + 1; +unsafe fn pool_mem( + pool: &SlotPool, + arena_gva: u64, + scratch_gva: u64, +) -> result::Result { + let limit = scratch_transport_end_gva(); let base = pool.base_addr(); let end = base + pool.byte_len() as u64; - if !base.is_multiple_of(PAGE_SIZE as u64) || base < scratch_gva || end > scratch_end { + if !arena_gva.is_multiple_of(PAGE_SIZE as u64) + || arena_gva <= scratch_gva + || !base.is_multiple_of(PAGE_SIZE as u64) + || base < arena_gva + || end > limit + { return Err(AllocError::InvalidArg); } - // SAFETY: The validated pool lies in live scratch on the serialized guest vCPU. - unsafe { Ok(GuestMemOps::new(scratch_gva, scratch_end)) } + // SAFETY: The validated pool lies in the arena on the serialized guest vCPU. + unsafe { Ok(GuestMemOps::new(arena_gva, end)) } } /// Build the uniform H2G pool. @@ -611,6 +630,27 @@ mod tests { use super::*; + #[test] + fn pool_mem_binds_the_arena_below_the_reserved_top() { + let limit = scratch_transport_end_gva(); + let page = PAGE_SIZE as u64; + let arena = limit - 8 * page; + let scratch = arena - 16 * page; + let pool = |base, pages| h2g_pool(base, pages, 256).unwrap(); + // SAFETY: only bounds are computed. No memory is touched. + unsafe { + assert!(pool_mem(&pool(arena + 4 * page, 4), arena, scratch).is_ok()); + // Into the guard page or the reserved pages. + assert!(pool_mem(&pool(arena + 4 * page, 5), arena, scratch).is_err()); + // Below the arena. + assert!(pool_mem(&pool(arena - page, 1), arena, scratch).is_err()); + // An arena below scratch, at its base, or not page aligned. + assert!(pool_mem(&pool(arena + 4 * page, 4), 0, scratch).is_err()); + assert!(pool_mem(&pool(arena + 4 * page, 4), scratch, scratch).is_err()); + assert!(pool_mem(&pool(arena + 4 * page, 4), arena + 8, scratch).is_err()); + } + } + fn encoded_message(external: &[u8]) -> EncodedMessage<'_> { let mut values = ExternalValues::new(); values.push_bytes(external).unwrap(); diff --git a/src/hyperlight_guest/src/transport/mem.rs b/src/hyperlight_guest/src/transport/mem.rs index 9d4deba611..d256e7e9ec 100644 --- a/src/hyperlight_guest/src/transport/mem.rs +++ b/src/hyperlight_guest/src/transport/mem.rs @@ -17,13 +17,14 @@ use super::backing; // Mark types not [`Send`] type NotThreadSafe = PhantomData<*mut ()>; -/// Bounded scratch memory access. +/// Memory access bounded to a range of scratch: the transport arena, for +/// the producers. /// -/// Copies reject buffers that overlap the accessed scratch range. +/// Copies reject buffers that overlap the accessed range. #[derive(Clone, Copy, Debug)] pub(crate) struct GuestMemOps { - scratch_gva: u64, - scratch_end: u64, + start: u64, + end: u64, _nts: NotThreadSafe, } @@ -32,24 +33,25 @@ pub(crate) struct GuestMemOps { pub struct GuestMemError; impl GuestMemOps { - /// Bind memory access to live scratch bounds. + /// Bind memory access to `[start, end)`, a live part of + /// scratch (the transport arena, for the producers). /// /// # Safety /// - /// Scratch must be live, initialized, and writable. + /// The range must be live, initialized, and writable scratch. /// All access and destruction must stay on the serialized guest vCPU. /// Serialize paging and share alias leaf entries across accessing roots. - pub(super) unsafe fn new(scratch_gva: u64, scratch_end: u64) -> Self { + pub(super) unsafe fn new(start: u64, end: u64) -> Self { Self { - scratch_gva, - scratch_end, + start, + end, _nts: PhantomData, } } fn ptr(&self, addr: u64, len: usize) -> Result<*mut u8, GuestMemError> { let end = addr.checked_add(len as u64).ok_or(GuestMemError)?; - if addr < self.scratch_gva || end > self.scratch_end { + if addr < self.start || end > self.end { return Err(GuestMemError); } Ok(addr as *mut u8) @@ -119,7 +121,7 @@ impl AsRef<[u8]> for GuestMapping { } } -// SAFETY: Every address is restricted to the scratch mapping. Payload +// SAFETY: Every address is restricted to the bound range of scratch. Payload // references rely on descriptor ownership, and ring flags use aligned atomics. unsafe impl MemOps for GuestMemOps { type Error = GuestMemError; diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index 6d22ef5f97..e9c9c07cb0 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -34,17 +34,20 @@ //! //! There is also a scratch region at the top of physical memory, //! which is mostly laid out as a large undifferentiated blob of -//! memory, although the transport arena and copied page tables have -//! fixed positions: +//! memory. The copied page tables start at the scratch base, which moves +//! with the scratch size. The transport arena's address does not depend +//! on the scratch size: //! //! +-------------------------------------------+ (top of physical memory) //! | Exception Stack, Metadata | -//! +-------------------------------------------+ (1 page below) +//! +-------------------------------------------+ (reserved pages below) +//! | Guard page (not in the guest page tables) | +//! +-------------------------------------------+ (SCRATCH_TRANSPORT_GAP_PAGES) +//! | Transport Arena | +//! +-------------------------------------------+ //! | Scratch Memory | //! +-------------------------------------------+ //! | Guest Page Tables | -//! +-------------------------------------------+ -//! | Transport Arena | //! +-------------------------------------------+ (scratch size) use std::fmt::Debug; @@ -353,10 +356,9 @@ impl SandboxMemoryLayout { .ok_or(MemoryRequestTooSmall(scratch_size, usize::MAX))?; let h2g_dims = QueueDims::new(h2g_queue_size, h2g_pool_pages) .ok_or(MemoryRequestTooSmall(scratch_size, usize::MAX))?; - let arena_base_gpa = hyperlight_common::layout::scratch_base_gpa(scratch_size); - let transport_arena = TransportArena::new(arena_base_gpa, g2h_dims, h2g_dims) + let transport_arena = Self::transport_arena_for(g2h_dims, h2g_dims) .ok_or(MemoryRequestTooSmall(scratch_size, usize::MAX))?; - let min_scratch_size = hyperlight_common::layout::min_scratch_size(transport_arena.size()); + let min_scratch_size = Self::min_scratch_for(&transport_arena); if scratch_size < min_scratch_size { return Err(MemoryRequestTooSmall(scratch_size, min_scratch_size)); } @@ -380,6 +382,45 @@ impl SandboxMemoryLayout { Ok(ret) } + /// Fixed scratch overhead: the arena and the guard page above it count + /// as transport memory. + fn min_scratch_for(arena: &TransportArena) -> usize { + hyperlight_common::layout::min_scratch_size( + arena + .size() + .saturating_add(hyperlight_common::layout::SCRATCH_TRANSPORT_GAP_PAGES * PAGE_SIZE), + ) + } + + /// Smallest scratch for this layout before page tables. + pub(crate) fn min_scratch(&self) -> usize { + Self::min_scratch_for(&self.transport_arena) + } + + /// The transport arena sits below the reserved top pages, at an + /// address that does not depend on the scratch size. + fn transport_arena_for(g2h: QueueDims, h2g: QueueDims) -> Option { + let len = TransportArena::new(0, g2h, h2g)?.size() as u64; + let base = hyperlight_common::layout::scratch_transport_end_gpa().checked_sub(len)?; + TransportArena::new(base, g2h, h2g) + } + + /// Scratch offset of the transport arena. + pub(crate) fn arena_scratch_offset(&self) -> usize { + debug_assert!( + self.transport_arena.base_addr() + > hyperlight_common::layout::scratch_base_gpa(self.scratch_size), + "the minimum scratch size keeps the arena above the scratch base" + ); + (self.transport_arena.base_addr() + - hyperlight_common::layout::scratch_base_gpa(self.scratch_size)) as usize + } + + /// Scratch offset of the checkpoint mailbox. + pub(crate) fn mbx_scratch_offset(&self) -> usize { + self.arena_scratch_offset() + self.transport_arena.mbx_offset() + } + pub(crate) fn heap_size(&self) -> usize { self.heap_size } @@ -454,9 +495,7 @@ impl SandboxMemoryLayout { /// `snapshot_size` (the guest-visible prefix of the blob) is an /// independent field and must be set separately. pub(crate) fn set_pt_size(&mut self, size: usize) -> Result<()> { - let min_fixed_scratch = - hyperlight_common::layout::min_scratch_size(self.transport_arena.size()); - let min_scratch = min_fixed_scratch.saturating_add(size); + let min_scratch = self.min_scratch().saturating_add(size); if self.scratch_size < min_scratch { return Err(MemoryRequestTooSmall(self.scratch_size, min_scratch)); } @@ -733,12 +772,12 @@ impl SandboxMemoryLayout { /// Offset from the beginning of the scratch region to the location /// where page tables are eagerly copied on restore. pub(crate) fn get_pt_base_scratch_offset(&self) -> usize { - self.transport_arena.size() + 0 } /// Base GPA to which the page tables are eagerly copied on restore. pub(crate) fn get_pt_base_gpa(&self) -> u64 { - self.transport_arena.end_addr() + hyperlight_common::layout::scratch_base_gpa(self.scratch_size) } /// First GPA available to the guest scratch allocator. @@ -746,7 +785,7 @@ impl SandboxMemoryLayout { self.get_pt_base_gpa() + self.pt_size.unwrap_or(0) as u64 } - /// Exact transport placement in the fixed scratch prefix. + /// Exact transport placement below the reserved scratch-top pages. pub(crate) fn get_transport_arena(&self) -> TransportArena { self.transport_arena } @@ -807,22 +846,26 @@ mod tests { } #[test] - fn transport_arena_starts_at_scratch_base() { + fn transport_arena_sits_below_the_reserved_top() { let cfg = SandboxConfiguration::default(); let mut layout = SandboxMemoryLayout::new(cfg, 4096, 0, None).unwrap(); let arena = layout.get_transport_arena(); let scratch_base = hyperlight_common::layout::scratch_base_gpa(layout.get_scratch_size()); - assert_eq!(arena.base_addr(), scratch_base); - assert_eq!(layout.get_pt_base_gpa(), arena.end_addr()); - assert_eq!(layout.get_pt_base_scratch_offset(), arena.size()); + assert_eq!( + arena.end_addr() + + (hyperlight_common::layout::SCRATCH_TRANSPORT_GAP_PAGES * PAGE_SIZE) as u64, + hyperlight_common::layout::scratch_reserved_top_gpa() + ); + assert_eq!(layout.get_pt_base_gpa(), scratch_base); + assert_eq!(layout.get_pt_base_scratch_offset(), 0); layout.set_pt_size(PAGE_SIZE).unwrap(); assert_eq!(layout.get_transport_arena(), arena); assert_eq!( layout.get_first_free_scratch_gpa(), - arena.end_addr() + PAGE_SIZE as u64 + scratch_base + PAGE_SIZE as u64 ); } @@ -830,8 +873,7 @@ mod tests { fn transport_memory_is_part_of_minimum_scratch_size() { let mut cfg = SandboxConfiguration::default(); let layout = SandboxMemoryLayout::new(cfg, 4096, 0, None).unwrap(); - let minimum = - hyperlight_common::layout::min_scratch_size(layout.get_transport_arena().size()); + let minimum = layout.min_scratch(); cfg.set_scratch_size(minimum); let mut layout = SandboxMemoryLayout::new(cfg, 4096, 0, None).unwrap(); @@ -982,23 +1024,23 @@ mod tests { pin_eq!(layout.get_scratch_size(), 0x30000); pin_eq!(layout.get_pt_size(), 0); - pin_eq!(layout.get_pt_base_scratch_offset(), 0x15000); + pin_eq!(layout.get_pt_base_scratch_offset(), 0); + // The transport arena sits below the reserved top pages, at an + // address that does not depend on the scratch size. The page + // tables start at the scratch base. With the `SCRATCH_TOP` pins + // above, these fix the absolute addresses. let arena = layout.get_transport_arena(); - let scratch_base_gpa = hyperlight_common::layout::scratch_base_gpa(0x30000); - pin_eq!(arena.g2h_ring_addr() - scratch_base_gpa, 0); - pin_eq!(arena.h2g_ring_addr() - scratch_base_gpa, 0x410); - pin_eq!(arena.mbx_addr() - scratch_base_gpa, 0x618); - pin_eq!(arena.g2h_pool_addr() - scratch_base_gpa, 0x1000); - pin_eq!(arena.h2g_pool_addr() - scratch_base_gpa, 0xd000); - pin_eq!(arena.end_addr() - scratch_base_gpa, 0x15000); - - // The transport arena sits at the scratch base. The page tables - // follow it. With the `SCRATCH_TOP` pins above, these fix the - // absolute addresses. + let scratch_top = hyperlight_common::layout::SCRATCH_TOP_GPA as u64 + 1; + pin_eq!(scratch_top - arena.g2h_ring_addr(), 0x22000); + pin_eq!(arena.h2g_ring_addr() - arena.g2h_ring_addr(), 0x410); + pin_eq!(arena.mbx_addr() - arena.g2h_ring_addr(), 0x618); + pin_eq!(arena.g2h_pool_addr() - arena.g2h_ring_addr(), 0x1000); + pin_eq!(arena.h2g_pool_addr() - arena.g2h_ring_addr(), 0xd000); + pin_eq!(scratch_top - arena.end_addr(), 0xd000); pin_eq!( - layout.get_pt_base_gpa() - hyperlight_common::layout::scratch_base_gpa(0x30000), - 0x15000 + layout.get_pt_base_gpa(), + hyperlight_common::layout::scratch_base_gpa(0x30000) ); // pt_size is zero here, so the first free scratch GPA equals // the page table base. @@ -1007,8 +1049,8 @@ mod tests { layout.get_pt_base_gpa() ); - // A second snapshot layout keeps the transport prefix fixed - // relative to its scratch base. + // A second snapshot layout, with a different scratch size, keeps + // the transport arena at the same address. let mut cfg = SandboxConfiguration::default(); cfg.set_heap_size(0x5000); cfg.set_scratch_size(0x40000); @@ -1027,20 +1069,20 @@ mod tests { pin_eq!(layout.get_scratch_size(), 0x40000); pin_eq!(layout.get_pt_size(), 0); - pin_eq!(layout.get_pt_base_scratch_offset(), 0x15000); + pin_eq!(layout.get_pt_base_scratch_offset(), 0); let arena = layout.get_transport_arena(); - let scratch_base_gpa = hyperlight_common::layout::scratch_base_gpa(0x40000); - pin_eq!(arena.g2h_ring_addr() - scratch_base_gpa, 0); - pin_eq!(arena.h2g_ring_addr() - scratch_base_gpa, 0x410); - pin_eq!(arena.mbx_addr() - scratch_base_gpa, 0x618); - pin_eq!(arena.g2h_pool_addr() - scratch_base_gpa, 0x1000); - pin_eq!(arena.h2g_pool_addr() - scratch_base_gpa, 0xd000); - pin_eq!(arena.end_addr() - scratch_base_gpa, 0x15000); + let scratch_top = hyperlight_common::layout::SCRATCH_TOP_GPA as u64 + 1; + pin_eq!(scratch_top - arena.g2h_ring_addr(), 0x22000); + pin_eq!(arena.h2g_ring_addr() - arena.g2h_ring_addr(), 0x410); + pin_eq!(arena.mbx_addr() - arena.g2h_ring_addr(), 0x618); + pin_eq!(arena.g2h_pool_addr() - arena.g2h_ring_addr(), 0x1000); + pin_eq!(arena.h2g_pool_addr() - arena.g2h_ring_addr(), 0xd000); + pin_eq!(scratch_top - arena.end_addr(), 0xd000); pin_eq!( - layout.get_pt_base_gpa() - hyperlight_common::layout::scratch_base_gpa(0x40000), - 0x15000 + layout.get_pt_base_gpa(), + hyperlight_common::layout::scratch_base_gpa(0x40000) ); pin_eq!( layout.get_first_free_scratch_gpa(), diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index 9705252114..c91c050b01 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -404,7 +404,7 @@ impl SandboxMemoryManager { if matches!(host_mgr.next_action, NextAction::Initialise(_)) { let cp = MailboxValue::CheckpointPending.as_bytes(); - let mbx = host_mgr.layout.get_transport_arena().mbx_offset(); + let mbx = host_mgr.layout.mbx_scratch_offset(); host_mgr.scratch_mem.copy_from_slice(cp, mbx)?; host_mgr.create_virtq_consumers()?; @@ -651,7 +651,7 @@ impl SandboxMemoryManager { /// /// The pending marker distinguishes a completed checkpoint from an early halt. pub(crate) fn begin_snapshot_checkpoint(&mut self) -> Result<()> { - let offset = self.layout.get_transport_arena().mbx_offset(); + let offset = self.layout.mbx_scratch_offset(); let cp = MailboxValue::CheckpointPending.as_bytes(); self.scratch_mem.copy_from_slice(cp, offset)?; @@ -674,7 +674,7 @@ impl SandboxMemoryManager { let status = self .scratch_mem - .read::(self.layout.get_transport_arena().mbx_offset()) + .read::(self.layout.mbx_scratch_offset()) .map_err(|error| HyperlightError::TransportError(error.to_string()))?; match MailboxValue::try_from(status) { @@ -757,7 +757,7 @@ impl SandboxMemoryManager { } else if matches!(snapshot.next_action(), NextAction::Initialise(_)) { self.scratch_mem.copy_from_slice( MailboxValue::CheckpointPending.as_bytes(), - self.layout.get_transport_arena().mbx_offset(), + self.layout.mbx_scratch_offset(), )?; self.create_virtq_consumers()?; } @@ -1219,7 +1219,7 @@ mod tests { let mut mgr = manager(&case); let buffer = case.h2g_desc(0).addr; - let mbx = mgr.layout.get_transport_arena().mbx_offset(); + let mbx = mgr.layout.mbx_scratch_offset(); let cp = MailboxValue::CheckpointComplete.as_bytes(); mgr.scratch_mem.copy_from_slice(cp, mbx).unwrap(); @@ -1259,7 +1259,7 @@ mod tests { let mut mgr = manager(&case); mgr.g2h_consumer = Some(case.g2h_consumer()); mgr.begin_snapshot_checkpoint().unwrap(); - let mbx = mgr.layout.get_transport_arena().mbx_offset(); + let mbx = mgr.layout.mbx_scratch_offset(); mgr.scratch_mem .copy_from_slice(MailboxValue::CheckpointComplete.as_bytes(), mbx) .unwrap(); @@ -1276,7 +1276,7 @@ mod tests { let queue = TestCase::new(); let mut mgr = manager(&queue); mgr.g2h_consumer = Some(queue.g2h_consumer()); - let mbx = mgr.layout.get_transport_arena().mbx_offset(); + let mbx = mgr.layout.mbx_scratch_offset(); mgr.scratch_mem.write::(mbx, status).unwrap(); let error = mgr.finish_snapshot_checkpoint().unwrap_err(); diff --git a/src/hyperlight_host/src/mem/virtq/mod.rs b/src/hyperlight_host/src/mem/virtq/mod.rs index c699306faa..d2616c81f0 100644 --- a/src/hyperlight_host/src/mem/virtq/mod.rs +++ b/src/hyperlight_host/src/mem/virtq/mod.rs @@ -140,7 +140,7 @@ impl VirtqSnapshot { scratch_mem: &HostSharedMemory, ) -> Result<(G2hConsumer, H2gConsumer)> { let (g2h_offset, h2g_offset) = ring_offsets(layout); - let mbx_offset = layout.get_transport_arena().mbx_offset(); + let mbx_offset = layout.mbx_scratch_offset(); let cp = MailboxValue::CheckpointComplete.as_bytes(); write_published_arena_gpa(scratch_mem, layout.get_transport_arena().base_addr())?; @@ -247,9 +247,7 @@ fn read_ring(scratch_mem: &HostSharedMemory, offset: usize, len: usize) -> Resul } fn ring_offsets(layout: &SandboxMemoryLayout) -> (usize, usize) { - let arena = layout.get_transport_arena(); - let scratch_base = hyperlight_common::layout::scratch_base_gpa(layout.get_scratch_size()); - let g2h_offset = (arena.base_addr() - scratch_base) as usize; - let (h2g_offset, ..) = arena.to_offsets(); + let g2h_offset = layout.arena_scratch_offset(); + let (h2g_offset, ..) = layout.get_transport_arena().to_offsets(); (g2h_offset, g2h_offset + h2g_offset) } diff --git a/src/hyperlight_host/src/mem/virtq/tests.rs b/src/hyperlight_host/src/mem/virtq/tests.rs index 0048fb79c0..e37fcbd8d4 100644 --- a/src/hyperlight_host/src/mem/virtq/tests.rs +++ b/src/hyperlight_host/src/mem/virtq/tests.rs @@ -32,12 +32,8 @@ impl TestCase { let layout = memory_layout(); let arena = layout.get_transport_arena(); let (g2h_layout, h2g_layout) = ring_layouts(&layout).unwrap(); - let to_gva = |gpa| { - hyperlight_common::layout::scratch_base_gva(SCRATCH_SIZE) - + (gpa - hyperlight_common::layout::scratch_base_gpa(SCRATCH_SIZE)) - }; - let g2h_pool_base = to_gva(arena.g2h_pool_addr()); - let h2g_pool_base = to_gva(arena.h2g_pool_addr()); + let g2h_pool_base = scratch_gva(arena.g2h_pool_addr()); + let h2g_pool_base = scratch_gva(arena.h2g_pool_addr()); let h2g = layout.get_h2g_queue_dims(); let mem = HostMemOps::new(&scratch); let buffer_size = layout.get_h2g_buffer_size(); @@ -91,6 +87,12 @@ impl TestCase { } } +/// The test scratch's GVA for `gpa`. +fn scratch_gva(gpa: u64) -> u64 { + hyperlight_common::layout::scratch_base_gva(SCRATCH_SIZE) + + (gpa - hyperlight_common::layout::scratch_base_gpa(SCRATCH_SIZE)) +} + pub(crate) fn memory_layout() -> SandboxMemoryLayout { let mut config = SandboxConfiguration::default(); config.set_scratch_size(SCRATCH_SIZE); @@ -138,7 +140,7 @@ fn snapshots_and_restores_rings() { let allocator = layout.get_first_free_scratch_gpa(); let allocator_offset = restored.mem_size() - SCRATCH_TOP_ALLOCATOR_OFFSET as usize; - let mbx_offset = layout.get_transport_arena().mbx_offset(); + let mbx_offset = layout.mbx_scratch_offset(); let cp = MailboxValue::CheckpointPending.as_bytes(); restored.write::(allocator_offset, allocator).unwrap(); @@ -323,7 +325,8 @@ fn restores_with_finalized_layout() { #[test] fn uses_scratch_payloads_outside_pools() { let case = TestCase::new(); - let addr = case.h2g_pool_base + memory_layout().get_h2g_queue_dims().pool_len() as u64; + // General scratch, below the arena: the allocator's first free page. + let addr = scratch_gva(memory_layout().get_first_free_scratch_gpa()); case.mem.write(addr, &[1, 2, 3]).unwrap(); let mut g2h_desc = Descriptor::new(addr, 3, 0, DescFlags::empty()); diff --git a/src/hyperlight_host/src/sandbox/initialized.rs b/src/hyperlight_host/src/sandbox/initialized.rs index 85db5a42f6..71e5f9615c 100644 --- a/src/hyperlight_host/src/sandbox/initialized.rs +++ b/src/hyperlight_host/src/sandbox/initialized.rs @@ -1264,6 +1264,23 @@ mod tests { assert!(sbox.mem_mgr.h2g_consumer.is_some()); } + /// The page between the transport arena and the reserved top pages is + /// unmapped, so an exception stack overrun faults before it reaches the + /// arena. + #[test] + fn transport_guard_page_is_unmapped() { + use hyperlight_common::layout::{scratch_reserved_top_gva, scratch_transport_end_gva}; + + let mut sbox = SandboxBuilder::from_file(simple_guest_as_pathbuf()) + .build() + .unwrap(); + let guard = scratch_transport_end_gva(); + let mut mapped = |gva: u64| sbox.call::("CheckMapped", gva).unwrap(); + assert!(mapped(guard - hyperlight_common::vmem::PAGE_SIZE as u64)); + assert!(!mapped(guard)); + assert!(mapped(scratch_reserved_top_gva())); + } + #[test] fn poison() { let mut sbox = SandboxBuilder::from_file(simple_guest_as_pathbuf()) @@ -1437,8 +1454,7 @@ mod tests { let defaults = SandboxConfiguration::default(); let layout = crate::mem::layout::SandboxMemoryLayout::new(defaults, 0, 0, None).unwrap(); - hyperlight_common::layout::min_scratch_size(layout.get_transport_arena().size()) - .next_multiple_of(page_size::get()) + layout.min_scratch().next_multiple_of(page_size::get()) } + 0x40000; let mut sbox1 = SandboxBuilder::from_file(simple_guest_as_pathbuf()) diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/media_types.rs b/src/hyperlight_host/src/sandbox/snapshot/file/media_types.rs index e745ef2aff..98797348fe 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/media_types.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/media_types.rs @@ -21,7 +21,7 @@ pub(in crate::sandbox::snapshot) const MT_TRANSPORT_CURRENT: &str = MT_TRANSPORT /// ABI version for the snapshot memory blob. Bumped when the /// host-guest contract for the snapshot bytes changes. See /// docs/snapshot-versioning.md. -pub(in crate::sandbox::snapshot) const SNAPSHOT_ABI_VERSION: u32 = 6; +pub(in crate::sandbox::snapshot) const SNAPSHOT_ABI_VERSION: u32 = 7; /// OCI standard annotation key for a manifest's tag inside an image /// index. Set on the manifest descriptor in `index.json`, not on the diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index 7e801ea927..88dde7cb5f 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -2091,7 +2091,7 @@ fn legacy_config_versions_rejected() { &path, OciTag::new("latest").unwrap(), )); - assert_err_contains(err, "incompatible with snapshot ABI 6"); + assert_err_contains(err, "incompatible with snapshot ABI 7"); } } diff --git a/src/hyperlight_host/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index 197edadf0e..91c37f4447 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -11,7 +11,10 @@ use std::sync::Arc; pub(crate) use file::host_cpu_vendor_golden_tag; pub use file::reference::{OciDigest, OciReference, OciTag}; use hyperlight_common::flatbuffer_wrappers::host_function_details::HostFunctionDetails; -use hyperlight_common::layout::{io_page, scratch_base_gpa, scratch_base_gva}; +use hyperlight_common::layout::{ + SCRATCH_TOP_GVA, io_page, scratch_base_gpa, scratch_base_gva, scratch_reserved_top_gpa, + scratch_reserved_top_gva, scratch_transport_end_gva, +}; use hyperlight_common::vmem; use hyperlight_common::vmem::{ BasicMapping, CowMapping, Mapping, MappingKind, PAGE_SIZE, SpaceAwareMapping, SpaceId, TableOps, @@ -282,20 +285,41 @@ fn map_specials(pt_buf: &GuestPageTableBuffer, scratch_size: usize) { }; unsafe { vmem::map(pt_buf, mapping) }; } - // Map the scratch region - let mapping = Mapping { - phys_base: scratch_base_gpa(scratch_size), - virt_base: scratch_base_gva(scratch_size), - len: scratch_size as u64, - kind: MappingKind::Basic(BasicMapping { - readable: true, - writable: true, - // assume that the guest will map these pages elsewhere if - // it actually needs to execute from them - executable: false, - }), + // Map the scratch region, except the guard pages under the reserved top + // pages. Left unmapped, they make an amd64 exception stack overrun fault + // before it reaches the transport arena. + let base_gpa = scratch_base_gpa(scratch_size); + let base_gva = scratch_base_gva(scratch_size); + let gap_gva = scratch_transport_end_gva(); + let top_gva = scratch_reserved_top_gva(); + let ranges = if base_gva < gap_gva { + [ + (base_gpa, base_gva, gap_gva - base_gva), + ( + scratch_reserved_top_gpa(), + top_gva, + SCRATCH_TOP_GVA as u64 + 1 - top_gva, + ), + ] + } else { + // Only tests build a scratch window too small to reach the gap. + [(base_gpa, base_gva, scratch_size as u64), (0, 0, 0)] }; - unsafe { vmem::map(pt_buf, mapping) }; + for (phys_base, virt_base, len) in ranges.into_iter().filter(|r| r.2 != 0) { + let mapping = Mapping { + phys_base, + virt_base, + len, + kind: MappingKind::Basic(BasicMapping { + readable: true, + writable: true, + // assume that the guest will map these pages elsewhere if + // it actually needs to execute from them + executable: false, + }), + }; + unsafe { vmem::map(pt_buf, mapping) }; + } } impl Snapshot { diff --git a/src/hyperlight_host/src/sandbox/snapshot/tripwires.rs b/src/hyperlight_host/src/sandbox/snapshot/tripwires.rs index 46302ed05b..beaa41a62e 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/tripwires.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/tripwires.rs @@ -16,7 +16,7 @@ use super::file::{ SNAPSHOT_ABI_VERSION, }; -const EXPECTED_ABI_VERSION: u32 = 6; +const EXPECTED_ABI_VERSION: u32 = 7; const EXPECTED_MT_CONFIG: &str = "application/vnd.hyperlight.snapshot.config.v3+json"; const EXPECTED_MT_SNAPSHOT: &str = "application/vnd.hyperlight.snapshot.memory.v1"; const EXPECTED_MT_TRANSPORT: &str = "application/vnd.hyperlight.snapshot.transport.v1"; diff --git a/src/hyperlight_host/tests/snapshot_goldens/goldens_version.rs b/src/hyperlight_host/tests/snapshot_goldens/goldens_version.rs index 164a276c8e..91ce709f96 100644 --- a/src/hyperlight_host/tests/snapshot_goldens/goldens_version.rs +++ b/src/hyperlight_host/tests/snapshot_goldens/goldens_version.rs @@ -8,7 +8,7 @@ //! publish. See `docs/snapshot-versioning.md`. /// Goldens version, a `vMAJOR.MINOR` string. -pub(crate) const GOLDENS_VERSION: &str = "v6.0"; +pub(crate) const GOLDENS_VERSION: &str = "v7.0"; /// Old majors kept loadable through a compatibility path, verified /// alongside `GOLDENS_VERSION`. A backwards-compatible break (Option 2) From 991129c1405b0d3389fa6e291be719ef669a33a1 Mon Sep 17 00:00:00 2001 From: danbugs Date: Fri, 16 Oct 2026 16:51:18 +0000 Subject: [PATCH 2/2] feat(sandbox): take snapshots that restore with a different scratch size Sandbox::snapshot_with_options takes a snapshot with SnapshotOptions, which leaves room for more options later. Sandbox::snapshot is the default-options case. SnapshotOptions::scratch_size gives the snapshot's layout another scratch size. The snapshot's page tables are built for that size, walking the live tables with the sandbox's own layout. Its virtqueue rings carry over unchanged, since the transport arena's address does not depend on the scratch size. Restore replaces scratch when the size differs. A size above the maximum memory size, too small for the transport, or not a whole number of host pages is rejected before the guest is entered. Room for the page tables is checked when they are built. A larger scratch window covers guest-virtual addresses below the current one, so building the snapshot fails if the guest has a mapping there. The snapshot loader refuses a scratch size this host cannot map. A guest can boot with the scratch its initialisation needs and run guest function calls from a snapshot with only the scratch they need, or with more scratch than it booted with. Less scratch makes restores that zero or replace all of it cost less. Signed-off-by: danbugs --- CHANGELOG.md | 1 + docs/paging-development-notes.md | 15 +- docs/virtio-host-guest-communication.md | 5 +- src/hyperlight_host/src/mem/layout.rs | 86 ++++++- src/hyperlight_host/src/mem/mgr.rs | 2 + src/hyperlight_host/src/mem/virtq/mod.rs | 24 +- src/hyperlight_host/src/mem/virtq/tests.rs | 15 +- .../src/sandbox/initialized.rs | 225 ++++++++++++++++-- .../src/sandbox/snapshot/file/mod.rs | 11 +- .../src/sandbox/snapshot/file/transport.rs | 45 ++-- .../src/sandbox/snapshot/file_tests.rs | 25 +- .../src/sandbox/snapshot/mod.rs | 155 ++++++++++-- 12 files changed, 510 insertions(+), 99 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f18394c84..f3ac0bdc57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Prerelease] - Unreleased ### Added +* `Sandbox::snapshot_with_options` takes a snapshot with `SnapshotOptions`. `SnapshotOptions::scratch_size` gives a restored sandbox a different scratch size. A guest can boot with the scratch its initialisation needs and run guest function calls from a snapshot with only the scratch they need, so each restore costs less. It can also give a guest more scratch than it booted with. ### Changed * Core dumps no longer record the guest binary path. The process name and command line are always ``, as they already were for sandboxes created from snapshots by @ludfjig in https://github.com/hyperlight-dev/hyperlight/pull/1975 diff --git a/docs/paging-development-notes.md b/docs/paging-development-notes.md index 06be5e4671..5c2406ae49 100644 --- a/docs/paging-development-notes.md +++ b/docs/paging-development-notes.md @@ -129,9 +129,18 @@ scratch region that will be used when this snapshot is next restored into a sandbox. This will then be baked into the guest page tables created in the snapshot. -TODO: add support, if found to be useful operationally, for either -dynamically growing the scratch region, or changing its size between -taking a snapshot and restoring it. +`Sandbox::snapshot_with_options`, with `SnapshotOptions::scratch_size`, +takes a snapshot whose scratch size differs from the sandbox's. Its +page tables map a window of that size, and restoring it replaces the +scratch region. A guest can boot with the scratch its initialisation +needs and run guest function calls from a snapshot with only the +scratch they need, or with more. + +The scratch window grows downward from the top of the address space. A +larger window covers guest-virtual addresses below the current one, so +the host rejects the size when the guest has a mapping there. The +physical window needs no such check: scratch hangs from a fixed top, +far above the largest snapshot region. ### Call descriptors diff --git a/docs/virtio-host-guest-communication.md b/docs/virtio-host-guest-communication.md index d2b7362138..64d293e650 100644 --- a/docs/virtio-host-guest-communication.md +++ b/docs/virtio-host-guest-communication.md @@ -476,8 +476,9 @@ not stored. ### Restore -Capture and load validate scratch size, ring lengths, and canonical transport -state against the layout. Admitted images and their layout remain immutable. +Capture and load validate ring lengths and canonical transport state against +the layout. Load also checks the scratch size the transport blob records. +Admitted images and their layout remain immutable. Transport admission precedes changes to sandbox status, the cached snapshot, and memory mappings. diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index e9c9c07cb0..8e886f9eec 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -360,7 +360,10 @@ impl SandboxMemoryLayout { .ok_or(MemoryRequestTooSmall(scratch_size, usize::MAX))?; let min_scratch_size = Self::min_scratch_for(&transport_arena); if scratch_size < min_scratch_size { - return Err(MemoryRequestTooSmall(scratch_size, min_scratch_size)); + return Err(MemoryRequestTooSmall( + scratch_size, + min_scratch_size.next_multiple_of(Self::host_page_size()), + )); } let mut ret = Self { @@ -382,6 +385,12 @@ impl SandboxMemoryLayout { Ok(ret) } + /// Granularity the host maps scratch in: host pages, which can be + /// larger than the guest's. + pub(crate) fn host_page_size() -> usize { + PAGE_SIZE.max(page_size::get()) + } + /// Fixed scratch overhead: the arena and the guard page above it count /// as transport memory. fn min_scratch_for(arena: &TransportArena) -> usize { @@ -405,6 +414,31 @@ impl SandboxMemoryLayout { TransportArena::new(base, g2h, h2g) } + /// This layout with another scratch size and no page tables yet. Room + /// for the page tables is checked when they are built (`set_pt_size`), + /// since their size depends on the guest's mappings. + pub(crate) fn with_scratch_size(&self, scratch_size: usize) -> Result { + if scratch_size > Self::MAX_MEMORY_SIZE { + return Err(MemoryRequestTooBig(scratch_size, Self::MAX_MEMORY_SIZE)); + } + // The host maps scratch in host pages, which can be larger than + // the guest's. A size checked here is one a restore can map. + let page = Self::host_page_size(); + let min = self.min_scratch().next_multiple_of(page); + if scratch_size < min { + return Err(MemoryRequestTooSmall(scratch_size, min)); + } + if !scratch_size.is_multiple_of(page) { + return Err(new_error!( + "scratch size {scratch_size} must be a multiple of {page}" + )); + } + let mut ret = *self; + ret.scratch_size = scratch_size; + ret.pt_size = None; + Ok(ret) + } + /// Scratch offset of the transport arena. pub(crate) fn arena_scratch_offset(&self) -> usize { debug_assert!( @@ -497,7 +531,12 @@ impl SandboxMemoryLayout { pub(crate) fn set_pt_size(&mut self, size: usize) -> Result<()> { let min_scratch = self.min_scratch().saturating_add(size); if self.scratch_size < min_scratch { - return Err(MemoryRequestTooSmall(self.scratch_size, min_scratch)); + // Report a size the host can map. + let page = Self::host_page_size(); + return Err(MemoryRequestTooSmall( + self.scratch_size, + min_scratch.next_multiple_of(page), + )); } self.pt_size = Some(size); Ok(()) @@ -869,6 +908,49 @@ mod tests { ); } + /// A different scratch size keeps the arena where it is, so a guest's + /// cached ring addresses stay valid in a resized snapshot. + #[test] + fn resizing_scratch_keeps_the_transport_arena() { + let mut cfg = SandboxConfiguration::default(); + cfg.set_scratch_size(4 << 20); + let layout = SandboxMemoryLayout::new(cfg, 4096, 0, None).unwrap(); + // Sizes in host pages, which can be larger than the guest's. + let page = SandboxMemoryLayout::host_page_size(); + let half = (layout.get_scratch_size() / 2).next_multiple_of(page); + let smaller = layout.with_scratch_size(half).unwrap(); + + assert_eq!(smaller.get_transport_arena(), layout.get_transport_arena()); + assert_eq!( + smaller.get_pt_base_gpa(), + hyperlight_common::layout::scratch_base_gpa(half) + ); + assert_eq!( + smaller.mbx_scratch_offset() + (layout.get_scratch_size() - half), + layout.mbx_scratch_offset() + ); + assert!(matches!( + layout.with_scratch_size(layout.min_scratch() - 1), + Err(MemoryRequestTooSmall(..)) + )); + assert!(layout.with_scratch_size(half + 1).is_err()); + assert!(matches!( + layout.with_scratch_size( + SandboxMemoryLayout::MAX_MEMORY_SIZE.next_multiple_of(page) + page + ), + Err(MemoryRequestTooBig(..)) + )); + + let larger = layout + .with_scratch_size(layout.get_scratch_size() * 2) + .unwrap(); + assert_eq!(larger.get_transport_arena(), layout.get_transport_arena()); + assert_eq!( + larger.mbx_scratch_offset(), + layout.mbx_scratch_offset() + layout.get_scratch_size() + ); + } + #[test] fn transport_memory_is_part_of_minimum_scratch_size() { let mut cfg = SandboxConfiguration::default(); diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index c91c050b01..fe99b6b857 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -426,6 +426,7 @@ impl SandboxMemoryManager { #[cfg(target_arch = "x86_64")] msrs: Vec, next_action: NextAction, host_functions: HostFunctionDetails, + scratch_size: usize, ) -> Result { let virtq = match (&self.g2h_consumer, &self.h2g_consumer) { (Some(_), Some(_)) => Some(VirtqSnapshot::capture(&self.layout, &self.scratch_mem)?), @@ -449,6 +450,7 @@ impl SandboxMemoryManager { self.snapshot_count + 1, host_functions, virtq, + scratch_size, )?; self.snapshot_count = snapshot.snapshot_generation(); Ok(snapshot) diff --git a/src/hyperlight_host/src/mem/virtq/mod.rs b/src/hyperlight_host/src/mem/virtq/mod.rs index d2616c81f0..4f281f3940 100644 --- a/src/hyperlight_host/src/mem/virtq/mod.rs +++ b/src/hyperlight_host/src/mem/virtq/mod.rs @@ -68,8 +68,6 @@ pub(crate) fn create_consumers( /// Validated ring images excluded from ordinary snapshot pages. #[derive(Debug, PartialEq, Eq)] pub(crate) struct VirtqSnapshot { - /// Scratch size used to derive transport GVAs. - scratch_size: usize, /// Guest-to-host ring bytes. g2h_ring: Vec, /// Host-to-guest ring bytes. @@ -80,15 +78,10 @@ impl VirtqSnapshot { /// Validate captured or decoded rings against the final transport geometry. pub(crate) fn new( layout: &SandboxMemoryLayout, - scratch_size: usize, g2h_ring: Vec, h2g_ring: Vec, ) -> Result { - let snapshot = Self { - scratch_size, - g2h_ring, - h2g_ring, - }; + let snapshot = Self { g2h_ring, h2g_ring }; snapshot.validate(layout)?; Ok(snapshot) @@ -115,12 +108,7 @@ impl VirtqSnapshot { layout.get_h2g_queue_dims().ring_len(), )?; - Self::new(layout, layout.get_scratch_size(), g2h_ring, h2g_ring) - } - - /// Scratch size used to interpret the saved ring addresses. - pub(crate) fn scratch_size(&self) -> usize { - self.scratch_size + Self::new(layout, g2h_ring, h2g_ring) } /// Canonical guest-to-host ring bytes. @@ -153,14 +141,6 @@ impl VirtqSnapshot { /// Check geometry, canonical rings, and distinct, aligned H2G pool slots. fn validate(&self, layout: &SandboxMemoryLayout) -> Result<()> { - if self.scratch_size != layout.get_scratch_size() { - return Err(new_error!( - "virtqueue snapshot scratch size {} does not match layout size {}", - self.scratch_size, - layout.get_scratch_size() - )); - } - if self.g2h_ring.len() != layout.get_g2h_queue_dims().ring_len() || self.h2g_ring.len() != layout.get_h2g_queue_dims().ring_len() { diff --git a/src/hyperlight_host/src/mem/virtq/tests.rs b/src/hyperlight_host/src/mem/virtq/tests.rs index e37fcbd8d4..ef3f5d532c 100644 --- a/src/hyperlight_host/src/mem/virtq/tests.rs +++ b/src/hyperlight_host/src/mem/virtq/tests.rs @@ -182,23 +182,12 @@ fn rejects_snapshot_geometry_mismatches() { let case = TestCase::new(); let layout = memory_layout(); let captured = VirtqSnapshot::capture(&layout, &case.scratch).unwrap(); - let scratch_size = layout.get_scratch_size(); let g2h = captured.g2h_ring(); let h2g = captured.h2g_ring(); - for (size, g2h_len, h2g_len) in [ - (scratch_size - vmem::PAGE_SIZE, g2h.len(), h2g.len()), - (scratch_size, g2h.len() - 1, h2g.len()), - (scratch_size, g2h.len(), h2g.len() - 1), - ] { + for (g2h_len, h2g_len) in [(g2h.len() - 1, h2g.len()), (g2h.len(), h2g.len() - 1)] { assert!( - VirtqSnapshot::new( - &layout, - size, - g2h[..g2h_len].to_vec(), - h2g[..h2g_len].to_vec(), - ) - .is_err() + VirtqSnapshot::new(&layout, g2h[..g2h_len].to_vec(), h2g[..h2g_len].to_vec()).is_err() ); } } diff --git a/src/hyperlight_host/src/sandbox/initialized.rs b/src/hyperlight_host/src/sandbox/initialized.rs index 71e5f9615c..a74d9631bb 100644 --- a/src/hyperlight_host/src/sandbox/initialized.rs +++ b/src/hyperlight_host/src/sandbox/initialized.rs @@ -16,7 +16,7 @@ use tracing_core::LevelFilter; use super::Callable; use super::file_mapping::prepare_file_cow; use super::host_funcs::FunctionRegistry; -use super::snapshot::Snapshot; +use super::snapshot::{Snapshot, SnapshotOptions}; use crate::func::{ParameterTuple, SupportedReturnType}; use crate::hypervisor::InterruptHandle; use crate::hypervisor::hyperlight_vm::{HyperlightVm, HyperlightVmError}; @@ -388,14 +388,56 @@ impl Sandbox { /// # Ok(()) /// # } /// ``` - #[instrument(err(Debug), skip_all, parent = Span::current())] pub fn snapshot(&mut self) -> Result> { - self.check_ready()?; + self.snapshot_with_options(&SnapshotOptions::default()) + } - if let Some(snapshot) = &self.snapshot { - return Ok(snapshot.clone()); + /// Like [`Self::snapshot`], with [`SnapshotOptions`] for how the + /// snapshot restores. Default options take the same snapshot as + /// [`Self::snapshot`]. Options that change the scratch size capture a + /// new snapshot each time, so callers should keep the result. It + /// returns the same sandbox status errors as [`Self::snapshot`]. + /// + /// # Examples + /// + /// ```no_run + /// # use hyperlight_host::SandboxBuilder; + /// # use hyperlight_host::sandbox::snapshot::SnapshotOptions; + /// # fn example() -> Result<(), Box> { + /// let mut sandbox = SandboxBuilder::from_file("guest.bin").build()?; + /// + /// // Run later guest function calls with 4 MiB of scratch. + /// let options = SnapshotOptions::default().scratch_size(4 << 20); + /// let snapshot = sandbox.snapshot_with_options(&options)?; + /// sandbox.restore(snapshot)?; + /// # Ok(()) + /// # } + /// ``` + #[instrument(err(Debug), skip_all, parent = Span::current())] + pub fn snapshot_with_options(&mut self, options: &SnapshotOptions) -> Result> { + self.check_ready()?; + // Unpacked in full so that a new option has to decide whether the + // cached plain snapshot still answers it. + let SnapshotOptions { scratch_size } = options; + let current = self.mem_mgr.layout.get_scratch_size(); + let scratch_size = scratch_size.unwrap_or(current); + + if scratch_size == current { + if let Some(snapshot) = &self.snapshot { + return Ok(snapshot.clone()); + } + let snapshot = Arc::new(self.capture_snapshot(scratch_size)?); + self.snapshot = Some(snapshot.clone()); + return Ok(snapshot); } + // Reject a bad size before the capture enters the guest. + self.mem_mgr.layout.with_scratch_size(scratch_size)?; + Ok(Arc::new(self.capture_snapshot(scratch_size)?)) + } + + /// Capture a snapshot whose scratch has `scratch_size` bytes. + fn capture_snapshot(&mut self, scratch_size: usize) -> Result { if self.transport_dirty { self.checkpoint_transport_for_snapshot()?; } @@ -435,7 +477,7 @@ impl Sandbox { })?) .into(); - let memory_snapshot = self.mem_mgr.snapshot( + self.mem_mgr.snapshot( mapped_regions_vec, &root_pt_gpas, stack_top_gpa, @@ -444,10 +486,8 @@ impl Sandbox { msrs, next_action, host_functions, - )?; - let snapshot = Arc::new(memory_snapshot); - self.snapshot = Some(snapshot.clone()); - Ok(snapshot) + scratch_size, + ) } fn checkpoint_transport_for_snapshot(&mut self) -> Result<()> { @@ -501,7 +541,9 @@ impl Sandbox { /// The restored state includes snapshot and scratch memory, vCPU state, /// stack state, the next VM action, captured MSRs on x86_64, and the removal /// of dynamic memory mappings. This discards leaked allocations, restores - /// allocator and lock state, and rolls back partial updates. + /// allocator and lock state, and rolls back partial updates. The sandbox + /// takes the snapshot's scratch size, which can differ from its own, as + /// for one taken with [`SnapshotOptions::scratch_size`]. /// /// Restore failures have three status outcomes: /// @@ -1238,6 +1280,7 @@ mod tests { use crate::mem::mgr::ExecutionMode; use crate::mem::shared_mem::{ExclusiveSharedMemory, GuestSharedMemory, SharedMemory as _}; use crate::sandbox::SandboxConfiguration; + use crate::sandbox::snapshot::SnapshotOptions; use crate::sandbox::uninitialized::{GuestBlob, GuestEnvironment}; use crate::{ GuestBinary, HyperlightError, Result, Sandbox, SandboxBuilder, SandboxStatus, @@ -1259,26 +1302,174 @@ mod tests { assert!(SandboxStatus::Unrecoverable.is_unrecoverable()); } + fn scratch(scratch_size: usize) -> SnapshotOptions { + SnapshotOptions::default().scratch_size(scratch_size) + } + fn assert_virtq_attached(sbox: &Sandbox) { assert!(sbox.mem_mgr.g2h_consumer.is_some()); assert!(sbox.mem_mgr.h2g_consumer.is_some()); } /// The page between the transport arena and the reserved top pages is - /// unmapped, so an exception stack overrun faults before it reaches the - /// arena. + /// unmapped, at boot and after restoring a resized snapshot, so an + /// exception stack overrun faults before it reaches the arena. #[test] fn transport_guard_page_is_unmapped() { use hyperlight_common::layout::{scratch_reserved_top_gva, scratch_transport_end_gva}; let mut sbox = SandboxBuilder::from_file(simple_guest_as_pathbuf()) + .scratch_size(8 << 20) .build() .unwrap(); + let small = sbox.snapshot_with_options(&scratch(2 << 20)).unwrap(); let guard = scratch_transport_end_gva(); - let mut mapped = |gva: u64| sbox.call::("CheckMapped", gva).unwrap(); - assert!(mapped(guard - hyperlight_common::vmem::PAGE_SIZE as u64)); - assert!(!mapped(guard)); - assert!(mapped(scratch_reserved_top_gva())); + for restore in [None, Some(small)] { + if let Some(snapshot) = restore { + sbox.restore(snapshot).unwrap(); + } + let mapped = + |sbox: &mut Sandbox, gva: u64| sbox.call::("CheckMapped", gva).unwrap(); + assert!(mapped( + &mut sbox, + guard - hyperlight_common::vmem::PAGE_SIZE as u64 + )); + assert!(!mapped(&mut sbox, guard)); + assert!(mapped(&mut sbox, scratch_reserved_top_gva())); + } + } + + /// A snapshot can give a restored sandbox less scratch, and a sandbox + /// restored from it runs guest function calls and can go back to its + /// original size. + #[test] + fn snapshot_scratch_size_resizes_on_restore() { + let mut sbox = SandboxBuilder::from_file(simple_guest_as_pathbuf()) + .scratch_size(8 << 20) + .build() + .unwrap(); + let full = sbox.snapshot().unwrap(); + // Default options and the sandbox's own size are a plain snapshot, + // already taken. + let default = sbox + .snapshot_with_options(&SnapshotOptions::default()) + .unwrap(); + assert!(Arc::ptr_eq(&default, &full)); + let same = sbox.snapshot_with_options(&scratch(8 << 20)).unwrap(); + assert!(Arc::ptr_eq(&same, &full)); + let small = sbox.snapshot_with_options(&scratch(2 << 20)).unwrap(); + assert!(matches!( + sbox.snapshot_with_options(&scratch(4096)), + Err(HyperlightError::MemoryRequestTooSmall(..)) + )); + assert!(matches!( + sbox.snapshot_with_options(&scratch(32 << 30)), + Err(HyperlightError::MemoryRequestTooBig(..)) + )); + // Rejected sizes do not enter the guest: a pending transport + // checkpoint, which a capture would run, is still pending. + let echo: String = sbox.call("Echo", "still".to_string()).unwrap(); + assert_eq!(echo, "still"); + assert!(sbox.transport_dirty); + assert!(sbox.snapshot_with_options(&scratch(4096)).is_err()); + assert!(sbox.snapshot_with_options(&scratch(32 << 30)).is_err()); + assert!(sbox.transport_dirty); + + // A size with room for the transport but not the page tables fails + // when the snapshot is built, and leaves the sandbox usable. + let page = crate::mem::layout::SandboxMemoryLayout::host_page_size(); + let no_pt_room = sbox.mem_mgr.layout.min_scratch().next_multiple_of(page); + assert!(matches!( + sbox.snapshot_with_options(&scratch(no_pt_room)), + Err(HyperlightError::MemoryRequestTooSmall(..)) + )); + let echo: String = sbox.call("Echo", "usable".to_string()).unwrap(); + assert_eq!(echo, "usable"); + + for _ in 0..3 { + sbox.restore(small.clone()).unwrap(); + assert_eq!(sbox.mem_mgr.scratch_mem.mem_size(), 2 << 20); + let echo: String = sbox.call("Echo", "hello".to_string()).unwrap(); + assert_eq!(echo, "hello"); + sbox.call::<()>("AllocAndWritePattern", 32u64 << 10) + .unwrap(); + } + // A snapshot of the resized sandbox keeps its size. + let again = sbox.snapshot().unwrap(); + assert_eq!(again.layout().get_scratch_size(), 2 << 20); + + // So does one saved to a file and loaded back. + let dir = tempfile::tempdir().unwrap(); + let tag = crate::sandbox::snapshot::OciTag::new("small").unwrap(); + small.save(dir.path(), &tag).unwrap(); + let loaded = crate::sandbox::snapshot::Snapshot::checked_load(dir.path(), tag).unwrap(); + sbox.restore(Arc::new(loaded)).unwrap(); + assert_eq!(sbox.mem_mgr.scratch_mem.mem_size(), 2 << 20); + let echo: String = sbox.call("Echo", "loaded".to_string()).unwrap(); + assert_eq!(echo, "loaded"); + + sbox.restore(full).unwrap(); + assert_eq!(sbox.mem_mgr.scratch_mem.mem_size(), 8 << 20); + let echo: String = sbox.call("Echo", "back".to_string()).unwrap(); + assert_eq!(echo, "back"); + } + + /// A snapshot can give a restored sandbox more scratch than it booted + /// with, for guest function calls that need it. + #[test] + fn snapshot_scratch_size_grows_on_restore() { + let mut sbox = SandboxBuilder::from_file(simple_guest_as_pathbuf()) + .heap_size(16 << 20) + .scratch_size(4 << 20) + .build() + .unwrap(); + let large = sbox.snapshot_with_options(&scratch(16 << 20)).unwrap(); + assert_eq!(large.layout().get_scratch_size(), 16 << 20); + // 8 MiB of heap writes need 8 MiB of copy-on-write scratch. + assert!(sbox.call::<()>("AllocAndWritePattern", 8u64 << 20).is_err()); + + for _ in 0..2 { + sbox.restore(large.clone()).unwrap(); + assert_eq!(sbox.mem_mgr.scratch_mem.mem_size(), 16 << 20); + sbox.call::<()>("AllocAndWritePattern", 8u64 << 20).unwrap(); + let echo: String = sbox.call("Echo", "grown".to_string()).unwrap(); + assert_eq!(echo, "grown"); + } + } + + /// The guest's page allocator stops at the transport arena. A resized + /// sandbox that runs out of scratch aborts once an allocation reaches the + /// arena, with nothing handed out inside it, and a restore recovers it. + #[test] + fn running_out_of_scratch_stops_at_the_transport_arena() { + let mut sbox = SandboxBuilder::from_file(simple_guest_as_pathbuf()) + .heap_size(8 << 20) + .scratch_size(8 << 20) + .build() + .unwrap(); + let small = sbox.snapshot_with_options(&scratch(2 << 20)).unwrap(); + sbox.restore(small.clone()).unwrap(); + // 4 MiB of heap writes need 4 MiB of copy-on-write scratch. + let err = sbox + .call::<()>("AllocAndWritePattern", 4u64 << 20) + .unwrap_err(); + assert!( + matches!(&err, HyperlightError::GuestAborted(code, msg) + if *code == ErrorCode::MallocFailed as u8 && msg.contains("Out of physical memory")), + "{err:?}" + ); + let size = sbox.mem_mgr.scratch_mem.mem_size(); + let offset = size - hyperlight_common::layout::SCRATCH_TOP_ALLOCATOR_OFFSET as usize; + let allocator = sbox.mem_mgr.scratch_mem.read::(offset).unwrap(); + let arena = sbox.mem_mgr.layout.get_transport_arena(); + // The failed allocation moved the pointer past the arena base. The + // limit is the arena, and nothing was handed out inside it. + assert!(allocator > arena.base_addr(), "{allocator:#x}"); + assert!(allocator < arena.base_addr() + 0x4000, "{allocator:#x}"); + + sbox.restore(small).unwrap(); + let echo: String = sbox.call("Echo", "recovered".to_string()).unwrap(); + assert_eq!(echo, "recovered"); } #[test] diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs index b336907f52..b0800d17f2 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/mod.rs @@ -519,7 +519,7 @@ impl Snapshot { let transport = self.state.virtq.as_ref().ok_or_else(|| { crate::new_error!("initialized snapshot has no canonical transport state") })?; - let transport_bytes = transport::encode(transport)?; + let transport_bytes = transport::encode(&self.state.layout, transport)?; let transport_digest = Digest256::from_bytes(&transport_bytes); put_blob(&blobs_dir, &transport_digest, &transport_bytes)?; @@ -846,6 +846,15 @@ impl Snapshot { // 4. config blob let cfg = load_config(&blobs_dir, cfg_desc, verify_blobs)?; + // A snapshot taken on a host with smaller pages may have a scratch + // size this host cannot map: refuse it before reading the blobs. + let host_page = crate::mem::layout::SandboxMemoryLayout::host_page_size(); + if !cfg.layout.scratch_size.is_multiple_of(host_page) { + return Err(crate::new_error!( + "snapshot scratch size {} is not a multiple of this host's {host_page}-byte pages", + cfg.layout.scratch_size + )); + } // 5. snapshot blob: open once, hash and mmap the same // handle so an attacker cannot swap the file between diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/transport.rs b/src/hyperlight_host/src/sandbox/snapshot/file/transport.rs index 936bd03617..109f92834b 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/transport.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/transport.rs @@ -13,8 +13,11 @@ const MAGIC: [u8; 8] = *b"HLVQSNAP"; const VERSION: u32 = 1; const HEADER_LEN: usize = 40; -/// Serialize the scratch size and ring images. -pub(super) fn encode(snapshot: &VirtqSnapshot) -> crate::Result> { +/// Serialize the snapshot's scratch size and ring images. +pub(super) fn encode( + layout: &SandboxMemoryLayout, + snapshot: &VirtqSnapshot, +) -> crate::Result> { let g2h_len = snapshot.g2h_ring().len(); let h2g_len = snapshot.h2g_ring().len(); @@ -37,7 +40,7 @@ pub(super) fn encode(snapshot: &VirtqSnapshot) -> crate::Result> { bytes.extend_from_slice(&MAGIC); bytes.extend_from_slice(&VERSION.to_le_bytes()); bytes.extend_from_slice(&0u32.to_le_bytes()); - bytes.extend_from_slice(&u64::try_from(snapshot.scratch_size())?.to_le_bytes()); + bytes.extend_from_slice(&u64::try_from(layout.get_scratch_size())?.to_le_bytes()); bytes.extend_from_slice(&u64::try_from(g2h_len)?.to_le_bytes()); bytes.extend_from_slice(&u64::try_from(h2g_len)?.to_le_bytes()); bytes.extend_from_slice(snapshot.g2h_ring()); @@ -92,6 +95,12 @@ pub(super) fn decode(layout: &SandboxMemoryLayout, bytes: &[u8]) -> crate::Resul )); }; let scratch_size = usize::try_from(u64::from_le_bytes(scratch_size))?; + if scratch_size != layout.get_scratch_size() { + return Err(crate::new_error!( + "snapshot transport scratch size {scratch_size} does not match layout size {}", + layout.get_scratch_size() + )); + } let Some(g2h_len) = read_field(&mut bytes) else { return Err(crate::new_error!( @@ -122,7 +131,7 @@ pub(super) fn decode(layout: &SandboxMemoryLayout, bytes: &[u8]) -> crate::Resul // The checked total length guarantees both ring slices fit. let (g2h_ring, h2g_ring) = bytes.split_at(g2h_len); - VirtqSnapshot::new(layout, scratch_size, g2h_ring.to_vec(), h2g_ring.to_vec()) + VirtqSnapshot::new(layout, g2h_ring.to_vec(), h2g_ring.to_vec()) } #[cfg(test)] @@ -155,14 +164,14 @@ mod tests { expected.extend_from_slice(snapshot.g2h_ring()); expected.extend_from_slice(snapshot.h2g_ring()); - assert_eq!(encode(&snapshot).unwrap(), expected); + assert_eq!(encode(&layout, &snapshot).unwrap(), expected); assert_eq!(decode(&layout, &expected).unwrap(), snapshot); } #[test] fn transport_blob_rejects_empty_rings() { let (layout, snapshot) = transport_snapshot(); - let mut bytes = encode(&snapshot).unwrap(); + let mut bytes = encode(&layout, &snapshot).unwrap(); bytes.truncate(HEADER_LEN); bytes[24..40].fill(0); @@ -173,7 +182,7 @@ mod tests { #[test] fn transport_blob_rejects_invalid_magic() { let (layout, snapshot) = transport_snapshot(); - let mut bytes = encode(&snapshot).unwrap(); + let mut bytes = encode(&layout, &snapshot).unwrap(); bytes[0] ^= 1; assert!( @@ -187,7 +196,7 @@ mod tests { #[test] fn transport_blob_rejects_version_mismatch() { let (layout, snapshot) = transport_snapshot(); - let mut bytes = encode(&snapshot).unwrap(); + let mut bytes = encode(&layout, &snapshot).unwrap(); bytes[8..12].copy_from_slice(&VERSION.wrapping_add(1).to_le_bytes()); assert!( @@ -201,7 +210,7 @@ mod tests { #[test] fn transport_blob_rejects_nonzero_reserved_field() { let (layout, snapshot) = transport_snapshot(); - let mut bytes = encode(&snapshot).unwrap(); + let mut bytes = encode(&layout, &snapshot).unwrap(); bytes[12] = 1; assert!( @@ -215,7 +224,7 @@ mod tests { #[test] fn transport_blob_rejects_truncated_header_fields() { let (layout, snapshot) = transport_snapshot(); - let bytes = encode(&snapshot).unwrap(); + let bytes = encode(&layout, &snapshot).unwrap(); for (range, field) in [ (0..8, "magic"), @@ -238,7 +247,7 @@ mod tests { #[test] fn transport_blob_rejects_truncated_ring_images() { let (layout, snapshot) = transport_snapshot(); - let bytes = encode(&snapshot).unwrap(); + let bytes = encode(&layout, &snapshot).unwrap(); for len in HEADER_LEN..bytes.len() { let error = decode(&layout, &bytes[..len]).unwrap_err(); @@ -249,7 +258,7 @@ mod tests { #[test] fn transport_blob_rejects_trailing_bytes() { let (layout, snapshot) = transport_snapshot(); - let mut bytes = encode(&snapshot).unwrap(); + let mut bytes = encode(&layout, &snapshot).unwrap(); bytes.push(3); assert!( @@ -264,7 +273,7 @@ mod tests { fn transport_blob_rejects_length_overflow() { for (g2h_len, h2g_len) in [(usize::MAX, 0), (0, usize::MAX)] { let (layout, snapshot) = transport_snapshot(); - let mut bytes = encode(&snapshot).unwrap(); + let mut bytes = encode(&layout, &snapshot).unwrap(); bytes[24..32].copy_from_slice(&(g2h_len as u64).to_le_bytes()); bytes[32..40].copy_from_slice(&(h2g_len as u64).to_le_bytes()); @@ -276,13 +285,13 @@ mod tests { #[test] fn transport_blob_rejects_layout_mismatch() { let (layout, snapshot) = transport_snapshot(); - let mut bytes = encode(&snapshot).unwrap(); - bytes[16..24].copy_from_slice(&(snapshot.scratch_size() as u64 + 1).to_le_bytes()); + let mut bytes = encode(&layout, &snapshot).unwrap(); + bytes[16..24].copy_from_slice(&(layout.get_scratch_size() as u64 + 1).to_le_bytes()); let error = decode(&layout, &bytes).unwrap_err(); assert!(error.to_string().contains("scratch size"), "{error}"); - bytes[16..24].copy_from_slice(&(snapshot.scratch_size() as u64).to_le_bytes()); + bytes[16..24].copy_from_slice(&(layout.get_scratch_size() as u64).to_le_bytes()); bytes[24..32].copy_from_slice(&(snapshot.g2h_ring().len() as u64 - 1).to_le_bytes()); bytes[32..40].copy_from_slice(&(snapshot.h2g_ring().len() as u64 + 1).to_le_bytes()); @@ -293,7 +302,7 @@ mod tests { #[test] fn transport_blob_rejects_noncanonical_rings() { let (layout, snapshot) = transport_snapshot(); - let mut bytes = encode(&snapshot).unwrap(); + let mut bytes = encode(&layout, &snapshot).unwrap(); bytes[HEADER_LEN] = 1; let error = decode(&layout, &bytes).unwrap_err(); @@ -321,7 +330,7 @@ mod tests { let layout = memory_layout(); let snapshot = VirtqSnapshot::capture(&layout, &case.scratch).unwrap(); - let mut bytes = encode(&snapshot).unwrap(); + let mut bytes = encode(&layout, &snapshot).unwrap(); let addr_offset = HEADER_LEN + snapshot.g2h_ring().len() + Descriptor::ADDR_OFFSET; let ring_addr = case.g2h_layout.desc_table_addr(); diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index 88dde7cb5f..e15b37bbe0 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -1265,8 +1265,10 @@ fn snapshot_size_smaller_than_layout_rejected() { cfg["layout"]["snapshot_size"] = Value::from(short); cfg["layout"]["pt_size"] = Value::from(mem - short); // Grow scratch to cover the larger pt tail so the scratch - // bound is not what trips. - cfg["layout"]["scratch_size"] = Value::from(mem + page); + // bound is not what trips. Whole host pages, so the host page + // check is not what trips either. + let host_page = crate::mem::layout::SandboxMemoryLayout::host_page_size() as u64; + cfg["layout"]["scratch_size"] = Value::from((mem + page).next_multiple_of(host_page)); }); let err = unwrap_err_snapshot(Snapshot::checked_load( &path, @@ -1275,6 +1277,25 @@ fn snapshot_size_smaller_than_layout_rejected() { assert_err_contains(err, "is smaller than the layout size"); } +#[test] +fn snapshot_scratch_size_not_in_host_pages_rejected() { + let snapshot = create_snapshot(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("snap"); + snapshot + .save(&path, &OciTag::new("latest").unwrap()) + .unwrap(); + rewrite_config(&path, |cfg| { + let size = cfg["layout"]["scratch_size"].as_u64().unwrap(); + cfg["layout"]["scratch_size"] = Value::from(size + 2048); + }); + let err = unwrap_err_snapshot(Snapshot::checked_load( + &path, + OciTag::new("latest").unwrap(), + )); + assert_err_contains(err, "is not a multiple of this host's"); +} + #[test] fn snapshot_layout_pt_size_unaligned_rejected() { let snapshot = create_snapshot(); diff --git a/src/hyperlight_host/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index 91c37f4447..0c57d185bb 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -59,6 +59,43 @@ pub enum NextAction { None, } +/// Options for +/// [`Sandbox::snapshot_with_options`](crate::Sandbox::snapshot_with_options). +/// The default options take the same snapshot as +/// [`Sandbox::snapshot`](crate::Sandbox::snapshot). +#[derive(Clone, Debug, Default)] +pub struct SnapshotOptions { + pub(crate) scratch_size: Option, +} + +impl SnapshotOptions { + /// Restoring the snapshot gives the sandbox a scratch region of + /// `scratch_size` bytes. By default it keeps the sandbox's current + /// size. + /// + /// A sandbox can boot with the scratch its initialisation needs, then + /// run guest function calls from a snapshot with only the scratch they + /// need, which makes each restore cheaper, or with more scratch than it + /// booted with. + /// + /// A size larger than the maximum memory size (`MemoryRequestTooBig`), + /// too small for the transport (`MemoryRequestTooSmall`), or not a + /// whole number of host pages is rejected before the guest is entered. + /// Room for the page tables depends on the guest's mappings and is + /// checked when the snapshot is built, after a pending transport + /// checkpoint has run in the guest: a size without it fails then, with + /// `MemoryRequestTooSmall`. A larger size whose scratch window would + /// cover a guest mapping below the current one fails then too. Both + /// failures leave the sandbox usable. Whether guest function calls fit + /// is not checked: one that needs more scratch fails in the guest, and + /// a later restore recovers the sandbox. + #[must_use] + pub fn scratch_size(mut self, scratch_size: usize) -> Self { + self.scratch_size = Some(scratch_size); + self + } +} + /// An immutable snapshot of sandbox state. pub struct Snapshot { state: Arc, @@ -463,12 +500,16 @@ impl Snapshot { #[allow(clippy::too_many_arguments)] /// Capture memory with optional validated transport images. /// - /// Page-table and snapshot sizes do not affect transport geometry. + /// `layout` is the sandbox's own: the live page tables and scratch are + /// read with it. The snapshot's page tables are built for + /// `scratch_size`, which can differ from the sandbox's own. A larger + /// scratch window must not cover a guest mapping. Page-table and + /// snapshot sizes do not affect transport geometry. #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn new( shared_mem: &mut SnapshotSharedMemory, scratch_mem: &mut S, - mut layout: SandboxMemoryLayout, + layout: SandboxMemoryLayout, load_info: LoadInfo, regions: Vec, root_pt_gpas: &[u64], @@ -480,9 +521,14 @@ impl Snapshot { snapshot_generation: u64, host_functions: HostFunctionDetails, virtq: Option, + scratch_size: usize, ) -> Result { + let mut target = layout.with_scratch_size(scratch_size)?; let mut phys_seen = HashMap::::new(); let scratch_gva = scratch_base_gva(layout.get_scratch_size()); + // A larger scratch window also covers this band below the current + // one, which `map_specials` would map over the guest's mappings. + let grown_band = scratch_base_gva(scratch_size)..scratch_gva; let memory = shared_mem.with_contents(|snap_c| { scratch_mem.with_contents(|scratch_c| { // Phase 1: walk every PT root together. This detects @@ -516,7 +562,7 @@ impl Snapshot { // spaces' tables. // TODO: Look for opportunities to hugepage map let mut snapshot_memory: Vec = Vec::new(); - let pt_buf = GuestPageTableBuffer::new(layout.get_pt_base_gpa() as usize); + let pt_buf = GuestPageTableBuffer::new(target.get_pt_base_gpa() as usize); // Allocate one root table per space and remember the // addresses returned by `alloc_table` instead of // assuming the buffer's physical layout. @@ -565,6 +611,15 @@ impl Snapshot { MappingKind::Unmapped => continue, MappingKind::ZeroInit(bm) => (MappingKind::ZeroInit(bm), false), }; + if !grown_band.is_empty() + && mapping.virt_base < grown_band.end + && mapping.virt_base + mapping.len > grown_band.start + { + return Err(crate::new_error!( + "a scratch size of {scratch_size} would map scratch over the guest mapping at {:#x}", + mapping.virt_base + )); + } let new_gpa = if has_contents { let Some(contents) = (unsafe { guest_page( @@ -611,7 +666,7 @@ impl Snapshot { // Phase 3: Map the scratch region into each root. for &root_addr in &root_addrs { pt_buf.set_root(root_addr); - map_specials(&pt_buf, layout.get_scratch_size()); + map_specials(&pt_buf, scratch_size); } pt_buf.set_root(pt_buf.initial_root()); @@ -622,7 +677,7 @@ impl Snapshot { // Phase 4: finalize PT bytes. let pt_data = pt_buf.into_bytes(); - layout.set_pt_size(pt_data.len())?; + target.set_pt_size(pt_data.len())?; snapshot_memory.extend(&pt_data); Ok::<_, crate::HyperlightError>(snapshot_memory) }) @@ -633,13 +688,13 @@ impl Snapshot { // mapping of the snapshot region avoids overlap with // `map_file_cow` regions installed immediately after the // snapshot in guest PA space. - let guest_visible_size = memory.len() - layout.get_pt_size(); + let guest_visible_size = memory.len() - target.get_pt_size(); debug_assert!(guest_visible_size.is_multiple_of(page_size::get())); - layout.set_snapshot_size(guest_visible_size); + target.set_snapshot_size(guest_visible_size); Ok(Self { state: Arc::new(SnapshotState { - layout, + layout: target, memory: ReadonlySharedMemory::from_bytes(&memory, guest_visible_size)?, load_info, stack_top_gva, @@ -885,18 +940,25 @@ mod tests { } fn make_simple_pt_mem(contents: &[u8]) -> SnapshotSharedMemory { + make_pt_mem(contents, &[SandboxMemoryLayout::BASE_ADDRESS as u64]) + } + + /// Page tables that map the first snapshot page at each of `gvas`. + fn make_pt_mem(contents: &[u8], gvas: &[u64]) -> SnapshotSharedMemory { let pt_buf = GuestPageTableBuffer::new(simple_pt_base()); - let mapping = Mapping { - phys_base: SandboxMemoryLayout::BASE_ADDRESS as u64, - virt_base: SandboxMemoryLayout::BASE_ADDRESS as u64, - len: page_size::get() as u64, - kind: MappingKind::Basic(BasicMapping { - readable: true, - writable: true, - executable: true, - }), - }; - unsafe { vmem::map(&pt_buf, mapping) }; + for &virt_base in gvas { + let mapping = Mapping { + phys_base: SandboxMemoryLayout::BASE_ADDRESS as u64, + virt_base, + len: page_size::get() as u64, + kind: MappingKind::Basic(BasicMapping { + readable: true, + writable: true, + executable: true, + }), + }; + unsafe { vmem::map(&pt_buf, mapping) }; + } super::map_specials(&pt_buf, PAGE_SIZE); let pt_bytes = pt_buf.into_bytes(); @@ -944,6 +1006,7 @@ mod tests { 1, HostFunctionDetails::default(), None, + mgr.layout.get_scratch_size(), ) .unwrap(); @@ -965,6 +1028,7 @@ mod tests { 2, HostFunctionDetails::default(), None, + mgr.layout.get_scratch_size(), ) .unwrap(); @@ -980,4 +1044,57 @@ mod tests { .with_contents(|contents| assert_eq!(&contents[0..pattern_b.len()], &pattern_b[..])) .unwrap(); } + fn snapshot_with_scratch( + snap_mem: SnapshotSharedMemory, + mgr: &mut SandboxMemoryManager, + pt_base: u64, + scratch_size: usize, + ) -> crate::Result { + super::Snapshot::new( + &mut snap_mem.build().0, + &mut mgr.scratch_mem, + mgr.layout, + LoadInfo::dummy(), + Vec::new(), + &[pt_base], + 0, + default_sregs(), + #[cfg(target_arch = "x86_64")] + Vec::new(), + super::NextAction::None, + 0, + 1, + HostFunctionDetails::default(), + None, + scratch_size, + ) + } + + /// A snapshot can have more scratch than its sandbox, unless the larger + /// window would cover a guest mapping below the current one. + #[test] + fn larger_scratch_must_not_cover_guest_mappings() { + let (mut mgr, pt_base) = make_simple_pt_mgr(); + let scratch_size = mgr.layout.get_scratch_size(); + let larger = scratch_size * 2; + let base = SandboxMemoryLayout::BASE_ADDRESS as u64; + let contents = vec![0u8; page_size::get()]; + + let plain = make_simple_pt_mem(&contents); + let Ok(snapshot) = snapshot_with_scratch(plain, &mut mgr, pt_base, larger) else { + panic!("a larger scratch window over no guest mapping is rejected"); + }; + assert_eq!(snapshot.layout().get_scratch_size(), larger); + + let in_band = super::scratch_base_gva(scratch_size) - PAGE_SIZE as u64; + let covered = make_pt_mem(&contents, &[base, in_band]); + let Err(err) = snapshot_with_scratch(covered, &mut mgr, pt_base, larger) else { + panic!("a larger scratch window covers a guest mapping"); + }; + assert!(err.to_string().contains(&format!("{in_band:#x}")), "{err}"); + + // The same mapping is kept when the scratch window does not grow. + let covered = make_pt_mem(&contents, &[base, in_band]); + assert!(snapshot_with_scratch(covered, &mut mgr, pt_base, scratch_size).is_ok()); + } }