diff --git a/src/hyperlight_common/src/arch/amd64/vmem.rs b/src/hyperlight_common/src/arch/amd64/vmem.rs index 618c56660..9505dda16 100644 --- a/src/hyperlight_common/src/arch/amd64/vmem.rs +++ b/src/hyperlight_common/src/arch/amd64/vmem.rs @@ -456,6 +456,7 @@ unsafe fn map_page< 0 | // R/W - Cow page is never writable PAGE_PRESENT // P - this entry is present } + MappingKind::Unmapped => 0, }; unsafe { write_entry_updating(op, r.update_parent, r.entry_ptr, pte); diff --git a/src/hyperlight_common/src/version_note.rs b/src/hyperlight_common/src/version_note.rs index 41b97f2ed..643dbd8ef 100644 --- a/src/hyperlight_common/src/version_note.rs +++ b/src/hyperlight_common/src/version_note.rs @@ -103,13 +103,13 @@ impl ElfNote { // desc must start at an 8-byte aligned offset from the note start. assert!( - core::mem::offset_of!(Self, desc) % 8 == 0, + core::mem::offset_of!(Self, desc).is_multiple_of(8), "desc is not 8-byte aligned" ); // Total note size must be a multiple of 8 for next-entry alignment. assert!( - size_of::() % 8 == 0, + size_of::().is_multiple_of(8), "total note size is not 8-byte aligned" ); diff --git a/src/hyperlight_common/src/vmem.rs b/src/hyperlight_common/src/vmem.rs index c72a6b9af..96de9f334 100644 --- a/src/hyperlight_common/src/vmem.rs +++ b/src/hyperlight_common/src/vmem.rs @@ -196,6 +196,7 @@ pub struct CowMapping { #[derive(Debug, PartialEq, Clone, Copy)] pub enum MappingKind { + Unmapped, Basic(BasicMapping), Cow(CowMapping), /* TODO: What useful things other than basic mappings actually diff --git a/src/hyperlight_host/build.rs b/src/hyperlight_host/build.rs index d599bedc1..6f3f9587a 100644 --- a/src/hyperlight_host/build.rs +++ b/src/hyperlight_host/build.rs @@ -105,6 +105,10 @@ fn main() -> Result<()> { crashdump: { all(feature = "crashdump", target_arch = "x86_64") }, // print_debug feature is aliased with debug_assertions to make it only available in debug-builds. print_debug: { all(feature = "print_debug", debug_assertions) }, + // the nanvix-unstable and gdb features both (only + // temporarily!) need to use writable/un-shared snapshot + // memories, and so can't share + unshared_snapshot_mem: { any(feature = "nanvix-unstable", feature = "gdb") }, } #[cfg(feature = "build-metadata")] diff --git a/src/hyperlight_host/src/error.rs b/src/hyperlight_host/src/error.rs index 831792cc3..3a1d0955c 100644 --- a/src/hyperlight_host/src/error.rs +++ b/src/hyperlight_host/src/error.rs @@ -104,10 +104,6 @@ pub enum HyperlightError { #[error("Unsupported type: {0}")] GuestInterfaceUnsupportedType(String), - /// The guest offset is invalid. - #[error("The guest offset {0} is invalid.")] - GuestOffsetIsInvalid(usize), - /// The guest binary was built with a different hyperlight-guest-bin version than the host expects. /// Hyperlight currently provides no backwards compatibility guarantees for guest binaries, /// so the guest and host versions must match exactly. This might change in the future. @@ -244,10 +240,6 @@ pub enum HyperlightError { #[error("Failed To Convert Return Value {0:?} to {1:?}")] ReturnValueConversionFailure(ReturnValue, &'static str), - /// Attempted to process a snapshot but the snapshot size does not match the current memory size - #[error("Snapshot Size Mismatch: Memory Size {0:?} Snapshot Size {1:?}")] - SnapshotSizeMismatch(usize, usize), - /// Tried to restore snapshot to a sandbox that is not the same as the one the snapshot was taken from #[error("Snapshot was taken from a different sandbox")] SnapshotSandboxMismatch, @@ -339,7 +331,6 @@ impl HyperlightError { | HyperlightError::PoisonedSandbox | HyperlightError::ExecutionAccessViolation(_) | HyperlightError::MemoryAccessViolation(_, _, _) - | HyperlightError::SnapshotSizeMismatch(_, _) | HyperlightError::MemoryRegionSizeMismatch(_, _, _) // HyperlightVmError::Restore is already handled manually in restore(), but we mark it // as poisoning here too for defense in depth. @@ -369,7 +360,6 @@ impl HyperlightError { | HyperlightError::GuestExecutionHungOnHostFunctionCall() | HyperlightError::GuestFunctionCallAlreadyInProgress() | HyperlightError::GuestInterfaceUnsupportedType(_) - | HyperlightError::GuestOffsetIsInvalid(_) | HyperlightError::HostFunctionNotFound(_) | HyperlightError::HyperlightVmError(HyperlightVmError::Create(_)) | HyperlightError::HyperlightVmError(HyperlightVmError::Initialize(_)) diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 6ef5a3322..a85df0ee2 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -21,7 +21,7 @@ mod x86_64_target; use std::io::{self, ErrorKind}; use std::net::TcpListener; use std::sync::{Arc, Mutex}; -use std::{slice, thread}; +use std::thread; use crossbeam_channel::{Receiver, Sender, TryRecvError}; use event_loop::event_loop_thread; @@ -36,10 +36,10 @@ use super::regs::CommonRegisters; use crate::HyperlightError; use crate::hypervisor::regs::CommonFpu; use crate::hypervisor::virtual_machine::{HypervisorError, RegisterError, VirtualMachine}; -use crate::mem::layout::SandboxMemoryLayout; +use crate::mem::layout::BaseGpaRegion; use crate::mem::memory_region::MemoryRegion; use crate::mem::mgr::SandboxMemoryManager; -use crate::mem::shared_mem::{HostSharedMemory, SharedMemory}; +use crate::mem::shared_mem::HostSharedMemory; #[derive(Debug, Error)] pub enum GdbTargetError { @@ -95,18 +95,11 @@ pub enum DebugMemoryAccessError { LockFailed(&'static str, u32, String), #[error("Failed to translate guest address {0:#x}")] TranslateGuestAddress(u64), + #[error("Failed to write to read-only region")] + WriteToReadOnly, } impl DebugMemoryAccess { - // TODO: There is a lot of common logic between both of these - // functions, as well as guest_page/access_gpa in snapshot.rs. It - // would be nice to factor that out at some point, but the - // snapshot versions deal with ExclusiveSharedMemory, since we - // never expect a guest to be running concurrent with a snapshot, - // and doesn't want to make unnecessary copies, since it runs over - // relatively large volumes of data, so it's not clear if it's - // terribly easy to combine them - /// Reads memory from the guest's address space with a maximum length of a PAGE_SIZE /// /// # Arguments @@ -120,74 +113,17 @@ impl DebugMemoryAccess { data: &mut [u8], gpa: u64, ) -> std::result::Result<(), DebugMemoryAccessError> { - let read_len = data.len(); - - let mem_offset = (gpa as usize) - .checked_sub(SandboxMemoryLayout::BASE_ADDRESS) - .ok_or_else(|| { - log::warn!( - "gpa={:#X} causes subtract with underflow: \"gpa - BASE_ADDRESS={:#X}-{:#X}\"", - gpa, - gpa, - SandboxMemoryLayout::BASE_ADDRESS - ); - DebugMemoryAccessError::TranslateGuestAddress(gpa) - })?; - - // First check the mapped memory regions to see if the address is within any of them - let mut region_found = false; - for reg in self.guest_mmap_regions.iter() { - if reg.guest_region.contains(&mem_offset) { - log::debug!("Found mapped region containing {:X}: {:#?}", gpa, reg); - - // Region found - calculate the offset within the region - let region_offset = mem_offset.checked_sub(reg.guest_region.start).ok_or_else(|| { - log::warn!( - "Cannot calculate offset in memory region: mem_offset={:#X}, base={:#X}", - mem_offset, - reg.guest_region.start, - ); - DebugMemoryAccessError::TranslateGuestAddress(mem_offset as u64) - })?; - - let host_start_ptr = <_ as Into>::into(reg.host_region.start); - let bytes: &[u8] = unsafe { - slice::from_raw_parts(host_start_ptr as *const u8, reg.guest_region.len()) - }; - data[..read_len].copy_from_slice(&bytes[region_offset..region_offset + read_len]); - - region_found = true; - break; - } - } - - if !region_found { - let mut mgr = self - .dbg_mem_access_fn - .try_lock() - .map_err(|e| DebugMemoryAccessError::LockFailed(file!(), line!(), e.to_string()))?; - let scratch_base = - hyperlight_common::layout::scratch_base_gpa(mgr.scratch_mem.mem_size()); - let (mem, offset, name): (&mut HostSharedMemory, _, _) = if gpa >= scratch_base { - ( - &mut mgr.scratch_mem, - (gpa - scratch_base) as usize, - "scratch", - ) - } else { - (&mut mgr.shared_mem, mem_offset, "snapshot") - }; - log::debug!( - "No mapped region found containing {:X}. Trying {} memory at offset {:X} ...", - gpa, - name, - offset - ); - mem.copy_to_slice(&mut data[..read_len], offset) - .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e)))?; - } - - Ok(()) + let mgr = self + .dbg_mem_access_fn + .try_lock() + .map_err(|e| DebugMemoryAccessError::LockFailed(file!(), line!(), e.to_string()))?; + + mgr.layout + .resolve_gpa(gpa, &self.guest_mmap_regions) + .ok_or(DebugMemoryAccessError::TranslateGuestAddress(gpa))? + .with_memories(&mgr.shared_mem, &mgr.scratch_mem) + .copy_to_slice(data) + .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e))) } /// Writes memory from the guest's address space with a maximum length of a PAGE_SIZE @@ -203,74 +139,30 @@ impl DebugMemoryAccess { data: &[u8], gpa: u64, ) -> std::result::Result<(), DebugMemoryAccessError> { - let write_len = data.len(); - - let mem_offset = (gpa as usize) - .checked_sub(SandboxMemoryLayout::BASE_ADDRESS) - .ok_or_else(|| { - log::warn!( - "gpa={:#X} causes subtract with underflow: \"gpa - BASE_ADDRESS={:#X}-{:#X}\"", - gpa, - gpa, - SandboxMemoryLayout::BASE_ADDRESS - ); - DebugMemoryAccessError::TranslateGuestAddress(gpa) - })?; - - // First check the mapped memory regions to see if the address is within any of them - let mut region_found = false; - for reg in self.guest_mmap_regions.iter() { - if reg.guest_region.contains(&mem_offset) { - log::debug!("Found mapped region containing {:X}: {:#?}", gpa, reg); - - // Region found - calculate the offset within the region - let region_offset = mem_offset.checked_sub(reg.guest_region.start).ok_or_else(|| { - log::warn!( - "Cannot calculate offset in memory region: mem_offset={:#X}, base={:#X}", - mem_offset, - reg.guest_region.start, - ); - DebugMemoryAccessError::TranslateGuestAddress(mem_offset as u64) - })?; - - let host_start_ptr = <_ as Into>::into(reg.host_region.start); - let bytes: &mut [u8] = unsafe { - slice::from_raw_parts_mut(host_start_ptr as *mut u8, reg.guest_region.len()) - }; - bytes[region_offset..region_offset + write_len].copy_from_slice(&data[..write_len]); - - region_found = true; - break; - } - } - - if !region_found { - let mut mgr = self - .dbg_mem_access_fn - .try_lock() - .map_err(|e| DebugMemoryAccessError::LockFailed(file!(), line!(), e.to_string()))?; - let scratch_base = - hyperlight_common::layout::scratch_base_gpa(mgr.scratch_mem.mem_size()); - let (mem, offset, name): (&mut HostSharedMemory, _, _) = if gpa >= scratch_base { - ( - &mut mgr.scratch_mem, - (gpa - scratch_base) as usize, - "scratch", - ) - } else { - (&mut mgr.shared_mem, mem_offset, "snapshot") - }; - log::debug!( - "No mapped region found containing {:X}. Trying {} memory at offset {:X} ...", - gpa, - name, - offset - ); - mem.copy_from_slice(&data[..write_len], offset) - .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e)))?; + let mgr = self + .dbg_mem_access_fn + .try_lock() + .map_err(|e| DebugMemoryAccessError::LockFailed(file!(), line!(), e.to_string()))?; + + let resolved = mgr + .layout + .resolve_gpa(gpa, &self.guest_mmap_regions) + .ok_or(DebugMemoryAccessError::TranslateGuestAddress(gpa))?; + + // We can only safely write (without causing UB in the host + // process) if the address is in the scratch region + match resolved.base { + #[cfg(unshared_snapshot_mem)] + BaseGpaRegion::Snapshot(()) => mgr + .shared_mem + .copy_from_slice(data, resolved.offset) + .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e))), + BaseGpaRegion::Scratch(()) => mgr + .scratch_mem + .copy_from_slice(data, resolved.offset) + .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e))), + _ => Err(DebugMemoryAccessError::WriteToReadOnly), } - - Ok(()) } } @@ -490,6 +382,7 @@ mod tests { use hyperlight_testing::dummy_guest_as_string; use super::*; + use crate::mem::layout::SandboxMemoryLayout; use crate::mem::memory_region::{MemoryRegionFlags, MemoryRegionType}; use crate::sandbox::UninitializedSandbox; use crate::sandbox::uninitialized::GuestBinary; diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs index 4fa9fba36..830b856c0 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs @@ -47,7 +47,7 @@ use crate::hypervisor::virtual_machine::{ }; use crate::hypervisor::{InterruptHandle, InterruptHandleImpl}; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags, MemoryRegionType}; -use crate::mem::mgr::SandboxMemoryManager; +use crate::mem::mgr::{SandboxMemoryManager, SnapshotSharedMemory}; use crate::mem::shared_mem::{GuestSharedMemory, HostSharedMemory, SharedMemory}; use crate::metrics::{METRIC_ERRONEOUS_VCPU_KICKS, METRIC_GUEST_CANCELLATION}; use crate::sandbox::host_funcs::FunctionRegistry; @@ -375,7 +375,7 @@ pub(crate) struct HyperlightVm { pub(super) snapshot_slot: u32, // The current snapshot region, used to keep it alive as long as // it is used & when unmapping - pub(super) snapshot_memory: Option, + pub(super) snapshot_memory: Option>, pub(super) scratch_slot: u32, // The slot number used for the scratch region // The current scratch region, used to keep it alive as long as it // is used & when unmapping @@ -460,7 +460,7 @@ impl HyperlightVm { /// Update the snapshot mapping to point to a new GuestSharedMemory pub(crate) fn update_snapshot_mapping( &mut self, - snapshot: GuestSharedMemory, + snapshot: SnapshotSharedMemory, ) -> Result<(), UpdateRegionError> { let guest_base = crate::mem::layout::SandboxMemoryLayout::BASE_ADDRESS as u64; let rgn = snapshot.mapping_at(guest_base, MemoryRegionType::Snapshot); diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs index e8f21e119..c10883ca7 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -74,7 +74,7 @@ impl HyperlightVm { #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] #[allow(clippy::too_many_arguments)] pub(crate) fn new( - snapshot_mem: GuestSharedMemory, + snapshot_mem: SnapshotSharedMemory, scratch_mem: GuestSharedMemory, _pml4_addr: u64, entrypoint: NextAction, @@ -885,7 +885,7 @@ mod tests { use crate::mem::memory_region::{GuestMemoryRegion, MemoryRegionFlags}; use crate::mem::mgr::{GuestPageTableBuffer, SandboxMemoryManager}; use crate::mem::ptr::RawPtr; - use crate::mem::shared_mem::ExclusiveSharedMemory; + use crate::mem::shared_mem::{ExclusiveSharedMemory, ReadonlySharedMemory}; use crate::sandbox::SandboxConfiguration; use crate::sandbox::host_funcs::FunctionRegistry; #[cfg(any(crashdump, gdb))] @@ -1479,20 +1479,22 @@ mod tests { layout.set_pt_size(pt_bytes.len()).unwrap(); let mem_size = layout.get_memory_size().unwrap(); - let mut eshm = ExclusiveSharedMemory::new(mem_size).unwrap(); + let mut snapshot_contents = vec![0u8; mem_size]; let snapshot_pt_start = mem_size - layout.get_pt_size(); - eshm.copy_from_slice(&pt_bytes, snapshot_pt_start).unwrap(); - eshm.copy_from_slice(code, layout.get_guest_code_offset()) - .unwrap(); + snapshot_contents[snapshot_pt_start..].copy_from_slice(&pt_bytes); + snapshot_contents + [layout.get_guest_code_offset()..layout.get_guest_code_offset() + code.len()] + .copy_from_slice(code); + layout.write_peb(&mut snapshot_contents).unwrap(); + let ro_mem = ReadonlySharedMemory::from_bytes(&snapshot_contents).unwrap(); let scratch_mem = ExclusiveSharedMemory::new(config.get_scratch_size()).unwrap(); - let mut mem_mgr = SandboxMemoryManager::new( + let mem_mgr = SandboxMemoryManager::new( layout, - eshm, + ro_mem.to_mgr_snapshot_mem().unwrap(), scratch_mem, NextAction::Initialise(layout.get_guest_code_address() as u64), ); - mem_mgr.write_memory_layout().unwrap(); let (mut hshm, gshm) = mem_mgr.build().unwrap(); diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index 829c61234..94f21057f 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -68,16 +68,151 @@ use tracing::{Span, instrument}; use super::memory_region::MemoryRegionType::{Code, Heap, InitData, Peb}; use super::memory_region::{ - DEFAULT_GUEST_BLOB_MEM_FLAGS, MemoryRegion_, MemoryRegionFlags, MemoryRegionKind, + DEFAULT_GUEST_BLOB_MEM_FLAGS, MemoryRegion, MemoryRegion_, MemoryRegionFlags, MemoryRegionKind, MemoryRegionVecBuilder, }; -use super::shared_mem::{ExclusiveSharedMemory, SharedMemory}; -use crate::error::HyperlightError::{ - GuestOffsetIsInvalid, MemoryRequestTooBig, MemoryRequestTooSmall, -}; +#[cfg(any(gdb, feature = "mem_profile"))] +use super::shared_mem::HostSharedMemory; +use super::shared_mem::{ExclusiveSharedMemory, ReadonlySharedMemory}; +use crate::error::HyperlightError::{MemoryRequestTooBig, MemoryRequestTooSmall}; use crate::sandbox::SandboxConfiguration; use crate::{Result, new_error}; +pub(crate) enum BaseGpaRegion { + Snapshot(Sn), + Scratch(Sc), + Mmap(MemoryRegion), +} + +// It's an invariant of this type, checked on creation, that the +// offset is in bounds for the base region. +pub(crate) struct ResolvedGpa { + pub(crate) offset: usize, + pub(crate) base: BaseGpaRegion, +} + +impl AsRef<[u8]> for ExclusiveSharedMemory { + fn as_ref(&self) -> &[u8] { + self.as_slice() + } +} +impl AsRef<[u8]> for ReadonlySharedMemory { + fn as_ref(&self) -> &[u8] { + self.as_slice() + } +} + +impl ResolvedGpa { + pub(crate) fn with_memories(self, sn: Sn2, sc: Sc2) -> ResolvedGpa { + ResolvedGpa { + offset: self.offset, + base: match self.base { + BaseGpaRegion::Snapshot(_) => BaseGpaRegion::Snapshot(sn), + BaseGpaRegion::Scratch(_) => BaseGpaRegion::Scratch(sc), + BaseGpaRegion::Mmap(r) => BaseGpaRegion::Mmap(r), + }, + } + } +} +impl<'a> BaseGpaRegion<&'a [u8], &'a [u8]> { + pub(crate) fn as_ref<'b>(&'b self) -> &'a [u8] { + match self { + BaseGpaRegion::Snapshot(sn) => sn, + BaseGpaRegion::Scratch(sc) => sc, + BaseGpaRegion::Mmap(r) => unsafe { + #[allow(clippy::useless_conversion)] + let host_region_base: usize = r.host_region.start.into(); + #[allow(clippy::useless_conversion)] + let host_region_end: usize = r.host_region.end.into(); + let len = host_region_end - host_region_base; + std::slice::from_raw_parts(host_region_base as *const u8, len) + }, + } + } +} +impl<'a> ResolvedGpa<&'a [u8], &'a [u8]> { + pub(crate) fn as_ref<'b>(&'b self) -> &'a [u8] { + let base = self.base.as_ref(); + if self.offset > base.len() { + return &[]; + } + &self.base.as_ref()[self.offset..] + } +} +#[cfg(any(gdb, feature = "mem_profile"))] +#[allow(unused)] // may be unused when nanvix-unstable is also enabled +pub(crate) trait ReadableSharedMemory { + fn copy_to_slice(&self, slice: &mut [u8], offset: usize) -> Result<()>; +} +#[cfg(any(gdb, feature = "mem_profile"))] +impl ReadableSharedMemory for &HostSharedMemory { + fn copy_to_slice(&self, slice: &mut [u8], offset: usize) -> Result<()> { + HostSharedMemory::copy_to_slice(self, slice, offset) + } +} +#[cfg(any(gdb, feature = "mem_profile"))] +mod coherence_hack { + use super::{ExclusiveSharedMemory, ReadonlySharedMemory}; + #[allow(unused)] // it actually is; see the impl below + pub(super) trait SharedMemoryAsRefMarker: AsRef<[u8]> {} + impl SharedMemoryAsRefMarker for ExclusiveSharedMemory {} + impl SharedMemoryAsRefMarker for &ExclusiveSharedMemory {} + impl SharedMemoryAsRefMarker for ReadonlySharedMemory {} + impl SharedMemoryAsRefMarker for &ReadonlySharedMemory {} +} +#[cfg(any(gdb, feature = "mem_profile"))] +impl ReadableSharedMemory for T { + fn copy_to_slice(&self, slice: &mut [u8], offset: usize) -> Result<()> { + let ss: &[u8] = self.as_ref(); + let end = offset + slice.len(); + if end > ss.len() { + return Err(new_error!( + "Attempt to read up to {} in memory of size {}", + offset + slice.len(), + self.as_ref().len() + )); + } + slice.copy_from_slice(&ss[offset..end]); + Ok(()) + } +} +#[cfg(any(gdb, feature = "mem_profile"))] +impl ResolvedGpa { + #[allow(unused)] // may be unused when nanvix-unstable is also enabled + pub(crate) fn copy_to_slice(&self, slice: &mut [u8]) -> Result<()> { + match &self.base { + BaseGpaRegion::Snapshot(sn) => sn.copy_to_slice(slice, self.offset), + BaseGpaRegion::Scratch(sc) => sc.copy_to_slice(slice, self.offset), + BaseGpaRegion::Mmap(r) => unsafe { + #[allow(clippy::useless_conversion)] + let host_region_base: usize = r.host_region.start.into(); + #[allow(clippy::useless_conversion)] + let host_region_end: usize = r.host_region.end.into(); + let len = host_region_end - host_region_base; + // Safety: it's a documented invariant of MemoryRegion + // that the memory must remain alive as long as the + // sandbox is alive, and the way this code is used, + // the lifetimes of the snapshot and scratch memories + // ensure that the sandbox is still alive. This could + // perhaps be cleaned up/improved/made harder to + // misuse significantly, but it would require a much + // larger rework. + let ss = std::slice::from_raw_parts(host_region_base as *const u8, len); + let end = self.offset + slice.len(); + if end > ss.len() { + return Err(new_error!( + "Attempt to read up to {} in memory of size {}", + self.offset + slice.len(), + ss.len() + )); + } + slice.copy_from_slice(&ss[self.offset..end]); + Ok(()) + }, + } + } +} + #[derive(Copy, Clone)] pub(crate) struct SandboxMemoryLayout { pub(super) sandbox_memory_config: SandboxConfiguration, @@ -109,6 +244,9 @@ pub(crate) struct SandboxMemoryLayout { // The size of the scratch region in physical memory; note that // this will appear under the top of physical memory. scratch_size: usize, + // The size of the snapshot region in physical memory; note that + // this will appear somewhere near the base of physical memory. + snapshot_size: usize, } impl Debug for SandboxMemoryLayout { @@ -239,8 +377,7 @@ impl SandboxMemoryLayout { // make sure init data starts at 4K boundary let init_data_offset = (guest_heap_buffer_offset + heap_size).next_multiple_of(PAGE_SIZE_USIZE); - - Ok(Self { + let mut ret = Self { peb_offset, heap_size, peb_input_data_offset, @@ -258,7 +395,10 @@ impl SandboxMemoryLayout { init_data_permissions, pt_size: None, scratch_size, - }) + snapshot_size: 0, + }; + ret.set_snapshot_size(ret.get_memory_size()?); + Ok(ret) } /// Get the offset in guest memory to the output data size @@ -450,10 +590,17 @@ impl SandboxMemoryLayout { if self.scratch_size < min_scratch { return Err(MemoryRequestTooSmall(self.scratch_size, min_scratch)); } + let old_pt_size = self.pt_size.unwrap_or(0); + self.snapshot_size = self.snapshot_size - old_pt_size + size; self.pt_size = Some(size); Ok(()) } + #[instrument(skip_all, parent = Span::current(), level= "Trace")] + pub(crate) fn set_snapshot_size(&mut self, new_size: usize) { + self.snapshot_size = new_size; + } + /// Get the size of the memory region used for page tables #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn get_pt_size(&self) -> usize { @@ -562,74 +709,76 @@ impl SandboxMemoryLayout { Ok(()) } - /// Write the finished memory layout to `shared_mem` and return - /// `Ok` if successful. + /// Write the finished memory layout to `mem` and return `Ok` if + /// successful. /// - /// Note: `shared_mem` may have been modified, even if `Err` was returned + /// Note: `mem` may have been modified, even if `Err` was returned /// from this function. #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn write( - &self, - shared_mem: &mut ExclusiveSharedMemory, - guest_offset: usize, - //TODO: Unused remove - _size: usize, - ) -> Result<()> { + pub(crate) fn write_peb(&self, mem: &mut [u8]) -> Result<()> { + let guest_offset = SandboxMemoryLayout::BASE_ADDRESS; + + fn write_u64(mem: &mut [u8], offset: usize, value: u64) -> Result<()> { + if offset + 8 > mem.len() { + return Err(new_error!( + "Cannot write to offset {} in slice of len {}", + offset, + mem.len() + )); + } + mem[offset..offset + 8].copy_from_slice(&u64::to_ne_bytes(value)); + Ok(()) + } + macro_rules! get_address { ($something:ident) => { u64::try_from(guest_offset + self.$something)? }; } - if guest_offset != SandboxMemoryLayout::BASE_ADDRESS - && guest_offset != shared_mem.base_addr() - { - return Err(GuestOffsetIsInvalid(guest_offset)); - } - // Start of setting up the PEB. The following are in the order of the PEB fields - // Skip guest_dispatch_function_ptr_offset because it is set by the guest - - // Skip code, is set when loading binary - // skip outb and outb context, is set when running in_proc - // Set up input buffer pointer - shared_mem.write_u64( + write_u64( + mem, self.get_input_data_size_offset(), self.sandbox_memory_config .get_input_data_size() .try_into()?, )?; - shared_mem.write_u64( + write_u64( + mem, self.get_input_data_pointer_offset(), self.get_input_data_buffer_gva(), )?; // Set up output buffer pointer - shared_mem.write_u64( + write_u64( + mem, self.get_output_data_size_offset(), self.sandbox_memory_config .get_output_data_size() .try_into()?, )?; - shared_mem.write_u64( + write_u64( + mem, self.get_output_data_pointer_offset(), self.get_output_data_buffer_gva(), )?; // Set up init data pointer - shared_mem.write_u64( + write_u64( + mem, self.get_init_data_size_offset(), (self.get_unaligned_memory_size() - self.init_data_offset).try_into()?, )?; let addr = get_address!(init_data_offset); - shared_mem.write_u64(self.get_init_data_pointer_offset(), addr)?; + write_u64(mem, self.get_init_data_pointer_offset(), addr)?; // Set up heap buffer pointer let addr = get_address!(guest_heap_buffer_offset); - shared_mem.write_u64(self.get_heap_size_offset(), self.heap_size.try_into()?)?; - shared_mem.write_u64(self.get_heap_pointer_offset(), addr)?; + write_u64(mem, self.get_heap_size_offset(), self.heap_size.try_into()?)?; + write_u64(mem, self.get_heap_pointer_offset(), addr)?; // Set up the file_mappings descriptor in the PEB. // - The `size` field holds the number of valid FileMappingInfo @@ -652,6 +801,37 @@ impl SandboxMemoryLayout { Ok(()) } + + /// Determine what region this gpa is in, and its offset into that region + pub(crate) fn resolve_gpa( + &self, + gpa: u64, + mmap_regions: &[MemoryRegion], + ) -> Option> { + let scratch_base = hyperlight_common::layout::scratch_base_gpa(self.scratch_size); + if gpa >= scratch_base && gpa < scratch_base + self.scratch_size as u64 { + return Some(ResolvedGpa { + offset: (gpa - scratch_base) as usize, + base: BaseGpaRegion::Scratch(()), + }); + } else if gpa >= SandboxMemoryLayout::BASE_ADDRESS as u64 + && gpa < SandboxMemoryLayout::BASE_ADDRESS as u64 + self.snapshot_size as u64 + { + return Some(ResolvedGpa { + offset: gpa as usize - SandboxMemoryLayout::BASE_ADDRESS, + base: BaseGpaRegion::Snapshot(()), + }); + } + for rgn in mmap_regions { + if gpa >= rgn.guest_region.start as u64 && gpa < rgn.guest_region.end as u64 { + return Some(ResolvedGpa { + offset: gpa as usize - rgn.guest_region.start, + base: BaseGpaRegion::Mmap(rgn.clone()), + }); + } + } + None + } } #[cfg(test)] diff --git a/src/hyperlight_host/src/mem/memory_region.rs b/src/hyperlight_host/src/mem/memory_region.rs index 4fc15c2ba..979b260dd 100644 --- a/src/hyperlight_host/src/mem/memory_region.rs +++ b/src/hyperlight_host/src/mem/memory_region.rs @@ -185,6 +185,12 @@ pub trait MemoryRegionKind { /// Type for memory regions that track both host and guest addresses. /// +/// When one of these is created, it always ends up in a sandbox +/// quickly. It's an invariant of this type that as long as one of +/// these is associated with a sandbox, it's always acceptable to read +/// from it, since a lot of the debug/crashdump/snapshot code +/// does. (Note: this means that _writable_ HostGuestMemoryRegions are +/// not possible to support at the moment). #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] pub struct HostGuestMemoryRegion {} @@ -323,7 +329,7 @@ impl MemoryRegionKind for CrashDumpMemoryRegion { #[cfg(crashdump)] pub(crate) type CrashDumpRegion = MemoryRegion_; -#[cfg(crashdump)] +#[cfg(all(crashdump, feature = "nanvix-unstable"))] impl HostGuestMemoryRegion { /// Extract the raw `usize` host address from the platform-specific /// host base type. diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index b3bd1177f..1a44b822e 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -27,13 +27,13 @@ use hyperlight_common::vmem::{BasicMapping, MappingKind}; use tracing::{Span, instrument}; use super::layout::SandboxMemoryLayout; -use super::shared_mem::{ExclusiveSharedMemory, GuestSharedMemory, HostSharedMemory, SharedMemory}; +use super::shared_mem::{ + ExclusiveSharedMemory, GuestSharedMemory, HostSharedMemory, ReadonlySharedMemory, SharedMemory, +}; use crate::hypervisor::regs::CommonSpecialRegisters; use crate::mem::memory_region::MemoryRegion; #[cfg(crashdump)] -use crate::mem::memory_region::{ - CrashDumpRegion, HostGuestMemoryRegion, MemoryRegionFlags, MemoryRegionType, -}; +use crate::mem::memory_region::{CrashDumpRegion, MemoryRegionFlags, MemoryRegionType}; use crate::sandbox::snapshot::{NextAction, Snapshot}; use crate::{Result, new_error}; @@ -67,6 +67,7 @@ fn mapping_kind_to_flags(kind: &MappingKind) -> (MemoryRegionFlags, MemoryRegion } (flags, MemoryRegionType::Scratch) } + MappingKind::Unmapped => (MemoryRegionFlags::empty(), MemoryRegionType::Snapshot), } } @@ -94,46 +95,48 @@ fn try_coalesce_region( false } -/// Check dynamic mmap regions for a GPA that wasn't found in snapshot/scratch, -/// and push matching regions. -#[cfg(all(feature = "crashdump", not(feature = "nanvix-unstable")))] -fn resolve_from_mmap_regions( - regions: &mut Vec, - mapping: &hyperlight_common::vmem::Mapping, - virt_base: usize, - virt_end: usize, - mmap_regions: &[MemoryRegion], -) { - let phys_start = mapping.phys_base as usize; - let phys_end = (mapping.phys_base + mapping.len) as usize; - - for rgn in mmap_regions { - if phys_start >= rgn.guest_region.start && phys_end <= rgn.guest_region.end { - let offset = phys_start - rgn.guest_region.start; - let host_base = HostGuestMemoryRegion::to_addr(rgn.host_region.start) + offset; - let host_end = host_base + mapping.len as usize; - let flags = rgn.flags; - - if try_coalesce_region(regions, virt_base, virt_end, host_base, flags) { - continue; - } - - regions.push(CrashDumpRegion { - guest_region: virt_base..virt_end, - host_region: host_base..host_end, - flags, - region_type: rgn.region_type, - }); - } +// It would be nice to have a simple type alias +// `SnapshotSharedMemory` that abstracts over the +// fact that the snapshot shared memory is `ReadonlySharedMemory` +// normally, but there is (temporary) support for writable +// `GuestSharedMemory` with `#[cfg(feature = +// "nanvix-unstable")]`. Unfortunately, rustc gets annoyed about an +// unused type parameter, unless one goes to a little bit of effort to +// trick it... +mod unused_hack { + #[cfg(not(unshared_snapshot_mem))] + use crate::mem::shared_mem::ReadonlySharedMemory; + use crate::mem::shared_mem::SharedMemory; + pub trait SnapshotSharedMemoryT { + type T; + } + pub struct SnapshotSharedMemory_; + impl SnapshotSharedMemoryT for SnapshotSharedMemory_ { + #[cfg(not(unshared_snapshot_mem))] + type T = ReadonlySharedMemory; + #[cfg(unshared_snapshot_mem)] + type T = S; + } + pub type SnapshotSharedMemory = ::T; +} +impl ReadonlySharedMemory { + pub(crate) fn to_mgr_snapshot_mem( + &self, + ) -> Result> { + #[cfg(not(unshared_snapshot_mem))] + let ret = self.clone(); + #[cfg(unshared_snapshot_mem)] + let ret = self.copy_to_writable()?; + Ok(ret) } } - +pub(crate) use unused_hack::SnapshotSharedMemory; /// A struct that is responsible for laying out and managing the memory /// for a given `Sandbox`. #[derive(Clone)] -pub(crate) struct SandboxMemoryManager { +pub(crate) struct SandboxMemoryManager { /// Shared memory for the Sandbox - pub(crate) shared_mem: S, + pub(crate) shared_mem: SnapshotSharedMemory, /// Scratch memory for the Sandbox pub(crate) scratch_mem: S, /// The memory layout of the underlying shared memory @@ -242,7 +245,7 @@ where #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn new( layout: SandboxMemoryLayout, - shared_mem: S, + shared_mem: SnapshotSharedMemory, scratch_mem: S, entrypoint: NextAction, ) -> Self { @@ -261,12 +264,6 @@ where &mut self.abort_buffer } - /// Get `SharedMemory` in `self` as a mutable reference - #[cfg(test)] - pub(crate) fn get_shared_mem_mut(&mut self) -> &mut S { - &mut self.shared_mem - } - /// Create a snapshot with the given mapped regions pub(crate) fn snapshot( &mut self, @@ -295,24 +292,12 @@ where impl SandboxMemoryManager { pub(crate) fn from_snapshot(s: &Snapshot) -> Result { let layout = *s.layout(); - let mut shared_mem = ExclusiveSharedMemory::new(s.mem_size())?; - shared_mem.copy_from_slice(s.memory(), 0)?; + let shared_mem = s.memory().to_mgr_snapshot_mem()?; let scratch_mem = ExclusiveSharedMemory::new(s.layout().get_scratch_size())?; let entrypoint = s.entrypoint(); Ok(Self::new(layout, shared_mem, scratch_mem, entrypoint)) } - /// Write memory layout - #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn write_memory_layout(&mut self) -> Result<()> { - let mem_size = self.shared_mem.mem_size(); - self.layout.write( - &mut self.shared_mem, - SandboxMemoryLayout::BASE_ADDRESS, - mem_size, - ) - } - /// Wraps ExclusiveSharedMemory::build // Morally, this should not have to be a Result: this operation is // infallible. The source of the Result is @@ -500,16 +485,25 @@ impl SandboxMemoryManager { pub(crate) fn restore_snapshot( &mut self, snapshot: &Snapshot, - ) -> Result<(Option, Option)> { - let gsnapshot = if self.shared_mem.mem_size() == snapshot.mem_size() { + ) -> Result<( + Option>, + Option, + )> { + let gsnapshot = if *snapshot.memory() == self.shared_mem { + // If the snapshot memory is already the correct memory, + // which is readonly, don't bother with restoring it, + // since its contents must be the same. Note that in the + // #[cfg(unshared_snapshot_mem)] case, this condition will + // never be true, since even immediately after a restore, + // self.shared_mem is a (writable) copy, not the original + // shared_mem. None } else { - let new_snapshot_mem = ExclusiveSharedMemory::new(snapshot.mem_size())?; + let new_snapshot_mem = snapshot.memory().to_mgr_snapshot_mem()?; let (hsnapshot, gsnapshot) = new_snapshot_mem.build(); self.shared_mem = hsnapshot; Some(gsnapshot) }; - self.shared_mem.restore_from_snapshot(snapshot)?; let new_scratch_size = snapshot.layout().get_scratch_size(); let gscratch = if new_scratch_size == self.scratch_mem.mem_size() { self.scratch_mem.zero()?; @@ -560,13 +554,21 @@ impl SandboxMemoryManager { // Copy the page tables into the scratch region let snapshot_pt_end = self.shared_mem.mem_size(); - let snapshot_pt_start = snapshot_pt_end - self.layout.get_pt_size(); - self.shared_mem.with_exclusivity(|snap| { - self.scratch_mem.with_exclusivity(|scratch| { - let bytes = &snap.as_slice()[snapshot_pt_start..snapshot_pt_end]; - scratch.copy_from_slice(bytes, self.layout.get_pt_base_scratch_offset()) - }) - })???; + let snapshot_pt_size = self.layout.get_pt_size(); + let snapshot_pt_start = snapshot_pt_end - snapshot_pt_size; + self.scratch_mem.with_exclusivity(|scratch| { + #[cfg(not(unshared_snapshot_mem))] + let bytes = &self.shared_mem.as_slice()[snapshot_pt_start..snapshot_pt_end]; + #[cfg(unshared_snapshot_mem)] + let bytes = { + let mut bytes = vec![0u8; snapshot_pt_size]; + self.shared_mem + .copy_to_slice(&mut bytes, snapshot_pt_start)?; + bytes + }; + #[allow(clippy::needless_borrow)] + scratch.copy_from_slice(&bytes, self.layout.get_pt_base_scratch_offset()) + })??; Ok(()) } @@ -581,24 +583,21 @@ impl SandboxMemoryManager { root_pt: u64, mmap_regions: &[MemoryRegion], ) -> Result> { - use crate::sandbox::snapshot::{SharedMemoryPageTableBuffer, access_gpa}; + use crate::sandbox::snapshot::SharedMemoryPageTableBuffer; - let scratch_size = self.scratch_mem.mem_size(); let len = hyperlight_common::layout::MAX_GVA; - let regions = self.shared_mem.with_exclusivity(|snapshot| { - self.scratch_mem.with_exclusivity(|scratch| { + let regions = self.shared_mem.with_contents(|snapshot| { + self.scratch_mem.with_contents(|scratch| { let pt_buf = - SharedMemoryPageTableBuffer::new(snapshot, scratch, scratch_size, root_pt); + SharedMemoryPageTableBuffer::new(snapshot, scratch, self.layout, root_pt); let mappings: Vec<_> = unsafe { hyperlight_common::vmem::virt_to_phys(&pt_buf, 0, len as u64) } .collect(); if mappings.is_empty() { - return Err(new_error!( - "No page table mappings found (len {len})", - )); + return Err(new_error!("No page table mappings found (len {len})",)); } let mut regions: Vec = Vec::new(); @@ -606,22 +605,13 @@ impl SandboxMemoryManager { let virt_base = mapping.virt_base as usize; let virt_end = (mapping.virt_base + mapping.len) as usize; - if let Some((mem, offset)) = - access_gpa(snapshot, scratch, scratch_size, mapping.phys_base) + if let Some(resolved) = self.layout.resolve_gpa(mapping.phys_base, mmap_regions) { let (flags, region_type) = mapping_kind_to_flags(&mapping.kind); - - if offset >= mem.mem_size() { - tracing::error!( - "Mapping for GPA {:#x} offset {:#x} out of bounds (size {:#x}), skipping", - mapping.phys_base, offset, mem.mem_size(), - ); - continue; - } - - let host_base = mem.base_addr() + offset; - let host_len = - (mapping.len as usize).min(mem.mem_size().saturating_sub(offset)); + let resolved = resolved.with_memories(snapshot, scratch); + let contents = resolved.as_ref(); + let host_base = contents.as_ptr() as usize; + let host_len = (mapping.len as usize).min(contents.len()); if try_coalesce_region(&mut regions, virt_base, virt_end, host_base, flags) { @@ -634,11 +624,6 @@ impl SandboxMemoryManager { flags, region_type, }); - } else { - // GPA not in snapshot/scratch — check dynamic mmap regions - resolve_from_mmap_regions( - &mut regions, mapping, virt_base, virt_end, mmap_regions, - ); } } @@ -660,6 +645,8 @@ impl SandboxMemoryManager { _root_pt: u64, mmap_regions: &[MemoryRegion], ) -> Result> { + use crate::mem::memory_region::HostGuestMemoryRegion; + let snapshot_base = SandboxMemoryLayout::BASE_ADDRESS; let snapshot_size = self.shared_mem.mem_size(); let snapshot_host = self.shared_mem.base_addr(); @@ -720,11 +707,9 @@ impl SandboxMemoryManager { use crate::sandbox::snapshot::{SharedMemoryPageTableBuffer, access_gpa}; - let scratch_size = self.scratch_mem.mem_size(); - - self.shared_mem.with_exclusivity(|snap| { - self.scratch_mem.with_exclusivity(|scratch| { - let pt_buf = SharedMemoryPageTableBuffer::new(snap, scratch, scratch_size, root_pt); + self.shared_mem.with_contents(|snap| { + self.scratch_mem.with_contents(|scratch| { + let pt_buf = SharedMemoryPageTableBuffer::new(snap, scratch, self.layout, root_pt); // Walk page tables to get all mappings that cover the GVA range let mappings: Vec<_> = unsafe { @@ -764,7 +749,7 @@ impl SandboxMemoryManager { // Translate the GPA to host memory let gpa = mapping.phys_base + page_offset as u64; - let (mem, offset) = access_gpa(snap, scratch, scratch_size, gpa) + let (mem, offset) = access_gpa(snap, scratch, self.layout, gpa) .ok_or_else(|| { new_error!( "Failed to resolve GPA {:#x} to host memory (GVA {:#x})", @@ -774,7 +759,6 @@ impl SandboxMemoryManager { })?; let slice = mem - .as_slice() .get(offset..offset + bytes_to_copy) .ok_or_else(|| { new_error!( diff --git a/src/hyperlight_host/src/mem/shared_mem.rs b/src/hyperlight_host/src/mem/shared_mem.rs index 5c4a1a63f..527322cc9 100644 --- a/src/hyperlight_host/src/mem/shared_mem.rs +++ b/src/hyperlight_host/src/mem/shared_mem.rs @@ -39,10 +39,8 @@ use windows::core::PCSTR; use super::memory_region::{ HostGuestMemoryRegion, MemoryRegion, MemoryRegionFlags, MemoryRegionKind, MemoryRegionType, }; -use crate::HyperlightError::SnapshotSizeMismatch; #[cfg(target_os = "windows")] use crate::HyperlightError::WindowsAPIError; -use crate::sandbox::snapshot::Snapshot; use crate::{HyperlightError, Result, log_then_return, new_error}; /// Makes sure that the given `offset` and `size` are within the bounds of the memory with size `mem_size`. @@ -680,6 +678,28 @@ impl ExclusiveSharedMemory { } } +fn mapping_at( + s: &impl SharedMemory, + gpa: u64, + region_type: MemoryRegionType, + flags: MemoryRegionFlags, +) -> MemoryRegion { + let guest_base = gpa as usize; + + #[cfg(not(windows))] + let host_base = s.base_addr(); + #[cfg(windows)] + let host_base = s.host_region_base(); + let host_end = ::add(host_base, s.mem_size()); + + MemoryRegion { + guest_region: guest_base..(guest_base + s.mem_size()), + host_region: host_base..host_end, + region_type, + flags, + } +} + impl GuestSharedMemory { /// Create a [`super::memory_region::MemoryRegion`] structure /// suitable for mapping this region into a VM @@ -692,46 +712,24 @@ impl GuestSharedMemory { MemoryRegionType::Scratch => { MemoryRegionFlags::READ | MemoryRegionFlags::WRITE | MemoryRegionFlags::EXECUTE } - // Without nanvix-unstable (default), the snapshot is read-only - // because guest page tables provide CoW semantics for writable - // pages. With nanvix-unstable there are no guest page tables, - // so the snapshot must be writable — otherwise writes (including - // the CPU setting the "Accessed" bit in GDT descriptors during - // segment loads) cause EPT violations that KVM retries forever. + #[cfg(unshared_snapshot_mem)] MemoryRegionType::Snapshot => { - #[cfg(not(feature = "nanvix-unstable"))] - { - MemoryRegionFlags::READ | MemoryRegionFlags::EXECUTE - } - #[cfg(feature = "nanvix-unstable")] - { - MemoryRegionFlags::READ | MemoryRegionFlags::WRITE | MemoryRegionFlags::EXECUTE - } + MemoryRegionFlags::READ | MemoryRegionFlags::WRITE | MemoryRegionFlags::EXECUTE } #[allow(clippy::panic)] - // In the future, all the host side knowledge about memory - // region types should collapse down to Snapshot vs - // Scratch, at which time this panicking case will be - // unnecessary. For now, we will panic if one of the - // legacy regions ends up in this function, which very - // much should be impossible (since the only callers of it - // directly pass a literal new-style region type). + // This will not ever actually panic: the only places this + // is called are HyperlightVm::update_snapshot_mapping and + // HyperlightVm::update_scratch_mapping. The latter + // statically uses the Scratch region type, and the former + // does not use this at all when the unshared_snapshot_mem + // feature is not set, since in that case the scratch + // mapping type is ReadonlySharedMemory, not + // GuestSharedMemory. _ => panic!( "GuestSharedMemory::mapping_at should only be used for Scratch or Snapshot regions" ), }; - let guest_base = guest_base as usize; - #[cfg(not(windows))] - let host_base = self.base_addr(); - #[cfg(windows)] - let host_base = self.host_region_base(); - let host_end = ::add(host_base, self.mem_size()); - MemoryRegion { - guest_region: guest_base..(guest_base + self.mem_size()), - host_region: host_base..host_end, - region_type, - flags, - } + mapping_at(self, guest_base, region_type, flags) } } @@ -800,12 +798,13 @@ pub trait SharedMemory { f: F, ) -> Result; - /// Restore a SharedMemory from a snapshot with matching size - fn restore_from_snapshot(&mut self, snapshot: &Snapshot) -> Result<()> { - if snapshot.memory().len() != self.mem_size() { - return Err(SnapshotSizeMismatch(self.mem_size(), snapshot.mem_size())); - } - self.with_exclusivity(|e| e.copy_from_slice(snapshot.memory(), 0))? + /// Run some code that is allowed to access the contents of the + /// SharedMemory as if it is a normal slice. By default, this is + /// implemented via [`SharedMemory::with_exclusivity`], which is + /// the correct implementation for a memory that can be mutated, + /// but a [`ReadonlySharedMemory`], can support this. + fn with_contents T>(&mut self, f: F) -> Result { + self.with_exclusivity(|m| f(m.as_slice())) } /// Zero a shared memory region @@ -1990,3 +1989,97 @@ mod tests { } } } + +/// A ReadonlySharedMemory is a different kind of shared memory, +/// separate from the exclusive/host/guest lifecycle, used to +/// represent read-only mappings of snapshot pages into the guest +/// efficiently. +#[derive(Clone, Debug)] +pub struct ReadonlySharedMemory { + region: Arc, +} +// Safety: HostMapping is only non-Send/Sync (causing +// ReadonlySharedMemory to not be automatically Send/Sync) because raw +// pointers are not ("as a lint", as the Rust docs say). We don't want +// to mark HostMapping Send/Sync immediately, because that could +// socially imply that it's "safe" to use unsafe accesses from +// multiple threads at once in more cases, including ones that don't +// actually ensure immutability/synchronisation. Since +// ReadonlySharedMemory can only be accessed by reading, and reading +// concurrently from multiple threads is not racy, +// ReadonlySharedMemory can be Send and Sync. +unsafe impl Send for ReadonlySharedMemory {} +unsafe impl Sync for ReadonlySharedMemory {} + +impl ReadonlySharedMemory { + pub(crate) fn from_bytes(contents: &[u8]) -> Result { + let mut anon = ExclusiveSharedMemory::new(contents.len())?; + anon.copy_from_slice(contents, 0)?; + Ok(ReadonlySharedMemory { + region: anon.region, + }) + } + + pub(crate) fn as_slice(&self) -> &[u8] { + unsafe { std::slice::from_raw_parts(self.base_ptr(), self.mem_size()) } + } + + #[cfg(unshared_snapshot_mem)] + pub(crate) fn copy_to_writable(&self) -> Result { + let mut writable = ExclusiveSharedMemory::new(self.mem_size())?; + writable.copy_from_slice(self.as_slice(), 0)?; + Ok(writable) + } + + #[cfg(not(unshared_snapshot_mem))] + pub(crate) fn build(self) -> (Self, Self) { + (self.clone(), self) + } + + #[cfg(not(unshared_snapshot_mem))] + pub(crate) fn mapping_at( + &self, + guest_base: u64, + region_type: MemoryRegionType, + ) -> MemoryRegion { + #[allow(clippy::panic)] + // This will not ever actually panic: the only place this is + // called is HyperlightVm::update_snapshot_mapping, which + // always calls it with the Snapshot region type. + if region_type != MemoryRegionType::Snapshot { + panic!("ReadonlySharedMemory::mapping_at should only be used for Snapshot regions"); + } + mapping_at( + self, + guest_base, + region_type, + MemoryRegionFlags::READ | MemoryRegionFlags::EXECUTE, + ) + } +} + +impl SharedMemory for ReadonlySharedMemory { + fn region(&self) -> &HostMapping { + &self.region + } + // There's no way to get exclusive (and therefore writable) access + // to a ReadonlySharedMemory. + fn with_exclusivity T>( + &mut self, + _: F, + ) -> Result { + Err(new_error!( + "Cannot take exclusive access to a ReadonlySharedMemory" + )) + } + // However, just access to the contents as a slice is doable + fn with_contents T>(&mut self, f: F) -> Result { + Ok(f(self.as_slice())) + } +} + +impl PartialEq for ReadonlySharedMemory { + fn eq(&self, other: &S) -> bool { + self.raw_ptr() == other.raw_ptr() + } +} diff --git a/src/hyperlight_host/src/sandbox/outb.rs b/src/hyperlight_host/src/sandbox/outb.rs index 76ec53cee..9704a1fe3 100644 --- a/src/hyperlight_host/src/sandbox/outb.rs +++ b/src/hyperlight_host/src/sandbox/outb.rs @@ -245,9 +245,7 @@ mod tests { use super::outb_log; use crate::GuestBinary; - use crate::mem::layout::SandboxMemoryLayout; use crate::mem::mgr::SandboxMemoryManager; - use crate::mem::shared_mem::SharedMemory; use crate::sandbox::SandboxConfiguration; use crate::sandbox::outb::GuestLogData; use crate::testing::log_values::test_value_as_str; @@ -274,13 +272,7 @@ mod tests { let new_mgr = || { let bin = GuestBinary::FilePath(simple_guest_as_string().unwrap()); let snapshot = crate::sandbox::snapshot::Snapshot::from_env(bin, sandbox_cfg).unwrap(); - let mut mgr = SandboxMemoryManager::from_snapshot(&snapshot).unwrap(); - let mem_size = mgr.get_shared_mem_mut().mem_size(); - let layout = mgr.layout; - let shared_mem = mgr.get_shared_mem_mut(); - layout - .write(shared_mem, SandboxMemoryLayout::BASE_ADDRESS, mem_size) - .unwrap(); + let mgr = SandboxMemoryManager::from_snapshot(&snapshot).unwrap(); let (hmgr, _) = mgr.build().unwrap(); hmgr }; @@ -386,13 +378,7 @@ mod tests { let bin = GuestBinary::FilePath(simple_guest_as_string().unwrap()); let snapshot = crate::sandbox::snapshot::Snapshot::from_env(bin, sandbox_cfg).unwrap(); - let mut mgr = SandboxMemoryManager::from_snapshot(&snapshot).unwrap(); - let mem_size = mgr.get_shared_mem_mut().mem_size(); - let layout = mgr.layout; - let shared_mem = mgr.get_shared_mem_mut(); - layout - .write(shared_mem, SandboxMemoryLayout::BASE_ADDRESS, mem_size) - .unwrap(); + let mgr = SandboxMemoryManager::from_snapshot(&snapshot).unwrap(); let (hmgr, _) = mgr.build().unwrap(); hmgr }; diff --git a/src/hyperlight_host/src/sandbox/snapshot.rs b/src/hyperlight_host/src/sandbox/snapshot.rs index 816bfe792..c5f0520a6 100644 --- a/src/hyperlight_host/src/sandbox/snapshot.rs +++ b/src/hyperlight_host/src/sandbox/snapshot.rs @@ -26,8 +26,8 @@ use crate::hypervisor::regs::CommonSpecialRegisters; use crate::mem::exe::LoadInfo; use crate::mem::layout::SandboxMemoryLayout; use crate::mem::memory_region::MemoryRegion; -use crate::mem::mgr::GuestPageTableBuffer; -use crate::mem::shared_mem::{ExclusiveSharedMemory, SharedMemory}; +use crate::mem::mgr::{GuestPageTableBuffer, SnapshotSharedMemory}; +use crate::mem::shared_mem::{ReadonlySharedMemory, SharedMemory}; use crate::sandbox::SandboxConfiguration; use crate::sandbox::uninitialized::{GuestBinary, GuestEnvironment}; @@ -70,7 +70,7 @@ pub struct Snapshot { /// configuration id will share the same layout layout: crate::mem::layout::SandboxMemoryLayout, /// Memory of the sandbox at the time this snapshot was taken - memory: Vec, + memory: ReadonlySharedMemory, /// The memory regions that were mapped when this snapshot was /// taken (excluding initial sandbox regions) regions: Vec, @@ -175,40 +175,32 @@ fn hash(memory: &[u8], regions: &[MemoryRegion]) -> Result<[u8; 32]> { } pub(crate) fn access_gpa<'a>( - snap: &'a ExclusiveSharedMemory, - scratch: &'a ExclusiveSharedMemory, - scratch_size: usize, + snap: &'a [u8], + scratch: &'a [u8], + layout: SandboxMemoryLayout, gpa: u64, -) -> Option<(&'a ExclusiveSharedMemory, usize)> { - let scratch_base = scratch_base_gpa(scratch_size); - if gpa >= scratch_base && gpa < scratch_base + scratch_size as u64 { - Some((scratch, (gpa - scratch_base) as usize)) - } else if gpa >= SandboxMemoryLayout::BASE_ADDRESS as u64 - && gpa < SandboxMemoryLayout::BASE_ADDRESS as u64 + snap.mem_size() as u64 - { - Some((snap, gpa as usize - SandboxMemoryLayout::BASE_ADDRESS)) - } else { - None - } +) -> Option<(&'a [u8], usize)> { + let resolved = layout.resolve_gpa(gpa, &[])?.with_memories(snap, scratch); + Some((resolved.base.as_ref(), resolved.offset)) } pub(crate) struct SharedMemoryPageTableBuffer<'a> { - snap: &'a ExclusiveSharedMemory, - scratch: &'a ExclusiveSharedMemory, - scratch_size: usize, + snap: &'a [u8], + scratch: &'a [u8], + layout: SandboxMemoryLayout, root: u64, } impl<'a> SharedMemoryPageTableBuffer<'a> { pub(crate) fn new( - snap: &'a ExclusiveSharedMemory, - scratch: &'a ExclusiveSharedMemory, - scratch_size: usize, + snap: &'a [u8], + scratch: &'a [u8], + layout: SandboxMemoryLayout, root: u64, ) -> Self { Self { snap, scratch, - scratch_size, + layout, root, } } @@ -219,8 +211,8 @@ impl<'a> hyperlight_common::vmem::TableReadOps for SharedMemoryPageTableBuffer<' addr + offset } unsafe fn read_entry(&self, addr: u64) -> u64 { - let memoff = access_gpa(self.snap, self.scratch, self.scratch_size, addr); - let Some(pte_bytes) = memoff.and_then(|(mem, off)| mem.as_slice().get(off..off + 8)) else { + let memoff = access_gpa(self.snap, self.scratch, self.layout, addr); + let Some(pte_bytes) = memoff.and_then(|(mem, off)| mem.get(off..off + 8)) else { // Attacker-controlled data pointed out-of-bounds. We'll // default to returning 0 in this case, which, for most // architectures (including x86-64 and arm64, the ones we @@ -249,19 +241,19 @@ impl<'a> core::convert::AsRef> for SharedMemoryP } } fn filtered_mappings<'a>( - snap: &'a ExclusiveSharedMemory, - scratch: &'a ExclusiveSharedMemory, + snap: &'a [u8], + scratch: &'a [u8], regions: &[MemoryRegion], - scratch_size: usize, + layout: SandboxMemoryLayout, root_pt: u64, ) -> Vec<(Mapping, &'a [u8])> { - let op = SharedMemoryPageTableBuffer::new(snap, scratch, scratch_size, root_pt); + let op = SharedMemoryPageTableBuffer::new(snap, scratch, layout, root_pt); unsafe { hyperlight_common::vmem::virt_to_phys(&op, 0, hyperlight_common::layout::MAX_GVA as u64) } .filter_map(move |mapping| { // the scratch map doesn't count - if mapping.virt_base >= scratch_base_gva(scratch_size) { + if mapping.virt_base >= scratch_base_gva(layout.get_scratch_size()) { return None; } // neither does the mapping of the snapshot's own page tables @@ -272,8 +264,7 @@ fn filtered_mappings<'a>( return None; } // todo: is it useful to warn if we can't resolve this? - let contents = - unsafe { guest_page(snap, scratch, regions, scratch_size, mapping.phys_base) }?; + let contents = unsafe { guest_page(snap, scratch, regions, layout, mapping.phys_base) }?; Some((mapping, contents)) }) .collect() @@ -287,32 +278,19 @@ fn filtered_mappings<'a>( /// alive and must not be mutated by any other thread: references to /// these regions may be created and live for `'a`. unsafe fn guest_page<'a>( - snap: &'a ExclusiveSharedMemory, - scratch: &'a ExclusiveSharedMemory, + snap: &'a [u8], + scratch: &'a [u8], regions: &[MemoryRegion], - scratch_size: usize, + layout: SandboxMemoryLayout, gpa: u64, ) -> Option<&'a [u8]> { - if !gpa.is_multiple_of(PAGE_SIZE as u64) { + let resolved = layout + .resolve_gpa(gpa, regions)? + .with_memories(snap, scratch); + if resolved.as_ref().len() < PAGE_SIZE { return None; } - let gpa_u = gpa as usize; - for rgn in regions { - if gpa_u >= rgn.guest_region.start && gpa_u + PAGE_SIZE <= rgn.guest_region.end { - let off = gpa_u - rgn.guest_region.start; - #[allow(clippy::useless_conversion)] - let host_region_base: usize = rgn.host_region.start.into(); - return Some(unsafe { - std::slice::from_raw_parts((host_region_base + off) as *const u8, PAGE_SIZE) - }); - } - } - let (mem, off) = access_gpa(snap, scratch, scratch_size, gpa)?; - if off + PAGE_SIZE <= mem.as_slice().len() { - Some(&mem.as_slice()[off..off + PAGE_SIZE]) - } else { - None - } + Some(&resolved.as_ref()[..PAGE_SIZE]) } fn map_specials(pt_buf: &GuestPageTableBuffer, scratch_size: usize) { @@ -382,6 +360,8 @@ impl Snapshot { &mut memory[layout.get_guest_code_offset()..], )?; + layout.write_peb(&mut memory)?; + blob.map(|x| layout.write_init_data(&mut memory, x.data)) .transpose()?; @@ -435,7 +415,7 @@ impl Snapshot { Ok(Self { sandbox_id: SANDBOX_CONFIGURATION_COUNTER.fetch_add(1, Ordering::Relaxed), - memory, + memory: ReadonlySharedMemory::from_bytes(&memory)?, layout, regions: extra_regions, load_info, @@ -456,7 +436,7 @@ impl Snapshot { /// instance of `Self` with the snapshot stored therein. #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn new( - shared_mem: &mut S, + shared_mem: &mut SnapshotSharedMemory, scratch_mem: &mut S, sandbox_id: u64, mut layout: SandboxMemoryLayout, @@ -467,22 +447,19 @@ impl Snapshot { sregs: CommonSpecialRegisters, entrypoint: NextAction, ) -> Result { - let memory = shared_mem.with_exclusivity(|snap_e| { - scratch_mem.with_exclusivity(|scratch_e| { - let scratch_size = layout.get_scratch_size(); - + use std::collections::HashMap; + let mut phys_seen = HashMap::::new(); + let memory = shared_mem.with_contents(|snap_c| { + scratch_mem.with_contents(|scratch_c| { // Pass 1: count how many pages need to live let live_pages = - filtered_mappings(snap_e, scratch_e, ®ions, scratch_size, root_pt_gpa); + filtered_mappings(snap_c, scratch_c, ®ions, layout, root_pt_gpa); // Pass 2: copy them, and map them // TODO: Look for opportunities to hugepage map let pt_buf = GuestPageTableBuffer::new(layout.get_pt_base_gpa() as usize); let mut snapshot_memory: Vec = Vec::new(); for (mapping, contents) in live_pages { - let new_offset = snapshot_memory.len(); - snapshot_memory.extend(contents); - let new_gpa = new_offset + SandboxMemoryLayout::BASE_ADDRESS; let kind = match mapping.kind { MappingKind::Cow(cm) => MappingKind::Cow(cm), MappingKind::Basic(bm) if bm.writable => MappingKind::Cow(CowMapping { @@ -494,9 +471,15 @@ impl Snapshot { writable: false, executable: bm.executable, }), + MappingKind::Unmapped => continue, }; + let new_gpa = phys_seen.entry(mapping.phys_base).or_insert_with(|| { + let new_offset = snapshot_memory.len(); + snapshot_memory.extend(contents); + new_offset + SandboxMemoryLayout::BASE_ADDRESS + }); let mapping = Mapping { - phys_base: new_gpa as u64, + phys_base: *new_gpa as u64, virt_base: mapping.virt_base, len: PAGE_SIZE as u64, kind, @@ -511,6 +494,7 @@ impl Snapshot { Ok::, crate::HyperlightError>(snapshot_memory) }) })???; + layout.set_snapshot_size(memory.len()); // We do not need the original regions anymore, as any uses of // them in the guest have been incorporated into the snapshot @@ -521,7 +505,7 @@ impl Snapshot { Ok(Self { sandbox_id, layout, - memory, + memory: ReadonlySharedMemory::from_bytes(&memory)?, regions, load_info, hash, @@ -541,15 +525,9 @@ impl Snapshot { &self.regions } - /// Return the size of the snapshot in bytes. - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn mem_size(&self) -> usize { - self.memory.len() - } - /// Return the main memory contents of the snapshot #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn memory(&self) -> &[u8] { + pub(crate) fn memory(&self) -> &ReadonlySharedMemory { &self.memory } @@ -597,18 +575,19 @@ mod tests { use crate::hypervisor::regs::CommonSpecialRegisters; use crate::mem::exe::LoadInfo; use crate::mem::layout::SandboxMemoryLayout; - use crate::mem::mgr::{GuestPageTableBuffer, SandboxMemoryManager}; - use crate::mem::shared_mem::{ExclusiveSharedMemory, HostSharedMemory, SharedMemory}; + use crate::mem::mgr::{GuestPageTableBuffer, SandboxMemoryManager, SnapshotSharedMemory}; + use crate::mem::shared_mem::{ + ExclusiveSharedMemory, HostSharedMemory, ReadonlySharedMemory, SharedMemory, + }; fn default_sregs() -> CommonSpecialRegisters { CommonSpecialRegisters::default() } - fn make_simple_pt_mems() -> (SandboxMemoryManager, u64) { - let cfg = crate::sandbox::SandboxConfiguration::default(); - let scratch_mem = ExclusiveSharedMemory::new(cfg.get_scratch_size()).unwrap(); - let pt_base = PAGE_SIZE + SandboxMemoryLayout::BASE_ADDRESS; - let pt_buf = GuestPageTableBuffer::new(pt_base); + const SIMPLE_PT_BASE: usize = PAGE_SIZE + SandboxMemoryLayout::BASE_ADDRESS; + + fn make_simple_pt_mem(contents: &[u8]) -> 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, @@ -623,86 +602,36 @@ mod tests { super::map_specials(&pt_buf, PAGE_SIZE); let pt_bytes = pt_buf.into_bytes(); - let mut snapshot_mem = ExclusiveSharedMemory::new(PAGE_SIZE + pt_bytes.len()).unwrap(); + let mut snapshot_mem = vec![0u8; PAGE_SIZE + pt_bytes.len()]; + snapshot_mem[0..PAGE_SIZE].copy_from_slice(contents); + snapshot_mem[PAGE_SIZE..].copy_from_slice(&pt_bytes); + ReadonlySharedMemory::from_bytes(&snapshot_mem) + .unwrap() + .to_mgr_snapshot_mem() + .unwrap() + } - snapshot_mem.copy_from_slice(&pt_bytes, PAGE_SIZE).unwrap(); + fn make_simple_pt_mgr() -> (SandboxMemoryManager, u64) { + let cfg = crate::sandbox::SandboxConfiguration::default(); + let scratch_mem = ExclusiveSharedMemory::new(cfg.get_scratch_size()).unwrap(); let mgr = SandboxMemoryManager::new( SandboxMemoryLayout::new(cfg, 4096, 0x3000, None).unwrap(), - snapshot_mem, + make_simple_pt_mem(&[0u8; PAGE_SIZE]), scratch_mem, super::NextAction::None, ); let (mgr, _) = mgr.build().unwrap(); - (mgr, pt_base as u64) - } - - #[test] - fn restore() { - // Simplified version of the original test - let data1 = vec![b'a'; PAGE_SIZE]; - let data2 = vec![b'b'; PAGE_SIZE]; - - let (mut mgr, pt_base) = make_simple_pt_mems(); - mgr.shared_mem.copy_from_slice(&data1, 0).unwrap(); - - // Take snapshot of data1 - let snapshot = super::Snapshot::new( - &mut mgr.shared_mem, - &mut mgr.scratch_mem, - 0, - mgr.layout, - LoadInfo::dummy(), - Vec::new(), - pt_base, - 0, - default_sregs(), - super::NextAction::None, - ) - .unwrap(); - - // Modify memory to data2 - mgr.shared_mem.copy_from_slice(&data2, 0).unwrap(); - mgr.shared_mem - .with_exclusivity(|e| assert_eq!(&e.as_slice()[0..data2.len()], &data2[..])) - .unwrap(); - - // Restore should bring back data1 - let _ = mgr.restore_snapshot(&snapshot).unwrap(); - mgr.shared_mem - .with_exclusivity(|e| assert_eq!(&e.as_slice()[0..data1.len()], &data1[..])) - .unwrap(); - } - - #[test] - fn snapshot_mem_size() { - let (mut mgr, pt_base) = make_simple_pt_mems(); - let size = mgr.shared_mem.mem_size(); - - let snapshot = super::Snapshot::new( - &mut mgr.shared_mem, - &mut mgr.scratch_mem, - 0, - mgr.layout, - LoadInfo::dummy(), - Vec::new(), - pt_base, - 0, - default_sregs(), - super::NextAction::None, - ) - .unwrap(); - assert_eq!(snapshot.mem_size(), size); + (mgr, SIMPLE_PT_BASE as u64) } #[test] fn multiple_snapshots_independent() { - let (mut mgr, pt_base) = make_simple_pt_mems(); + let (mut mgr, pt_base) = make_simple_pt_mgr(); // Create first snapshot with pattern A let pattern_a = vec![0xAA; PAGE_SIZE]; - mgr.shared_mem.copy_from_slice(&pattern_a, 0).unwrap(); let snapshot_a = super::Snapshot::new( - &mut mgr.shared_mem, + &mut make_simple_pt_mem(&pattern_a).build().0, &mut mgr.scratch_mem, 1, mgr.layout, @@ -717,9 +646,8 @@ mod tests { // Create second snapshot with pattern B let pattern_b = vec![0xBB; PAGE_SIZE]; - mgr.shared_mem.copy_from_slice(&pattern_b, 0).unwrap(); let snapshot_b = super::Snapshot::new( - &mut mgr.shared_mem, + &mut make_simple_pt_mem(&pattern_b).build().0, &mut mgr.scratch_mem, 2, mgr.layout, @@ -732,19 +660,16 @@ mod tests { ) .unwrap(); - // Clear memory - mgr.shared_mem.copy_from_slice(&[0; PAGE_SIZE], 0).unwrap(); - // Restore snapshot A mgr.restore_snapshot(&snapshot_a).unwrap(); mgr.shared_mem - .with_exclusivity(|e| assert_eq!(&e.as_slice()[0..pattern_a.len()], &pattern_a[..])) + .with_contents(|contents| assert_eq!(&contents[0..pattern_a.len()], &pattern_a[..])) .unwrap(); // Restore snapshot B mgr.restore_snapshot(&snapshot_b).unwrap(); mgr.shared_mem - .with_exclusivity(|e| assert_eq!(&e.as_slice()[0..pattern_b.len()], &pattern_b[..])) + .with_contents(|contents| assert_eq!(&contents[0..pattern_b.len()], &pattern_b[..])) .unwrap(); } } diff --git a/src/hyperlight_host/src/sandbox/trace/mem_profile.rs b/src/hyperlight_host/src/sandbox/trace/mem_profile.rs index 54905b272..62cb281fa 100644 --- a/src/hyperlight_host/src/sandbox/trace/mem_profile.rs +++ b/src/hyperlight_host/src/sandbox/trace/mem_profile.rs @@ -20,6 +20,8 @@ use fallible_iterator::FallibleIterator; use framehop::Unwinder; use crate::hypervisor::regs::CommonRegisters; +#[cfg(not(unshared_snapshot_mem))] +use crate::mem::layout::ReadableSharedMemory; use crate::mem::layout::SandboxMemoryLayout; use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::HostSharedMemory; @@ -96,10 +98,15 @@ impl MemTraceInfo { mem_mgr: &SandboxMemoryManager, ) -> Result> { let mut read_stack = |addr| { + let mut buf: [u8; 8] = [0u8; 8]; mem_mgr .shared_mem - .read::((addr - SandboxMemoryLayout::BASE_ADDRESS as u64) as usize) - .map_err(|_| ()) + .copy_to_slice( + &mut buf, + (addr - SandboxMemoryLayout::BASE_ADDRESS as u64) as usize, + ) + .map_err(|_| ())?; + Ok(u64::from_ne_bytes(buf)) }; let mut cache = self .unwind_cache diff --git a/src/hyperlight_host/src/sandbox/uninitialized.rs b/src/hyperlight_host/src/sandbox/uninitialized.rs index 6cbe7921b..e737d08da 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized.rs @@ -362,11 +362,9 @@ impl UninitializedSandbox { } }; - let mut mem_mgr_wrapper = + let mem_mgr_wrapper = SandboxMemoryManager::::from_snapshot(snapshot.as_ref())?; - mem_mgr_wrapper.write_memory_layout()?; - let host_funcs = Arc::new(Mutex::new(FunctionRegistry::default())); let mut sandbox = Self {