diff --git a/src/hyperlight_host/src/func/guest_err.rs b/src/hyperlight_host/src/func/guest_err.rs index af3a0bbe3..27db6d231 100644 --- a/src/hyperlight_host/src/func/guest_err.rs +++ b/src/hyperlight_host/src/func/guest_err.rs @@ -17,15 +17,17 @@ limitations under the License. use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode; use crate::error::HyperlightError::{GuestError, StackOverflow}; +use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::HostSharedMemory; use crate::metrics::{METRIC_GUEST_ERROR, METRIC_GUEST_ERROR_LABEL_CODE}; -use crate::sandbox::mem_mgr::MemMgrWrapper; use crate::{Result, log_then_return}; /// Check for a guest error and return an `Err` if one was found, /// and `Ok` if one was not found. -pub(crate) fn check_for_guest_error(mgr: &mut MemMgrWrapper) -> Result<()> { - let guest_err = mgr.as_mut().get_guest_error().ok(); +pub(crate) fn check_for_guest_error( + mgr: &mut SandboxMemoryManager, +) -> Result<()> { + let guest_err = mgr.get_guest_error().ok(); let Some(guest_err) = guest_err else { return Ok(()); }; diff --git a/src/hyperlight_host/src/func/host_functions.rs b/src/hyperlight_host/src/func/host_functions.rs index 3944b0703..871d04c97 100644 --- a/src/hyperlight_host/src/func/host_functions.rs +++ b/src/hyperlight_host/src/func/host_functions.rs @@ -61,7 +61,7 @@ impl Registerable for UninitializedSandbox { return_type: Output::TYPE, }; - (*hfs).register_host_function(name.to_string(), entry, self.mgr.unwrap_mgr_mut()) + (*hfs).register_host_function(name.to_string(), entry, &mut self.mgr) } #[cfg(all(feature = "seccomp", target_os = "linux"))] fn register_host_function_with_syscalls( @@ -82,7 +82,7 @@ impl Registerable for UninitializedSandbox { return_type: Output::TYPE, }; - (*hfs).register_host_function(name.to_string(), entry, self.mgr.unwrap_mgr_mut()) + (*hfs).register_host_function(name.to_string(), entry, &mut self.mgr) } } @@ -210,7 +210,7 @@ pub(crate) fn register_host_function>>, + dbg_mem_access_fn: Arc>>, ) -> crate::Result<()> { let addr = self.translate_gva(vcpu_fd, addr)?; @@ -235,7 +235,7 @@ pub(super) trait GuestDebug { vcpu_fd: &Self::Vcpu, mut gva: u64, mut data: &mut [u8], - dbg_mem_access_fn: Arc>>, + dbg_mem_access_fn: Arc>>, ) -> crate::Result<()> { let data_len = data.len(); log::debug!("Read addr: {:X} len: {:X}", gva, data_len); @@ -259,7 +259,6 @@ pub(super) trait GuestDebug { dbg_mem_access_fn .try_lock() .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .unwrap_mgr_mut() .get_shared_mem_mut() .copy_to_slice(&mut data[..read_len], offset)?; @@ -285,7 +284,7 @@ pub(super) trait GuestDebug { &mut self, vcpu_fd: &Self::Vcpu, addr: u64, - dbg_mem_access_fn: Arc>>, + dbg_mem_access_fn: Arc>>, ) -> crate::Result<()> { let addr = self.translate_gva(vcpu_fd, addr)?; @@ -309,7 +308,7 @@ pub(super) trait GuestDebug { vcpu_fd: &Self::Vcpu, mut gva: u64, mut data: &[u8], - dbg_mem_access_fn: Arc>>, + dbg_mem_access_fn: Arc>>, ) -> crate::Result<()> { let data_len = data.len(); log::debug!("Write addr: {:X} len: {:X}", gva, data_len); @@ -333,7 +332,6 @@ pub(super) trait GuestDebug { dbg_mem_access_fn .try_lock() .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .unwrap_mgr_mut() .get_shared_mem_mut() .copy_from_slice(&data[..write_len], offset)?; diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index 1907d8e0b..02be9bed0 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -75,13 +75,13 @@ use super::{HyperlightExit, Hypervisor, InterruptHandle, LinuxInterruptHandle, V use crate::HyperlightError; use crate::hypervisor::get_memory_access_violation; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; +use crate::mem::mgr::SandboxMemoryManager; use crate::mem::ptr::{GuestPtr, RawPtr}; use crate::mem::shared_mem::HostSharedMemory; use crate::sandbox::SandboxConfiguration; #[cfg(feature = "trace_guest")] use crate::sandbox::TraceInfo; use crate::sandbox::host_funcs::FunctionRegistry; -use crate::sandbox::mem_mgr::MemMgrWrapper; use crate::sandbox::outb::handle_outb; #[cfg(crashdump)] use crate::sandbox::uninitialized::SandboxRuntimeConfig; @@ -94,8 +94,8 @@ mod debug { use super::mshv_bindings::hv_x64_exception_intercept_message; use super::{HypervLinuxDriver, *}; use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs}; + use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::HostSharedMemory; - use crate::sandbox::mem_mgr::MemMgrWrapper; use crate::{Result, new_error}; impl HypervLinuxDriver { @@ -126,7 +126,7 @@ mod debug { pub(crate) fn process_dbg_request( &mut self, req: DebugMsg, - dbg_mem_access_fn: Arc>>, + dbg_mem_access_fn: Arc>>, ) -> Result { if let Some(debug) = self.debug.as_mut() { match req { @@ -174,7 +174,6 @@ mod debug { .map_err(|e| { new_error!("Error locking at {}:{}: {}", file!(), line!(), e) })? - .unwrap_mgr() .layout .get_guest_code_address(); @@ -312,7 +311,7 @@ pub(crate) struct HypervLinuxDriver { orig_rsp: GuestPtr, entrypoint: u64, interrupt_handle: Arc, - mem_mgr: Option>, + mem_mgr: Option>, host_funcs: Option>>, sandbox_regions: Vec, // Initially mapped regions when sandbox is created @@ -584,10 +583,10 @@ impl Hypervisor for HypervLinuxDriver { peb_addr: RawPtr, seed: u64, page_size: u32, - mem_mgr: MemMgrWrapper, + mem_mgr: SandboxMemoryManager, host_funcs: Arc>, max_guest_log_level: Option, - #[cfg(gdb)] dbg_mem_access_fn: Arc>>, + #[cfg(gdb)] dbg_mem_access_fn: Arc>>, ) -> Result<()> { self.mem_mgr = Some(mem_mgr); self.host_funcs = Some(host_funcs); @@ -659,7 +658,7 @@ impl Hypervisor for HypervLinuxDriver { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - #[cfg(gdb)] dbg_mem_access_fn: Arc>>, + #[cfg(gdb)] dbg_mem_access_fn: Arc>>, ) -> Result<()> { // Reset general purpose registers, then set RIP and RSP let regs = StandardRegisters { @@ -705,7 +704,7 @@ impl Hypervisor for HypervLinuxDriver { #[cfg(feature = "trace_guest")] { // We need to handle the borrow checker issue where we need both: - // - &mut MemMgrWrapper (from self.mem_mgr.as_mut()) + // - &mut SandboxMemoryManager (from self.mem_mgr) // - &mut dyn Hypervisor (from self) // We'll use a temporary approach to extract the mem_mgr temporarily let mem_mgr_option = self.mem_mgr.take(); @@ -1021,7 +1020,7 @@ impl Hypervisor for HypervLinuxDriver { #[cfg(gdb)] fn handle_debug( &mut self, - dbg_mem_access_fn: Arc>>, + dbg_mem_access_fn: Arc>>, stop_reason: VcpuStopReason, ) -> Result<()> { if self.debug.is_none() { diff --git a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs index f8b4727d1..0f98613b3 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs @@ -58,12 +58,12 @@ use crate::hypervisor::fpu::FP_CONTROL_WORD_DEFAULT; use crate::hypervisor::get_memory_access_violation; use crate::hypervisor::wrappers::WHvGeneralRegisters; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; +use crate::mem::mgr::SandboxMemoryManager; use crate::mem::ptr::{GuestPtr, RawPtr}; use crate::mem::shared_mem::HostSharedMemory; #[cfg(feature = "trace_guest")] use crate::sandbox::TraceInfo; use crate::sandbox::host_funcs::FunctionRegistry; -use crate::sandbox::mem_mgr::MemMgrWrapper; use crate::sandbox::outb::handle_outb; #[cfg(crashdump)] use crate::sandbox::uninitialized::SandboxRuntimeConfig; @@ -78,8 +78,8 @@ mod debug { use super::{HypervWindowsDriver, *}; use crate::Result; use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs}; + use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::HostSharedMemory; - use crate::sandbox::mem_mgr::MemMgrWrapper; impl HypervWindowsDriver { /// Resets the debug information to disable debugging @@ -109,7 +109,7 @@ mod debug { pub(crate) fn process_dbg_request( &mut self, req: DebugMsg, - dbg_mem_access_fn: Arc>>, + dbg_mem_access_fn: Arc>>, ) -> Result { if let Some(debug) = self.debug.as_mut() { match req { @@ -157,7 +157,6 @@ mod debug { .map_err(|e| { new_error!("Error locking at {}:{}: {}", file!(), line!(), e) })? - .unwrap_mgr() .layout .get_guest_code_address(); @@ -279,7 +278,7 @@ pub(crate) struct HypervWindowsDriver { entrypoint: u64, orig_rsp: GuestPtr, interrupt_handle: Arc, - mem_mgr: Option>, + mem_mgr: Option>, host_funcs: Option>>, sandbox_regions: Vec, // Initially mapped regions when sandbox is created @@ -599,10 +598,10 @@ impl Hypervisor for HypervWindowsDriver { peb_address: RawPtr, seed: u64, page_size: u32, - mem_mgr: MemMgrWrapper, + mem_mgr: SandboxMemoryManager, host_funcs: Arc>, max_guest_log_level: Option, - #[cfg(gdb)] dbg_mem_access_hdl: Arc>>, + #[cfg(gdb)] dbg_mem_access_hdl: Arc>>, ) -> Result<()> { self.mem_mgr = Some(mem_mgr); self.host_funcs = Some(host_funcs); @@ -652,7 +651,7 @@ impl Hypervisor for HypervWindowsDriver { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - #[cfg(gdb)] dbg_mem_access_hdl: Arc>>, + #[cfg(gdb)] dbg_mem_access_hdl: Arc>>, ) -> Result<()> { // Reset general purpose registers, then set RIP and RSP let regs = WHvGeneralRegisters { @@ -696,7 +695,7 @@ impl Hypervisor for HypervWindowsDriver { #[cfg(feature = "trace_guest")] { // We need to handle the borrow checker issue where we need both: - // - &mut MemMgrWrapper (from self.mem_mgr.as_mut()) + // - &mut SandboxMemoryManager (from self.mem_mgr.as_mut()) // - &mut dyn Hypervisor (from self) // We'll use a temporary approach to extract the mem_mgr temporarily let mem_mgr_option = self.mem_mgr.take(); @@ -955,7 +954,7 @@ impl Hypervisor for HypervWindowsDriver { #[cfg(gdb)] fn handle_debug( &mut self, - dbg_mem_access_fn: Arc>>, + dbg_mem_access_fn: Arc>>, stop_reason: super::gdb::VcpuStopReason, ) -> Result<()> { if self.debug.is_none() { diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index be76129fb..d048e20f3 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -42,13 +42,13 @@ use super::{HyperlightExit, Hypervisor, InterruptHandle, LinuxInterruptHandle, V use crate::HyperlightError; use crate::hypervisor::get_memory_access_violation; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; +use crate::mem::mgr::SandboxMemoryManager; use crate::mem::ptr::{GuestPtr, RawPtr}; use crate::mem::shared_mem::HostSharedMemory; use crate::sandbox::SandboxConfiguration; #[cfg(feature = "trace_guest")] use crate::sandbox::TraceInfo; use crate::sandbox::host_funcs::FunctionRegistry; -use crate::sandbox::mem_mgr::MemMgrWrapper; use crate::sandbox::outb::handle_outb; #[cfg(crashdump)] use crate::sandbox::uninitialized::SandboxRuntimeConfig; @@ -86,8 +86,8 @@ mod debug { use crate::hypervisor::gdb::{ DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason, X86_64Regs, }; + use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::HostSharedMemory; - use crate::sandbox::mem_mgr::MemMgrWrapper; use crate::{Result, new_error}; impl KVMDriver { @@ -118,7 +118,7 @@ mod debug { pub(crate) fn process_dbg_request( &mut self, req: DebugMsg, - dbg_mem_access_fn: Arc>>, + dbg_mem_access_fn: Arc>>, ) -> Result { if let Some(debug) = self.debug.as_mut() { match req { @@ -166,7 +166,6 @@ mod debug { .map_err(|e| { new_error!("Error locking at {}:{}: {}", file!(), line!(), e) })? - .unwrap_mgr() .layout .get_guest_code_address(); @@ -292,7 +291,7 @@ pub(crate) struct KVMDriver { entrypoint: u64, orig_rsp: GuestPtr, interrupt_handle: Arc, - mem_mgr: Option>, + mem_mgr: Option>, host_funcs: Option>>, sandbox_regions: Vec, // Initially mapped regions when sandbox is created @@ -472,10 +471,10 @@ impl Hypervisor for KVMDriver { peb_addr: RawPtr, seed: u64, page_size: u32, - mem_mgr: MemMgrWrapper, + mem_mgr: SandboxMemoryManager, host_funcs: Arc>, max_guest_log_level: Option, - #[cfg(gdb)] dbg_mem_access_fn: Arc>>, + #[cfg(gdb)] dbg_mem_access_fn: Arc>>, ) -> Result<()> { self.mem_mgr = Some(mem_mgr); self.host_funcs = Some(host_funcs); @@ -570,7 +569,7 @@ impl Hypervisor for KVMDriver { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - #[cfg(gdb)] dbg_mem_access_fn: Arc>>, + #[cfg(gdb)] dbg_mem_access_fn: Arc>>, ) -> Result<()> { // Reset general purpose registers, then set RIP and RSP let regs = kvm_regs { @@ -623,7 +622,7 @@ impl Hypervisor for KVMDriver { #[cfg(feature = "trace_guest")] { // We need to handle the borrow checker issue where we need both: - // - &mut MemMgrWrapper (from self.mem_mgr.as_mut()) + // - &mut SandboxMemoryManager (from self.mem_mgr.as_mut()) // - &mut dyn Hypervisor (from self) // We'll use a temporary approach to extract the mem_mgr temporarily let mem_mgr_option = self.mem_mgr.take(); @@ -898,7 +897,7 @@ impl Hypervisor for KVMDriver { #[cfg(gdb)] fn handle_debug( &mut self, - dbg_mem_access_fn: Arc>>, + dbg_mem_access_fn: Arc>>, stop_reason: VcpuStopReason, ) -> Result<()> { if self.debug.is_none() { diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index 4e55e7ac5..928d9ac2e 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -70,10 +70,10 @@ use std::time::Duration; #[cfg(gdb)] use gdb::VcpuStopReason; +use crate::mem::mgr::SandboxMemoryManager; use crate::mem::ptr::RawPtr; use crate::mem::shared_mem::HostSharedMemory; use crate::sandbox::host_funcs::FunctionRegistry; -use crate::sandbox::mem_mgr::MemMgrWrapper; cfg_if::cfg_if! { if #[cfg(feature = "init-paging")] { @@ -141,10 +141,10 @@ pub(crate) trait Hypervisor: Debug + Send { peb_addr: RawPtr, seed: u64, page_size: u32, - mem_mgr: MemMgrWrapper, + mem_mgr: SandboxMemoryManager, host_funcs: Arc>, guest_max_log_level: Option, - #[cfg(gdb)] dbg_mem_access_fn: Arc>>, + #[cfg(gdb)] dbg_mem_access_fn: Arc>>, ) -> Result<()>; /// Map a region of host memory into the sandbox. @@ -171,7 +171,7 @@ pub(crate) trait Hypervisor: Debug + Send { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - #[cfg(gdb)] dbg_mem_access_fn: Arc>>, + #[cfg(gdb)] dbg_mem_access_fn: Arc>>, ) -> Result<()>; /// Handle an IO exit from the internally stored vCPU. @@ -236,7 +236,7 @@ pub(crate) trait Hypervisor: Debug + Send { /// handles the cases when the vCPU stops due to a Debug event fn handle_debug( &mut self, - _dbg_mem_access_fn: Arc>>, + _dbg_mem_access_fn: Arc>>, _stop_reason: VcpuStopReason, ) -> Result<()> { unimplemented!() @@ -289,7 +289,7 @@ impl VirtualCPU { #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] pub(crate) fn run( hv: &mut dyn Hypervisor, - #[cfg(gdb)] dbg_mem_access_fn: Arc>>, + #[cfg(gdb)] dbg_mem_access_fn: Arc>>, ) -> Result<()> { loop { match hv.run() { diff --git a/src/hyperlight_host/src/lib.rs b/src/hyperlight_host/src/lib.rs index e1cc2e5a2..34c7ddd2a 100644 --- a/src/hyperlight_host/src/lib.rs +++ b/src/hyperlight_host/src/lib.rs @@ -87,8 +87,6 @@ pub(crate) mod testing; /// The re-export for the `HyperlightError` type pub use error::HyperlightError; -/// Re-export for `HypervisorWrapper` trait -/// Re-export for `MemMgrWrapper` type /// A sandbox that can call be used to make multiple calls to guest functions, /// and otherwise reused multiple times pub use sandbox::MultiUseSandbox; diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index 787f095e2..f578f3ea8 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -74,6 +74,10 @@ pub(crate) struct SandboxMemoryManager { pub(crate) entrypoint_offset: Offset, /// How many memory regions were mapped after sandbox creation pub(crate) mapped_rgns: u64, + /// Stack cookie for stack guard verification + pub(crate) stack_cookie: [u8; STACK_COOKIE_LEN], + /// Buffer for accumulating guest abort messages + pub(crate) abort_buffer: Vec, } impl SandboxMemoryManager @@ -82,11 +86,12 @@ where { /// Create a new `SandboxMemoryManager` with the given parameters #[instrument(skip_all, parent = Span::current(), level= "Trace")] - fn new( + pub(crate) fn new( layout: SandboxMemoryLayout, shared_mem: S, load_addr: RawPtr, entrypoint_offset: Offset, + stack_cookie: [u8; STACK_COOKIE_LEN], ) -> Self { Self { layout, @@ -94,10 +99,24 @@ where load_addr, entrypoint_offset, mapped_rgns: 0, + stack_cookie, + abort_buffer: Vec::new(), } } + /// Get the stack cookie + #[instrument(skip_all, parent = Span::current(), level= "Trace")] + pub(crate) fn get_stack_cookie(&self) -> &[u8; STACK_COOKIE_LEN] { + &self.stack_cookie + } + + /// Get mutable access to the abort buffer + pub(crate) fn get_abort_buffer_mut(&mut self) -> &mut Vec { + &mut self.abort_buffer + } + /// Get `SharedMemory` in `self` as a mutable reference + #[cfg(any(gdb, test))] pub(crate) fn get_shared_mem_mut(&mut self) -> &mut S { &mut self.shared_mem } @@ -334,8 +353,18 @@ impl SandboxMemoryManager { &mut shared_mem.as_mut_slice()[layout.get_guest_code_offset()..], )?; + let stack_cookie = rand::random::<[u8; STACK_COOKIE_LEN]>(); + let stack_offset = layout.get_top_of_user_stack_offset(); + shared_mem.copy_from_slice(&stack_cookie, stack_offset)?; + Ok(( - Self::new(layout, shared_mem, load_addr, entrypoint_offset), + Self::new( + layout, + shared_mem, + load_addr, + entrypoint_offset, + stack_cookie, + ), load_info, )) } @@ -376,12 +405,23 @@ impl SandboxMemoryManager { Ok(()) } - /// Set the stack guard to `cookie` using `layout` to calculate - /// its location and `shared_mem` to write it. + /// Write memory layout #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn set_stack_guard(&mut self, cookie: &[u8; STACK_COOKIE_LEN]) -> Result<()> { - let stack_offset = self.layout.get_top_of_user_stack_offset(); - self.shared_mem.copy_from_slice(cookie, stack_offset) + 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, + ) + } + + /// Write init data + #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] + pub(crate) fn write_init_data(&mut self, user_memory: &[u8]) -> Result<()> { + self.layout + .write_init_data(&mut self.shared_mem, user_memory)?; + Ok(()) } /// Wraps ExclusiveSharedMemory::build @@ -398,14 +438,18 @@ impl SandboxMemoryManager { layout: self.layout, load_addr: self.load_addr.clone(), entrypoint_offset: self.entrypoint_offset, - mapped_rgns: 0, + mapped_rgns: self.mapped_rgns, + stack_cookie: self.stack_cookie, + abort_buffer: self.abort_buffer, }, SandboxMemoryManager { shared_mem: gshm, layout: self.layout, load_addr: self.load_addr.clone(), entrypoint_offset: self.entrypoint_offset, - mapped_rgns: 0, + mapped_rgns: self.mapped_rgns, + stack_cookie: self.stack_cookie, + abort_buffer: Vec::new(), // Guest doesn't need abort buffer }, ) } @@ -425,10 +469,11 @@ impl SandboxMemoryManager { /// documentation at the bottom `set_stack_guard` for description /// of why it isn't. #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn check_stack_guard(&self, cookie: [u8; STACK_COOKIE_LEN]) -> Result { + pub(crate) fn check_stack_guard(&self) -> Result { + let expected = self.stack_cookie; let offset = self.layout.get_top_of_user_stack_offset(); - let test_cookie: [u8; STACK_COOKIE_LEN] = self.shared_mem.read(offset)?; - let cmp_res = cookie.iter().cmp(test_cookie.iter()); + let actual: [u8; STACK_COOKIE_LEN] = self.shared_mem.read(offset)?; + let cmp_res = expected.iter().cmp(actual.iter()); Ok(cmp_res == Ordering::Equal) } diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index b1b41fff2..9223c5e1b 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -31,9 +31,9 @@ use hyperlight_common::flatbuffer_wrappers::function_types::{ use hyperlight_common::flatbuffer_wrappers::util::estimate_flatbuffer_capacity; use tracing::{Span, instrument}; +use super::Callable; use super::host_funcs::FunctionRegistry; use super::snapshot::Snapshot; -use super::{Callable, WrapperGetter}; use crate::HyperlightError::SnapshotSandboxMismatch; use crate::func::guest_err::check_for_guest_error; use crate::func::{ParameterTuple, SupportedReturnType}; @@ -41,10 +41,10 @@ use crate::hypervisor::{Hypervisor, InterruptHandle}; #[cfg(unix)] use crate::mem::memory_region::MemoryRegionType; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; +use crate::mem::mgr::SandboxMemoryManager; use crate::mem::ptr::RawPtr; use crate::mem::shared_mem::HostSharedMemory; use crate::metrics::maybe_time_and_emit_guest_call; -use crate::sandbox::mem_mgr::MemMgrWrapper; use crate::{Result, log_then_return}; /// Global counter for assigning unique IDs to sandboxes @@ -59,11 +59,11 @@ pub struct MultiUseSandbox { id: u64, // We need to keep a reference to the host functions, even if the compiler marks it as unused. The compiler cannot detect our dynamic usages of the host function in `HyperlightFunction::call`. pub(super) _host_funcs: Arc>, - pub(crate) mem_mgr: MemMgrWrapper, + pub(crate) mem_mgr: SandboxMemoryManager, vm: Box, dispatch_ptr: RawPtr, #[cfg(gdb)] - dbg_mem_access_fn: Arc>>, + dbg_mem_access_fn: Arc>>, /// If the current state of the sandbox has been captured in a snapshot, /// that snapshot is stored here. snapshot: Option, @@ -78,10 +78,10 @@ impl MultiUseSandbox { #[instrument(skip_all, parent = Span::current(), level = "Trace")] pub(super) fn from_uninit( host_funcs: Arc>, - mgr: MemMgrWrapper, + mgr: SandboxMemoryManager, vm: Box, dispatch_ptr: RawPtr, - #[cfg(gdb)] dbg_mem_access_fn: Arc>>, + #[cfg(gdb)] dbg_mem_access_fn: Arc>>, ) -> MultiUseSandbox { Self { id: SANDBOX_ID_COUNTER.fetch_add(1, Ordering::Relaxed), @@ -125,10 +125,7 @@ impl MultiUseSandbox { } let mapped_regions_iter = self.vm.get_mapped_regions(); let mapped_regions_vec: Vec = mapped_regions_iter.cloned().collect(); - let memory_snapshot = self - .mem_mgr - .unwrap_mgr_mut() - .snapshot(self.id, mapped_regions_vec)?; + let memory_snapshot = self.mem_mgr.snapshot(self.id, mapped_regions_vec)?; let inner = Arc::new(memory_snapshot); let snapshot = Snapshot { inner }; self.snapshot = Some(snapshot.clone()); @@ -179,9 +176,7 @@ impl MultiUseSandbox { return Err(SnapshotSandboxMismatch); } - self.mem_mgr - .unwrap_mgr_mut() - .restore_snapshot(&snapshot.inner)?; + self.mem_mgr.restore_snapshot(&snapshot.inner)?; let current_regions: HashSet<_> = self.vm.get_mapped_regions().cloned().collect(); let snapshot_regions: HashSet<_> = snapshot.inner.regions().iter().cloned().collect(); @@ -326,7 +321,7 @@ impl MultiUseSandbox { // Reset snapshot since we are mutating the sandbox state self.snapshot = None; unsafe { self.vm.map_region(rgn) }?; - self.mem_mgr.unwrap_mgr_mut().mapped_rgns += 1; + self.mem_mgr.mapped_rgns += 1; Ok(()) } @@ -406,9 +401,7 @@ impl MultiUseSandbox { let mut builder = FlatBufferBuilder::with_capacity(estimated_capacity); let buffer = fc.encode(&mut builder); - self.get_mgr_wrapper_mut() - .as_mut() - .write_guest_function_call(buffer)?; + self.mem_mgr.write_guest_function_call(buffer)?; self.vm.dispatch_call_from_host( self.dispatch_ptr.clone(), @@ -417,11 +410,9 @@ impl MultiUseSandbox { )?; self.mem_mgr.check_stack_guard()?; - check_for_guest_error(self.get_mgr_wrapper_mut())?; + check_for_guest_error(&mut self.mem_mgr)?; - self.get_mgr_wrapper_mut() - .as_mut() - .get_guest_function_call_result() + self.mem_mgr.get_guest_function_call_result() })(); // In the happy path we do not need to clear io-buffers from the host because: @@ -430,7 +421,7 @@ impl MultiUseSandbox { // - any serialized host function call are zeroed out by us (the host) during deserialization, see `get_host_function_call` // - any serialized host function result is zeroed out by the guest during deserialization, see `get_host_return_value` if res.is_err() { - self.get_mgr_wrapper_mut().as_mut().clear_io_buffers(); + self.mem_mgr.clear_io_buffers(); } res } @@ -479,15 +470,6 @@ impl Callable for MultiUseSandbox { } } -impl WrapperGetter for MultiUseSandbox { - fn get_mgr_wrapper(&self) -> &MemMgrWrapper { - &self.mem_mgr - } - fn get_mgr_wrapper_mut(&mut self) -> &mut MemMgrWrapper { - &mut self.mem_mgr - } -} - impl std::fmt::Debug for MultiUseSandbox { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("MultiUseSandbox") diff --git a/src/hyperlight_host/src/sandbox/mem_mgr.rs b/src/hyperlight_host/src/sandbox/mem_mgr.rs deleted file mode 100644 index eb25af9da..000000000 --- a/src/hyperlight_host/src/sandbox/mem_mgr.rs +++ /dev/null @@ -1,123 +0,0 @@ -/* -Copyright 2025 The Hyperlight Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -use tracing::{Span, instrument}; - -use crate::Result; -use crate::mem::layout::SandboxMemoryLayout; -use crate::mem::mgr::{STACK_COOKIE_LEN, SandboxMemoryManager}; -use crate::mem::shared_mem::{ - ExclusiveSharedMemory, GuestSharedMemory, HostSharedMemory, SharedMemory, -}; - -/// StackCookie -pub type StackCookie = [u8; STACK_COOKIE_LEN]; - -/// A container with methods for accessing `SandboxMemoryManager` and other -/// related objects -#[derive(Clone)] -pub(crate) struct MemMgrWrapper { - mgr: SandboxMemoryManager, - stack_cookie: StackCookie, - abort_buffer: Vec, -} - -impl MemMgrWrapper { - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub(super) fn new(mgr: SandboxMemoryManager, stack_cookie: StackCookie) -> Self { - Self { - mgr, - stack_cookie, - abort_buffer: Vec::new(), - } - } - - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn unwrap_mgr(&self) -> &SandboxMemoryManager { - &self.mgr - } - - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn unwrap_mgr_mut(&mut self) -> &mut SandboxMemoryManager { - &mut self.mgr - } - - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub(super) fn get_stack_cookie(&self) -> &StackCookie { - &self.stack_cookie - } - - pub fn get_abort_buffer_mut(&mut self) -> &mut Vec { - &mut self.abort_buffer - } -} - -impl AsMut> for MemMgrWrapper { - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - fn as_mut(&mut self) -> &mut SandboxMemoryManager { - self.unwrap_mgr_mut() - } -} - -impl AsRef> for MemMgrWrapper { - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - fn as_ref(&self) -> &SandboxMemoryManager { - self.unwrap_mgr() - } -} - -impl MemMgrWrapper { - pub(crate) fn build( - self, - ) -> ( - MemMgrWrapper, - SandboxMemoryManager, - ) { - let (hshm, gshm) = self.mgr.build(); - (MemMgrWrapper::new(hshm, self.stack_cookie), gshm) - } - - #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - pub(super) fn write_memory_layout(&mut self) -> Result<()> { - let mgr = self.unwrap_mgr_mut(); - let layout = mgr.layout; - let shared_mem = mgr.get_shared_mem_mut(); - let mem_size = shared_mem.mem_size(); - layout.write(shared_mem, SandboxMemoryLayout::BASE_ADDRESS, mem_size) - } - - #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - pub(super) fn write_init_data(&mut self, user_memory: &[u8]) -> Result<()> { - let mgr = self.unwrap_mgr_mut(); - let layout = mgr.layout; - let shared_mem = mgr.get_shared_mem_mut(); - layout.write_init_data(shared_mem, user_memory)?; - Ok(()) - } -} - -impl MemMgrWrapper { - /// Check the stack guard against the given `stack_cookie`. - /// - /// Return `Ok(true)` if the given cookie matches the one in guest memory, - /// and `Ok(false)` otherwise. Return `Err` if it could not be found or - /// there was some other error. - #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn check_stack_guard(&self) -> Result { - self.unwrap_mgr() - .check_stack_guard(*self.get_stack_cookie()) - } -} diff --git a/src/hyperlight_host/src/sandbox/mod.rs b/src/hyperlight_host/src/sandbox/mod.rs index b3f54e738..2f8bee004 100644 --- a/src/hyperlight_host/src/sandbox/mod.rs +++ b/src/hyperlight_host/src/sandbox/mod.rs @@ -23,9 +23,6 @@ pub(crate) mod hypervisor; /// Functionality for dealing with initialized sandboxes that can /// call 0 or more guest functions pub mod initialized_multi_use; -/// Functionality for interacting with a sandbox's internally-stored -/// `SandboxMemoryManager` -pub(crate) mod mem_mgr; pub(crate) mod outb; /// Functionality for creating uninitialized sandboxes, manipulating them, /// and converting them to initialized sandboxes. @@ -59,7 +56,6 @@ pub use uninitialized::GuestBinary; /// Re-export for `UninitializedSandbox` type pub use uninitialized::UninitializedSandbox; -use self::mem_mgr::MemMgrWrapper; #[cfg(target_os = "windows")] use crate::hypervisor::windows_hypervisor_platform; use crate::mem::shared_mem::HostSharedMemory; @@ -227,12 +223,6 @@ impl TraceInfo { } } -pub(crate) trait WrapperGetter { - #[allow(dead_code)] - fn get_mgr_wrapper(&self) -> &MemMgrWrapper; - fn get_mgr_wrapper_mut(&mut self) -> &mut MemMgrWrapper; -} - #[cfg(test)] mod tests { use std::sync::Arc; diff --git a/src/hyperlight_host/src/sandbox/outb.rs b/src/hyperlight_host/src/sandbox/outb.rs index 20a5d8f94..8f9c1166f 100644 --- a/src/hyperlight_host/src/sandbox/outb.rs +++ b/src/hyperlight_host/src/sandbox/outb.rs @@ -33,7 +33,6 @@ use tracing::{Span, instrument}; use tracing_log::format_trace; use super::host_funcs::FunctionRegistry; -use super::mem_mgr::MemMgrWrapper; #[cfg(feature = "trace_guest")] use crate::hypervisor::Hypervisor; #[cfg(feature = "trace_guest")] @@ -109,7 +108,7 @@ pub(super) fn outb_log(mgr: &mut SandboxMemoryManager) -> Resu const ABORT_TERMINATOR: u8 = 0xFF; const MAX_ABORT_BUFFER_LEN: usize = 1024; -fn outb_abort(mem_mgr: &mut MemMgrWrapper, data: u32) -> Result<()> { +fn outb_abort(mem_mgr: &mut SandboxMemoryManager, data: u32) -> Result<()> { let buffer = mem_mgr.get_abort_buffer_mut(); let bytes = data.to_le_bytes(); // [len, b1, b2, b3] @@ -269,25 +268,23 @@ pub(super) fn record_guest_trace_frame( /// Handles OutB operations from the guest. #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn handle_outb( - mem_mgr: &mut MemMgrWrapper, + mem_mgr: &mut SandboxMemoryManager, host_funcs: Arc>, #[cfg(feature = "trace_guest")] _hv: &mut dyn Hypervisor, port: u16, data: u32, ) -> Result<()> { match port.try_into()? { - OutBAction::Log => outb_log(mem_mgr.as_mut()), + OutBAction::Log => outb_log(mem_mgr), OutBAction::CallFunction => { - let call = mem_mgr.as_mut().get_host_function_call()?; // pop output buffer + let call = mem_mgr.get_host_function_call()?; // pop output buffer let name = call.function_name.clone(); let args: Vec = call.parameters.unwrap_or(vec![]); let res = host_funcs .try_lock() .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? .call_host_function(&name, args)?; - mem_mgr - .as_mut() - .write_response_from_host_method_call(&res)?; // push input buffers + mem_mgr.write_response_from_host_method_call(&res)?; // push input buffers Ok(()) } @@ -305,7 +302,7 @@ pub(crate) fn handle_outb( } #[cfg(feature = "unwind_guest")] OutBAction::TraceRecordStack => { - let Ok(stack) = unwind(_hv, mem_mgr.as_ref(), _hv.trace_info_as_ref()) else { + let Ok(stack) = unwind(_hv, mem_mgr, _hv.trace_info_as_ref()) else { return Ok(()); }; record_trace_frame(_hv.trace_info_as_ref(), 1u64, |f| { @@ -314,7 +311,7 @@ pub(crate) fn handle_outb( } #[cfg(feature = "mem_profile")] OutBAction::TraceMemoryAlloc => { - let Ok(stack) = unwind(_hv, mem_mgr.as_ref(), _hv.trace_info_as_ref()) else { + let Ok(stack) = unwind(_hv, mem_mgr, _hv.trace_info_as_ref()) else { return Ok(()); }; let Ok(amt) = _hv.read_trace_reg(crate::hypervisor::TraceRegister::RAX) else { @@ -331,7 +328,7 @@ pub(crate) fn handle_outb( } #[cfg(feature = "mem_profile")] OutBAction::TraceMemoryFree => { - let Ok(stack) = unwind(_hv, mem_mgr.as_ref(), _hv.trace_info_as_ref()) else { + let Ok(stack) = unwind(_hv, mem_mgr, _hv.trace_info_as_ref()) else { return Ok(()); }; let Ok(ptr) = _hv.read_trace_reg(crate::hypervisor::TraceRegister::RCX) else { @@ -355,7 +352,6 @@ pub(crate) fn handle_outb( // Read the trace records from the guest memory mem_mgr - .as_ref() .shared_mem .copy_to_slice(buffer, ptr as usize - SandboxMemoryLayout::BASE_ADDRESS) .map_err(|e| { diff --git a/src/hyperlight_host/src/sandbox/uninitialized.rs b/src/hyperlight_host/src/sandbox/uninitialized.rs index 6d905cf52..679d5e0e5 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized.rs @@ -23,7 +23,6 @@ use log::LevelFilter; use tracing::{Span, instrument}; use super::host_funcs::{FunctionRegistry, default_writer_func}; -use super::mem_mgr::MemMgrWrapper; use super::uninitialized_evolve::evolve_impl_multi_use; use crate::func::host_functions::{HostFunction, register_host_function}; use crate::func::{ParameterTuple, SupportedReturnType}; @@ -31,7 +30,7 @@ use crate::func::{ParameterTuple, SupportedReturnType}; use crate::log_build_details; use crate::mem::exe::ExeInfo; use crate::mem::memory_region::{DEFAULT_GUEST_BLOB_MEM_FLAGS, MemoryRegionFlags}; -use crate::mem::mgr::{STACK_COOKIE_LEN, SandboxMemoryManager}; +use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::ExclusiveSharedMemory; use crate::sandbox::SandboxConfiguration; use crate::{MultiUseSandbox, Result, new_error}; @@ -77,7 +76,7 @@ pub struct UninitializedSandbox { /// Registered host functions pub(crate) host_funcs: Arc>, /// The memory manager for the sandbox. - pub(crate) mgr: MemMgrWrapper, + pub(crate) mgr: SandboxMemoryManager, pub(crate) max_guest_log_level: Option, pub(crate) config: SandboxConfiguration, #[cfg(any(crashdump, gdb))] @@ -88,7 +87,7 @@ pub struct UninitializedSandbox { impl Debug for UninitializedSandbox { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("UninitializedSandbox") - .field("memory_layout", &self.mgr.unwrap_mgr().layout) + .field("memory_layout", &self.mgr.layout) .finish() } } @@ -233,17 +232,11 @@ impl UninitializedSandbox { } }; - let (mut mem_mgr_wrapper, load_info) = { - let (mut mgr, load_info) = UninitializedSandbox::load_guest_binary( - sandbox_cfg, - &guest_binary, - guest_blob.as_ref(), - )?; - - let stack_guard = Self::create_stack_guard(); - mgr.set_stack_guard(&stack_guard)?; - (MemMgrWrapper::new(mgr, stack_guard), load_info) - }; + let (mut mem_mgr_wrapper, load_info) = UninitializedSandbox::load_guest_binary( + sandbox_cfg, + &guest_binary, + guest_blob.as_ref(), + )?; mem_mgr_wrapper.write_memory_layout()?; @@ -272,11 +265,6 @@ impl UninitializedSandbox { Ok(sandbox) } - #[instrument(skip_all, parent = Span::current(), level = "Trace")] - fn create_stack_guard() -> [u8; STACK_COOKIE_LEN] { - rand::random::<[u8; STACK_COOKIE_LEN]>() - } - /// Load the file at `bin_path_str` into a PE file, then attempt to /// load the PE file into a `SandboxMemoryManager` and return it. /// diff --git a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs index e3334fb3e..f73e334e8 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs @@ -33,12 +33,12 @@ use crate::mem::ptr_offset::Offset; use crate::mem::shared_mem::GuestSharedMemory; #[cfg(any(feature = "init-paging", target_os = "windows"))] use crate::mem::shared_mem::SharedMemory; +use crate::sandbox::HostSharedMemory; #[cfg(feature = "trace_guest")] use crate::sandbox::TraceInfo; #[cfg(gdb)] use crate::sandbox::config::DebugInfo; use crate::sandbox::host_funcs::FunctionRegistry; -use crate::sandbox::{HostSharedMemory, MemMgrWrapper}; #[cfg(target_os = "linux")] use crate::signal_handlers::setup_signal_handlers; use crate::{MultiUseSandbox, Result, UninitializedSandbox, log_then_return, new_error}; @@ -62,7 +62,7 @@ fn evolve_impl( where TransformFunc: Fn( Arc>, - MemMgrWrapper, + SandboxMemoryManager, Box, RawPtr, ) -> Result, @@ -104,7 +104,7 @@ where dbg_mem_access_hdl, )?; - let dispatch_function_addr = hshm.as_ref().get_pointer_to_dispatch_function()?; + let dispatch_function_addr = hshm.get_pointer_to_dispatch_function()?; if dispatch_function_addr == 0 { return Err(new_error!("Dispatch function address is null")); }