From 20ad22f7f3b2afb7f86837707d382a61d042a6d9 Mon Sep 17 00:00:00 2001 From: danbugs Date: Tue, 6 May 2025 20:33:51 +0000 Subject: [PATCH 1/2] [common/peb,guest,host/{mem,sandbox}] removed the guest error region Guest errors are now transmitted via the input/output stacks. Signed-off-by: danbugs --- src/hyperlight_common/src/mem/mod.rs | 1 - src/hyperlight_guest/src/entrypoint.rs | 3 - src/hyperlight_guest/src/guest_error.rs | 56 ++------------- .../src/guest_function_call.rs | 4 +- src/hyperlight_host/src/func/guest_err.rs | 17 +++-- src/hyperlight_host/src/mem/layout.rs | 70 +------------------ src/hyperlight_host/src/mem/memory_region.rs | 2 - src/hyperlight_host/src/mem/mgr.rs | 22 ++---- src/hyperlight_host/src/sandbox/config.rs | 50 +------------ .../src/sandbox/uninitialized.rs | 1 - 10 files changed, 29 insertions(+), 197 deletions(-) diff --git a/src/hyperlight_common/src/mem/mod.rs b/src/hyperlight_common/src/mem/mod.rs index f24a757fe..3ae9a3688 100644 --- a/src/hyperlight_common/src/mem/mod.rs +++ b/src/hyperlight_common/src/mem/mod.rs @@ -84,7 +84,6 @@ pub struct GuestPanicContextData { pub struct HyperlightPEB { pub security_cookie_seed: u64, pub guest_function_dispatch_ptr: u64, - pub guestErrorData: GuestErrorData, pub pCode: *mut c_char, pub pOutb: *mut c_void, pub pOutbContext: *mut c_void, diff --git a/src/hyperlight_guest/src/entrypoint.rs b/src/hyperlight_guest/src/entrypoint.rs index 2a6f6c562..657f35543 100644 --- a/src/hyperlight_guest/src/entrypoint.rs +++ b/src/hyperlight_guest/src/entrypoint.rs @@ -23,7 +23,6 @@ use log::LevelFilter; use spin::Once; use crate::gdt::load_gdt; -use crate::guest_error::reset_error; use crate::guest_function_call::dispatch_function; use crate::guest_logger::init_logger; use crate::host_function_call::{outb, OutBAction}; @@ -146,8 +145,6 @@ pub extern "win64" fn entrypoint(peb_address: u64, seed: u64, ops: u64, max_log_ (*peb_ptr).guest_function_dispatch_ptr = dispatch_function as usize as u64; - reset_error(); - hyperlight_main(); } }); diff --git a/src/hyperlight_guest/src/guest_error.rs b/src/hyperlight_guest/src/guest_error.rs index c255789e3..bb4e32748 100644 --- a/src/hyperlight_guest/src/guest_error.rs +++ b/src/hyperlight_guest/src/guest_error.rs @@ -14,73 +14,27 @@ See the License for the specific language governing permissions and limitations under the License. */ -use alloc::string::{String, ToString}; +use alloc::string::ToString; use alloc::vec::Vec; use core::ffi::{c_char, CStr}; use hyperlight_common::flatbuffer_wrappers::guest_error::{ErrorCode, GuestError}; -use log::error; use crate::entrypoint::halt; use crate::host_function_call::{outb, OutBAction}; -use crate::P_PEB; +use crate::shared_output_data::push_shared_output_data; pub(crate) fn write_error(error_code: ErrorCode, message: Option<&str>) { let guest_error = GuestError::new( error_code.clone(), message.map_or("".to_string(), |m| m.to_string()), ); - let mut guest_error_buffer: Vec = (&guest_error) + let guest_error_buffer: Vec = (&guest_error) .try_into() .expect("Invalid guest_error_buffer, could not be converted to a Vec"); - unsafe { - assert!(!(*P_PEB.unwrap()).guestErrorData.guestErrorBuffer.is_null()); - let len = guest_error_buffer.len(); - if guest_error_buffer.len() > (*P_PEB.unwrap()).guestErrorData.guestErrorSize as usize { - error!( - "Guest error buffer is too small to hold the error message: size {} buffer size {} message may be truncated", - guest_error_buffer.len(), - (*P_PEB.unwrap()).guestErrorData.guestErrorSize as usize - ); - // get the length of the message - let message_len = message.map_or("".to_string(), |m| m.to_string()).len(); - // message is too long, truncate it - let truncate_len = message_len - - (guest_error_buffer.len() - - (*P_PEB.unwrap()).guestErrorData.guestErrorSize as usize); - let truncated_message = message - .map_or("".to_string(), |m| m.to_string()) - .chars() - .take(truncate_len) - .collect::(); - let guest_error = GuestError::new(error_code, truncated_message); - guest_error_buffer = (&guest_error) - .try_into() - .expect("Invalid guest_error_buffer, could not be converted to a Vec"); - } - - // Optimally, we'd use copy_from_slice here, but, because - // p_guest_error_buffer is a *mut c_void, we can't do that. - // Instead, we do the copying manually using pointer arithmetic. - // Plus; before, we'd do an assert w/ the result from copy_from_slice, - // but, because copy_nonoverlapping doesn't return anything, we can't do that. - // Instead, we do the prior asserts/checks to check the destination pointer isn't null - // and that there is enough space in the destination buffer for the copy. - let dest_ptr = (*P_PEB.unwrap()).guestErrorData.guestErrorBuffer as *mut u8; - core::ptr::copy_nonoverlapping(guest_error_buffer.as_ptr(), dest_ptr, len); - } -} - -pub(crate) fn reset_error() { - unsafe { - let peb_ptr = P_PEB.unwrap(); - core::ptr::write_bytes( - (*peb_ptr).guestErrorData.guestErrorBuffer, - 0, - (*peb_ptr).guestErrorData.guestErrorSize as usize, - ); - } + push_shared_output_data(guest_error_buffer) + .expect("Unable to push guest error to shared output data"); } pub(crate) fn set_error(error_code: ErrorCode, message: &str) { diff --git a/src/hyperlight_guest/src/guest_function_call.rs b/src/hyperlight_guest/src/guest_function_call.rs index 60d5548db..2c67cf2b5 100644 --- a/src/hyperlight_guest/src/guest_function_call.rs +++ b/src/hyperlight_guest/src/guest_function_call.rs @@ -23,7 +23,7 @@ use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode; use crate::entrypoint::halt; use crate::error::{HyperlightGuestError, Result}; -use crate::guest_error::{reset_error, set_error}; +use crate::guest_error::set_error; use crate::shared_input_data::try_pop_shared_input_data_into; use crate::shared_output_data::push_shared_output_data; use crate::REGISTERED_GUEST_FUNCTIONS; @@ -81,8 +81,6 @@ pub(crate) fn call_guest_function(function_call: FunctionCall) -> Result #[no_mangle] #[inline(never)] fn internal_dispatch_function() -> Result<()> { - reset_error(); - #[cfg(debug_assertions)] log::trace!("internal_dispatch_function"); diff --git a/src/hyperlight_host/src/func/guest_err.rs b/src/hyperlight_host/src/func/guest_err.rs index 627237934..a8fd87e5f 100644 --- a/src/hyperlight_host/src/func/guest_err.rs +++ b/src/hyperlight_host/src/func/guest_err.rs @@ -21,18 +21,27 @@ 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::{log_then_return, Result}; + /// 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: &MemMgrWrapper) -> Result<()> { - let guest_err = mgr.as_ref().get_guest_error()?; +pub(crate) fn check_for_guest_error(mgr: &mut MemMgrWrapper) -> Result<()> { + let guest_err = mgr.as_mut().get_guest_error().ok(); + let Some(guest_err) = guest_err else { + return Ok(()); + }; + + metrics::counter!( + METRIC_GUEST_ERROR, + METRIC_GUEST_ERROR_LABEL_CODE => (guest_err.code as u64).to_string() + ) + .increment(1); + match guest_err.code { ErrorCode::NoError => Ok(()), ErrorCode::StackOverflow => { - metrics::counter!(METRIC_GUEST_ERROR, METRIC_GUEST_ERROR_LABEL_CODE => (guest_err.code as u64).to_string()).increment(1); log_then_return!(StackOverflow()); } _ => { - metrics::counter!(METRIC_GUEST_ERROR, METRIC_GUEST_ERROR_LABEL_CODE => (guest_err.code as u64).to_string()).increment(1); log_then_return!(GuestError(guest_err.code, guest_err.message.clone())); } } diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index fb45e96e7..6b63ead08 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -22,8 +22,7 @@ use rand::{rng, RngCore}; use tracing::{instrument, Span}; use super::memory_region::MemoryRegionType::{ - Code, GuardPage, GuestErrorData, Heap, InputData, OutputData, PageTables, PanicContext, Peb, - Stack, + Code, GuardPage, Heap, InputData, OutputData, PageTables, PanicContext, Peb, Stack, }; use super::memory_region::{MemoryRegion, MemoryRegionFlags, MemoryRegionVecBuilder}; use super::mgr::AMOUNT_OF_MEMORY_PER_PT; @@ -45,8 +44,6 @@ use crate::{log_then_return, new_error, Result}; // +-------------------------------------------+ // | Input Data | // +-------------------------------------------+ -// | Guest Error Log | -// +-------------------------------------------+ // | PEB Struct | (HyperlightPEB size) // +-------------------------------------------+ // | Guest Code | @@ -60,9 +57,6 @@ use crate::{log_then_return, new_error, Result}; // | PML4 | // +-------------------------------------------+ 0x0_000 -/// - `GuestError` - contains a buffer for any guest error that occurred. -/// the length of this field is `GuestErrorBufferSize` from `SandboxConfiguration` -/// /// - `InputData` - this is a buffer that is used for input data to the host program. /// the length of this field is `InputDataSize` from `SandboxConfiguration` /// @@ -95,7 +89,6 @@ pub(crate) struct SandboxMemoryLayout { peb_offset: usize, peb_security_cookie_seed_offset: usize, peb_guest_dispatch_function_ptr_offset: usize, // set by guest in guest entrypoint - peb_guest_error_offset: usize, peb_code_and_outb_pointer_offset: usize, peb_runmode_offset: usize, peb_input_data_offset: usize, @@ -106,7 +99,6 @@ pub(crate) struct SandboxMemoryLayout { // The following are the actual values // that are written to the PEB struct - pub(super) guest_error_buffer_offset: usize, pub(super) input_data_buffer_offset: usize, pub(super) output_data_buffer_offset: usize, guest_panic_context_buffer_offset: usize, @@ -143,10 +135,6 @@ impl Debug for SandboxMemoryLayout { "Guest Dispatch Function Pointer Offset", &format_args!("{:#x}", self.peb_guest_dispatch_function_ptr_offset), ) - .field( - "Guest Error Offset", - &format_args!("{:#x}", self.peb_guest_error_offset), - ) .field( "Code and OutB Pointer Offset", &format_args!("{:#x}", self.peb_code_and_outb_pointer_offset), @@ -171,10 +159,6 @@ impl Debug for SandboxMemoryLayout { "Guest Stack Offset", &format_args!("{:#x}", self.peb_guest_stack_data_offset), ) - .field( - "Guest Error Buffer Offset", - &format_args!("{:#x}", self.guest_error_buffer_offset), - ) .field( "Input Data Buffer Offset", &format_args!("{:#x}", self.input_data_buffer_offset), @@ -259,7 +243,6 @@ impl SandboxMemoryLayout { peb_offset + offset_of!(HyperlightPEB, security_cookie_seed); let peb_guest_dispatch_function_ptr_offset = peb_offset + offset_of!(HyperlightPEB, guest_function_dispatch_ptr); - let peb_guest_error_offset = peb_offset + offset_of!(HyperlightPEB, guestErrorData); let peb_code_and_outb_pointer_offset = peb_offset + offset_of!(HyperlightPEB, pCode); let peb_runmode_offset = peb_offset + offset_of!(HyperlightPEB, runMode); let peb_input_data_offset = peb_offset + offset_of!(HyperlightPEB, inputdata); @@ -272,12 +255,8 @@ impl SandboxMemoryLayout { // The following offsets are the actual values that relate to memory layout, // which are written to PEB struct let peb_address = Self::BASE_ADDRESS + peb_offset; - let guest_error_buffer_offset = round_up_to( - peb_guest_stack_data_offset + size_of::(), - PAGE_SIZE_USIZE, - ); let input_data_buffer_offset = round_up_to( - guest_error_buffer_offset + cfg.get_guest_error_buffer_size(), + peb_guest_stack_data_offset + size_of::(), PAGE_SIZE_USIZE, ); let output_data_buffer_offset = round_up_to( @@ -305,7 +284,6 @@ impl SandboxMemoryLayout { heap_size, peb_security_cookie_seed_offset, peb_guest_dispatch_function_ptr_offset, - peb_guest_error_offset, peb_code_and_outb_pointer_offset, peb_runmode_offset, peb_input_data_offset, @@ -313,7 +291,6 @@ impl SandboxMemoryLayout { peb_guest_panic_context_offset, peb_heap_data_offset, peb_guest_stack_data_offset, - guest_error_buffer_offset, sandbox_memory_config: cfg, code_size, input_data_buffer_offset, @@ -333,18 +310,6 @@ impl SandboxMemoryLayout { self.peb_runmode_offset } - /// Get the offset in guest memory to the max size of the guest error buffer - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub(super) fn get_guest_error_buffer_size_offset(&self) -> usize { - self.peb_guest_error_offset - } - - /// Get the offset in guest memory to the error message buffer pointer - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - fn get_guest_error_buffer_pointer_offset(&self) -> usize { - self.peb_guest_error_offset + size_of::() - } - /// Get the offset in guest memory to the output data size #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub(super) fn get_output_data_size_offset(&self) -> usize { @@ -542,7 +507,6 @@ impl SandboxMemoryLayout { let mut total_mapped_memory_size: usize = round_up_to(code_size, PAGE_SIZE_USIZE); total_mapped_memory_size += round_up_to(stack_size, PAGE_SIZE_USIZE); total_mapped_memory_size += round_up_to(heap_size, PAGE_SIZE_USIZE); - total_mapped_memory_size += round_up_to(cfg.get_guest_error_buffer_size(), PAGE_SIZE_USIZE); total_mapped_memory_size += round_up_to(cfg.get_input_data_size(), PAGE_SIZE_USIZE); total_mapped_memory_size += round_up_to(cfg.get_output_data_size(), PAGE_SIZE_USIZE); total_mapped_memory_size += @@ -626,30 +590,12 @@ impl SandboxMemoryLayout { } // PEB - let guest_error_offset = builder.push_page_aligned( + let input_data_offset = builder.push_page_aligned( size_of::(), MemoryRegionFlags::READ | MemoryRegionFlags::WRITE, Peb, ); - let expected_guest_error_offset = - TryInto::::try_into(self.guest_error_buffer_offset)?; - - if guest_error_offset != expected_guest_error_offset { - return Err(new_error!( - "Guest error offset does not match expected Guest error offset expected: {}, actual: {}", - expected_guest_error_offset, - guest_error_offset - )); - } - - // guest error - let input_data_offset = builder.push_page_aligned( - self.sandbox_memory_config.get_guest_error_buffer_size(), - MemoryRegionFlags::READ | MemoryRegionFlags::WRITE, - GuestErrorData, - ); - let expected_input_data_offset = TryInto::::try_into(self.input_data_buffer_offset)?; if input_data_offset != expected_input_data_offset { @@ -818,14 +764,6 @@ impl SandboxMemoryLayout { // Skip guest_dispatch_function_ptr_offset because it is set by the guest - // Set up Guest Error Fields - let addr = get_address!(guest_error_buffer); - shared_mem.write_u64(self.get_guest_error_buffer_pointer_offset(), addr)?; - shared_mem.write_u64( - self.get_guest_error_buffer_size_offset(), - u64::try_from(self.sandbox_memory_config.get_guest_error_buffer_size())?, - )?; - // Skip code, is set when loading binary // skip outb and outb context, is set when running in_proc @@ -958,8 +896,6 @@ mod tests { expected_size += round_up_to(size_of::(), PAGE_SIZE_USIZE); - expected_size += round_up_to(cfg.get_guest_error_buffer_size(), PAGE_SIZE_USIZE); - expected_size += round_up_to(cfg.get_input_data_size(), PAGE_SIZE_USIZE); expected_size += round_up_to(cfg.get_output_data_size(), PAGE_SIZE_USIZE); diff --git a/src/hyperlight_host/src/mem/memory_region.rs b/src/hyperlight_host/src/mem/memory_region.rs index 638ed6655..89ff27a3f 100644 --- a/src/hyperlight_host/src/mem/memory_region.rs +++ b/src/hyperlight_host/src/mem/memory_region.rs @@ -131,8 +131,6 @@ pub enum MemoryRegionType { Code, /// The region contains the PEB Peb, - /// The region contains the Guest Error Data - GuestErrorData, /// The region contains the Input Data InputData, /// The region contains the Output Data diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index e172b2018..9ff566364 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -199,7 +199,6 @@ where MemoryRegionType::OutputData => PAGE_PRESENT | PAGE_RW | PAGE_NX, MemoryRegionType::Peb => PAGE_PRESENT | PAGE_RW | PAGE_NX, MemoryRegionType::PanicContext => PAGE_PRESENT | PAGE_RW | PAGE_NX, - MemoryRegionType::GuestErrorData => PAGE_PRESENT | PAGE_RW | PAGE_NX, MemoryRegionType::PageTables => PAGE_PRESENT | PAGE_RW | PAGE_NX, }, // If there is an error then the address isn't mapped so mark it as not present @@ -591,22 +590,11 @@ impl SandboxMemoryManager { /// Get the guest error data #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn get_guest_error(&self) -> Result { - // get memory buffer max size - let err_buffer_size_offset = self.layout.get_guest_error_buffer_size_offset(); - let max_err_buffer_size = self.shared_mem.read::(err_buffer_size_offset)?; - - // get guest error from layout and shared mem - let mut guest_error_buffer = vec![b'0'; usize::try_from(max_err_buffer_size)?]; - let err_msg_offset = self.layout.guest_error_buffer_offset; - self.shared_mem - .copy_to_slice(guest_error_buffer.as_mut_slice(), err_msg_offset)?; - GuestError::try_from(guest_error_buffer.as_slice()).map_err(|e| { - new_error!( - "get_guest_error: failed to convert buffer to GuestError: {}", - e - ) - }) + pub(crate) fn get_guest_error(&mut self) -> Result { + self.shared_mem.try_pop_buffer_into::( + self.layout.output_data_buffer_offset, + self.layout.sandbox_memory_config.get_output_data_size(), + ) } /// Read guest panic data from the `SharedMemory` contained within `self` diff --git a/src/hyperlight_host/src/sandbox/config.rs b/src/hyperlight_host/src/sandbox/config.rs index e0bbaddbe..56ec5f75d 100644 --- a/src/hyperlight_host/src/sandbox/config.rs +++ b/src/hyperlight_host/src/sandbox/config.rs @@ -1,3 +1,5 @@ +// TODO(danbugs:297): forgot to remove stuff related to host exceptions and +// host function definitions in my previous PR. Will remove them in the next commit. /* Copyright 2024 The Hyperlight Authors. @@ -36,8 +38,6 @@ pub struct SandboxConfiguration { /// Guest gdb debug port #[cfg(gdb)] guest_debug_info: Option, - /// The maximum size of the guest error buffer. - guest_error_buffer_size: usize, /// The size of the memory buffer that is made available for Guest Function /// Definitions host_function_definition_size: usize, @@ -117,10 +117,6 @@ impl SandboxConfiguration { pub const DEFAULT_HOST_EXCEPTION_SIZE: usize = 0x4000; /// The minimum size for host exceptions pub const MIN_HOST_EXCEPTION_SIZE: usize = 0x4000; - /// The default size for guest error messages - pub const DEFAULT_GUEST_ERROR_BUFFER_SIZE: usize = 0x100; - /// The minimum size for guest error messages - pub const MIN_GUEST_ERROR_BUFFER_SIZE: usize = 0x80; /// The default value for max initialization time (in milliseconds) pub const DEFAULT_MAX_INITIALIZATION_TIME: u16 = 2000; /// The minimum value for max initialization time (in milliseconds) @@ -156,7 +152,6 @@ impl SandboxConfiguration { output_data_size: usize, function_definition_size: usize, host_exception_size: usize, - guest_error_buffer_size: usize, stack_size_override: Option, heap_size_override: Option, kernel_stack_size: usize, @@ -174,10 +169,6 @@ impl SandboxConfiguration { Self::MIN_HOST_FUNCTION_DEFINITION_SIZE, ), host_exception_size: max(host_exception_size, Self::MIN_HOST_EXCEPTION_SIZE), - guest_error_buffer_size: max( - guest_error_buffer_size, - Self::MIN_GUEST_ERROR_BUFFER_SIZE, - ), stack_size_override: stack_size_override.unwrap_or(0), heap_size_override: heap_size_override.unwrap_or(0), kernel_stack_size: max(kernel_stack_size, Self::MIN_KERNEL_STACK_SIZE), @@ -268,14 +259,6 @@ impl SandboxConfiguration { self.host_exception_size = max(host_exception_size, Self::MIN_HOST_EXCEPTION_SIZE); } - /// Set the size of the memory buffer that is made available for serialising guest error messages - /// the minimum value is MIN_GUEST_ERROR_BUFFER_SIZE - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub fn set_guest_error_buffer_size(&mut self, guest_error_buffer_size: usize) { - self.guest_error_buffer_size = - max(guest_error_buffer_size, Self::MIN_GUEST_ERROR_BUFFER_SIZE); - } - /// Set the stack size to use in the guest sandbox. If set to 0, the stack size will be determined from the PE file header #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub fn set_stack_size(&mut self, stack_size: u64) { @@ -367,11 +350,6 @@ impl SandboxConfiguration { self.guest_debug_info = Some(debug_info); } - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn get_guest_error_buffer_size(&self) -> usize { - self.guest_error_buffer_size - } - #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn get_host_function_definition_size(&self) -> usize { self.host_function_definition_size @@ -457,7 +435,6 @@ impl Default for SandboxConfiguration { Self::DEFAULT_OUTPUT_SIZE, Self::DEFAULT_HOST_FUNCTION_DEFINITION_SIZE, Self::DEFAULT_HOST_EXCEPTION_SIZE, - Self::DEFAULT_GUEST_ERROR_BUFFER_SIZE, None, None, Self::DEFAULT_KERNEL_STACK_SIZE, @@ -486,7 +463,6 @@ mod tests { const OUTPUT_DATA_SIZE_OVERRIDE: usize = 0x4001; const HOST_FUNCTION_DEFINITION_SIZE_OVERRIDE: usize = 0x4002; const HOST_EXCEPTION_SIZE_OVERRIDE: usize = 0x4003; - const GUEST_ERROR_BUFFER_SIZE_OVERRIDE: usize = 0x40004; const MAX_EXECUTION_TIME_OVERRIDE: u16 = 1010; const MAX_WAIT_FOR_CANCELLATION_OVERRIDE: u8 = 200; const MAX_INITIALIZATION_TIME_OVERRIDE: u16 = 2000; @@ -497,7 +473,6 @@ mod tests { OUTPUT_DATA_SIZE_OVERRIDE, HOST_FUNCTION_DEFINITION_SIZE_OVERRIDE, HOST_EXCEPTION_SIZE_OVERRIDE, - GUEST_ERROR_BUFFER_SIZE_OVERRIDE, Some(STACK_SIZE_OVERRIDE), Some(HEAP_SIZE_OVERRIDE), KERNEL_STACK_SIZE_OVERRIDE, @@ -534,10 +509,6 @@ mod tests { cfg.host_function_definition_size ); assert_eq!(HOST_EXCEPTION_SIZE_OVERRIDE, cfg.host_exception_size); - assert_eq!( - GUEST_ERROR_BUFFER_SIZE_OVERRIDE, - cfg.guest_error_buffer_size - ); assert_eq!(MAX_EXECUTION_TIME_OVERRIDE, cfg.max_execution_time); assert_eq!( MAX_WAIT_FOR_CANCELLATION_OVERRIDE, @@ -560,7 +531,6 @@ mod tests { SandboxConfiguration::MIN_OUTPUT_SIZE - 1, SandboxConfiguration::MIN_HOST_FUNCTION_DEFINITION_SIZE - 1, SandboxConfiguration::MIN_HOST_EXCEPTION_SIZE - 1, - SandboxConfiguration::MIN_GUEST_ERROR_BUFFER_SIZE - 1, None, None, SandboxConfiguration::MIN_KERNEL_STACK_SIZE - 1, @@ -591,10 +561,6 @@ mod tests { SandboxConfiguration::MIN_HOST_EXCEPTION_SIZE, cfg.host_exception_size ); - assert_eq!( - SandboxConfiguration::MIN_GUEST_ERROR_BUFFER_SIZE, - cfg.guest_error_buffer_size - ); assert_eq!(0, cfg.stack_size_override); assert_eq!(0, cfg.heap_size_override); assert_eq!( @@ -620,7 +586,6 @@ mod tests { SandboxConfiguration::MIN_HOST_FUNCTION_DEFINITION_SIZE - 1, ); cfg.set_host_exception_size(SandboxConfiguration::MIN_HOST_EXCEPTION_SIZE - 1); - cfg.set_guest_error_buffer_size(SandboxConfiguration::MIN_GUEST_ERROR_BUFFER_SIZE - 1); cfg.set_max_execution_time(Duration::from_millis( SandboxConfiguration::MIN_MAX_EXECUTION_TIME as u64, )); @@ -644,10 +609,6 @@ mod tests { SandboxConfiguration::MIN_HOST_EXCEPTION_SIZE, cfg.host_exception_size ); - assert_eq!( - SandboxConfiguration::MIN_GUEST_ERROR_BUFFER_SIZE, - cfg.guest_error_buffer_size - ); assert_eq!( SandboxConfiguration::MIN_MAX_EXECUTION_TIME, cfg.max_execution_time @@ -670,13 +631,6 @@ mod tests { use crate::sandbox::config::DebugInfo; proptest! { - #[test] - fn error_buffer_size(size in SandboxConfiguration::MIN_GUEST_ERROR_BUFFER_SIZE..=SandboxConfiguration::MIN_GUEST_ERROR_BUFFER_SIZE * 10) { - let mut cfg = SandboxConfiguration::default(); - cfg.set_guest_error_buffer_size(size); - prop_assert_eq!(size, cfg.get_guest_error_buffer_size()); - } - #[test] fn host_function_definition_size(size in SandboxConfiguration::MIN_HOST_FUNCTION_DEFINITION_SIZE..=SandboxConfiguration::MIN_HOST_FUNCTION_DEFINITION_SIZE * 10) { let mut cfg = SandboxConfiguration::default(); diff --git a/src/hyperlight_host/src/sandbox/uninitialized.rs b/src/hyperlight_host/src/sandbox/uninitialized.rs index d61183b36..9f730c092 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized.rs @@ -436,7 +436,6 @@ mod tests { cfg.set_output_data_size(0x1000); cfg.set_host_function_definition_size(0x1000); cfg.set_host_exception_size(0x1000); - cfg.set_guest_error_buffer_size(0x1000); cfg.set_stack_size(0x1000); cfg.set_heap_size(0x1000); cfg.set_max_execution_time(Duration::from_millis(1001)); From 97b1876ed50a13d0468a2ef3efde5537c612ee2f Mon Sep 17 00:00:00 2001 From: danbugs Date: Tue, 6 May 2025 20:51:42 +0000 Subject: [PATCH 2/2] [host/sandbox/{config,uninit]] cleaned up legacy config options Removed cfg options that became unused after PRs #451 , #453 , and #457 merged. Signed-off-by: danbugs --- src/hyperlight_host/src/sandbox/config.rs | 130 ------------------ .../src/sandbox/uninitialized.rs | 2 - 2 files changed, 132 deletions(-) diff --git a/src/hyperlight_host/src/sandbox/config.rs b/src/hyperlight_host/src/sandbox/config.rs index 56ec5f75d..363c5e471 100644 --- a/src/hyperlight_host/src/sandbox/config.rs +++ b/src/hyperlight_host/src/sandbox/config.rs @@ -1,5 +1,3 @@ -// TODO(danbugs:297): forgot to remove stuff related to host exceptions and -// host function definitions in my previous PR. Will remove them in the next commit. /* Copyright 2024 The Hyperlight Authors. @@ -38,12 +36,6 @@ pub struct SandboxConfiguration { /// Guest gdb debug port #[cfg(gdb)] guest_debug_info: Option, - /// The size of the memory buffer that is made available for Guest Function - /// Definitions - host_function_definition_size: usize, - /// The size of the memory buffer that is made available for serialising - /// Host Exceptions - host_exception_size: usize, /// The size of the memory buffer that is made available for input to the /// Guest Binary input_data_size: usize, @@ -64,10 +56,6 @@ pub struct SandboxConfiguration { /// field should be represented as an `Option`, that type is not /// FFI-safe, so it cannot be. heap_size_override: u64, - /// The kernel_stack_size to use in the guest sandbox. If set to 0, the default kernel stack size will be used. - /// The value will be increased to a multiple page size when memory is allocated if necessary. - /// - kernel_stack_size: usize, /// The max_execution_time of a guest execution in milliseconds. If set to 0, the max_execution_time /// will be set to the default value of 1000ms if the guest execution does not complete within the time specified /// then the execution will be cancelled, the minimum value is 1ms @@ -107,16 +95,6 @@ impl SandboxConfiguration { pub const DEFAULT_OUTPUT_SIZE: usize = 0x4000; /// The minimum size of output data pub const MIN_OUTPUT_SIZE: usize = 0x2000; - /// The default size of host function definitions - /// Host function definitions has its own page in memory, in order to be READ-ONLY - /// from a guest's perspective. - pub const DEFAULT_HOST_FUNCTION_DEFINITION_SIZE: usize = 0x1000; - /// The minimum size of host function definitions - pub const MIN_HOST_FUNCTION_DEFINITION_SIZE: usize = 0x1000; - /// The default size for host exceptions - pub const DEFAULT_HOST_EXCEPTION_SIZE: usize = 0x4000; - /// The minimum size for host exceptions - pub const MIN_HOST_EXCEPTION_SIZE: usize = 0x4000; /// The default value for max initialization time (in milliseconds) pub const DEFAULT_MAX_INITIALIZATION_TIME: u16 = 2000; /// The minimum value for max initialization time (in milliseconds) @@ -139,10 +117,6 @@ impl SandboxConfiguration { pub const DEFAULT_GUEST_PANIC_CONTEXT_BUFFER_SIZE: usize = 0x400; /// The minimum value for guest panic context data pub const MIN_GUEST_PANIC_CONTEXT_BUFFER_SIZE: usize = 0x400; - /// The minimum value for kernel stack size - pub const MIN_KERNEL_STACK_SIZE: usize = 0x1000; - /// The default value for kernel stack size - pub const DEFAULT_KERNEL_STACK_SIZE: usize = Self::MIN_KERNEL_STACK_SIZE; #[allow(clippy::too_many_arguments)] /// Create a new configuration for a sandbox with the given sizes. @@ -150,11 +124,8 @@ impl SandboxConfiguration { fn new( input_data_size: usize, output_data_size: usize, - function_definition_size: usize, - host_exception_size: usize, stack_size_override: Option, heap_size_override: Option, - kernel_stack_size: usize, max_execution_time: Option, max_initialization_time: Option, max_wait_for_cancellation: Option, @@ -164,14 +135,8 @@ impl SandboxConfiguration { Self { input_data_size: max(input_data_size, Self::MIN_INPUT_SIZE), output_data_size: max(output_data_size, Self::MIN_OUTPUT_SIZE), - host_function_definition_size: max( - function_definition_size, - Self::MIN_HOST_FUNCTION_DEFINITION_SIZE, - ), - host_exception_size: max(host_exception_size, Self::MIN_HOST_EXCEPTION_SIZE), stack_size_override: stack_size_override.unwrap_or(0), heap_size_override: heap_size_override.unwrap_or(0), - kernel_stack_size: max(kernel_stack_size, Self::MIN_KERNEL_STACK_SIZE), max_execution_time: { match max_execution_time { Some(max_execution_time) => match max_execution_time.as_millis() { @@ -242,23 +207,6 @@ impl SandboxConfiguration { self.output_data_size = max(output_data_size, Self::MIN_OUTPUT_SIZE); } - /// Set the size of the memory buffer that is made available for serialising host function definitions - /// the minimum value is MIN_HOST_FUNCTION_DEFINITION_SIZE - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub fn set_host_function_definition_size(&mut self, host_function_definition_size: usize) { - self.host_function_definition_size = max( - host_function_definition_size, - Self::MIN_HOST_FUNCTION_DEFINITION_SIZE, - ); - } - - /// Set the size of the memory buffer that is made available for serialising host exceptions - /// the minimum value is MIN_HOST_EXCEPTION_SIZE - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub fn set_host_exception_size(&mut self, host_exception_size: usize) { - self.host_exception_size = max(host_exception_size, Self::MIN_HOST_EXCEPTION_SIZE); - } - /// Set the stack size to use in the guest sandbox. If set to 0, the stack size will be determined from the PE file header #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub fn set_stack_size(&mut self, stack_size: u64) { @@ -271,13 +219,6 @@ impl SandboxConfiguration { self.heap_size_override = heap_size; } - /// Set the kernel stack size to use in the guest sandbox. If less than the minimum value of MIN_KERNEL_STACK_SIZE, the minimum value will be used. - /// If its not a multiple of the page size, it will be increased to the a multiple of the page size when memory is allocated. - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub fn set_kernel_stack_size(&mut self, kernel_stack_size: usize) { - self.kernel_stack_size = max(kernel_stack_size, Self::MIN_KERNEL_STACK_SIZE); - } - /// Set the maximum execution time of a guest function execution. If set to 0, the max_execution_time /// will be set to the default value of DEFAULT_MAX_EXECUTION_TIME if the guest execution does not complete within the time specified /// then the execution will be cancelled, the minimum value is MIN_MAX_EXECUTION_TIME @@ -350,16 +291,6 @@ impl SandboxConfiguration { self.guest_debug_info = Some(debug_info); } - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn get_host_function_definition_size(&self) -> usize { - self.host_function_definition_size - } - - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn get_host_exception_size(&self) -> usize { - self.host_exception_size - } - #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn get_input_data_size(&self) -> usize { self.input_data_size @@ -413,11 +344,6 @@ impl SandboxConfiguration { .unwrap_or_else(|| exe_info.stack_reserve()) } - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub(crate) fn get_kernel_stack_size(&self) -> usize { - self.kernel_stack_size - } - /// If self.heap_size_override is non-zero, return it. Otherwise, /// return exe_info.heap_reserve() #[instrument(skip_all, parent = Span::current(), level= "Trace")] @@ -433,11 +359,8 @@ impl Default for SandboxConfiguration { Self::new( Self::DEFAULT_INPUT_SIZE, Self::DEFAULT_OUTPUT_SIZE, - Self::DEFAULT_HOST_FUNCTION_DEFINITION_SIZE, - Self::DEFAULT_HOST_EXCEPTION_SIZE, None, None, - Self::DEFAULT_KERNEL_STACK_SIZE, None, None, None, @@ -461,21 +384,15 @@ mod tests { const HEAP_SIZE_OVERRIDE: u64 = 0x50000; const INPUT_DATA_SIZE_OVERRIDE: usize = 0x4000; const OUTPUT_DATA_SIZE_OVERRIDE: usize = 0x4001; - const HOST_FUNCTION_DEFINITION_SIZE_OVERRIDE: usize = 0x4002; - const HOST_EXCEPTION_SIZE_OVERRIDE: usize = 0x4003; const MAX_EXECUTION_TIME_OVERRIDE: u16 = 1010; const MAX_WAIT_FOR_CANCELLATION_OVERRIDE: u8 = 200; const MAX_INITIALIZATION_TIME_OVERRIDE: u16 = 2000; const GUEST_PANIC_CONTEXT_BUFFER_SIZE_OVERRIDE: usize = 0x4005; - const KERNEL_STACK_SIZE_OVERRIDE: usize = 0x4000; let mut cfg = SandboxConfiguration::new( INPUT_DATA_SIZE_OVERRIDE, OUTPUT_DATA_SIZE_OVERRIDE, - HOST_FUNCTION_DEFINITION_SIZE_OVERRIDE, - HOST_EXCEPTION_SIZE_OVERRIDE, Some(STACK_SIZE_OVERRIDE), Some(HEAP_SIZE_OVERRIDE), - KERNEL_STACK_SIZE_OVERRIDE, Some(Duration::from_millis(MAX_EXECUTION_TIME_OVERRIDE as u64)), Some(Duration::from_millis( MAX_INITIALIZATION_TIME_OVERRIDE as u64, @@ -501,14 +418,8 @@ mod tests { cfg.heap_size_override = 2048; assert_eq!(1024, cfg.stack_size_override); assert_eq!(2048, cfg.heap_size_override); - assert_eq!(16384, cfg.kernel_stack_size); assert_eq!(INPUT_DATA_SIZE_OVERRIDE, cfg.input_data_size); assert_eq!(OUTPUT_DATA_SIZE_OVERRIDE, cfg.output_data_size); - assert_eq!( - HOST_FUNCTION_DEFINITION_SIZE_OVERRIDE, - cfg.host_function_definition_size - ); - assert_eq!(HOST_EXCEPTION_SIZE_OVERRIDE, cfg.host_exception_size); assert_eq!(MAX_EXECUTION_TIME_OVERRIDE, cfg.max_execution_time); assert_eq!( MAX_WAIT_FOR_CANCELLATION_OVERRIDE, @@ -529,11 +440,8 @@ mod tests { let mut cfg = SandboxConfiguration::new( SandboxConfiguration::MIN_INPUT_SIZE - 1, SandboxConfiguration::MIN_OUTPUT_SIZE - 1, - SandboxConfiguration::MIN_HOST_FUNCTION_DEFINITION_SIZE - 1, - SandboxConfiguration::MIN_HOST_EXCEPTION_SIZE - 1, None, None, - SandboxConfiguration::MIN_KERNEL_STACK_SIZE - 1, Some(Duration::from_millis( SandboxConfiguration::MIN_MAX_EXECUTION_TIME as u64, )), @@ -549,18 +457,6 @@ mod tests { ); assert_eq!(SandboxConfiguration::MIN_INPUT_SIZE, cfg.input_data_size); assert_eq!(SandboxConfiguration::MIN_OUTPUT_SIZE, cfg.output_data_size); - assert_eq!( - SandboxConfiguration::MIN_KERNEL_STACK_SIZE, - cfg.kernel_stack_size - ); - assert_eq!( - SandboxConfiguration::MIN_HOST_FUNCTION_DEFINITION_SIZE, - cfg.host_function_definition_size - ); - assert_eq!( - SandboxConfiguration::MIN_HOST_EXCEPTION_SIZE, - cfg.host_exception_size - ); assert_eq!(0, cfg.stack_size_override); assert_eq!(0, cfg.heap_size_override); assert_eq!( @@ -582,10 +478,6 @@ mod tests { cfg.set_input_data_size(SandboxConfiguration::MIN_INPUT_SIZE - 1); cfg.set_output_data_size(SandboxConfiguration::MIN_OUTPUT_SIZE - 1); - cfg.set_host_function_definition_size( - SandboxConfiguration::MIN_HOST_FUNCTION_DEFINITION_SIZE - 1, - ); - cfg.set_host_exception_size(SandboxConfiguration::MIN_HOST_EXCEPTION_SIZE - 1); cfg.set_max_execution_time(Duration::from_millis( SandboxConfiguration::MIN_MAX_EXECUTION_TIME as u64, )); @@ -601,14 +493,6 @@ mod tests { assert_eq!(SandboxConfiguration::MIN_INPUT_SIZE, cfg.input_data_size); assert_eq!(SandboxConfiguration::MIN_OUTPUT_SIZE, cfg.output_data_size); - assert_eq!( - SandboxConfiguration::MIN_HOST_FUNCTION_DEFINITION_SIZE, - cfg.host_function_definition_size - ); - assert_eq!( - SandboxConfiguration::MIN_HOST_EXCEPTION_SIZE, - cfg.host_exception_size - ); assert_eq!( SandboxConfiguration::MIN_MAX_EXECUTION_TIME, cfg.max_execution_time @@ -631,20 +515,6 @@ mod tests { use crate::sandbox::config::DebugInfo; proptest! { - #[test] - fn host_function_definition_size(size in SandboxConfiguration::MIN_HOST_FUNCTION_DEFINITION_SIZE..=SandboxConfiguration::MIN_HOST_FUNCTION_DEFINITION_SIZE * 10) { - let mut cfg = SandboxConfiguration::default(); - cfg.set_host_function_definition_size(size); - prop_assert_eq!(size, cfg.get_host_function_definition_size()); - } - - #[test] - fn host_exception_size(size in SandboxConfiguration::MIN_HOST_EXCEPTION_SIZE..=SandboxConfiguration::MIN_HOST_EXCEPTION_SIZE * 10) { - let mut cfg = SandboxConfiguration::default(); - cfg.set_host_exception_size(size); - prop_assert_eq!(size, cfg.get_host_exception_size()); - } - #[test] fn input_data_size(size in SandboxConfiguration::MIN_INPUT_SIZE..=SandboxConfiguration::MIN_INPUT_SIZE * 10) { let mut cfg = SandboxConfiguration::default(); diff --git a/src/hyperlight_host/src/sandbox/uninitialized.rs b/src/hyperlight_host/src/sandbox/uninitialized.rs index 9f730c092..33013c981 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized.rs @@ -434,8 +434,6 @@ mod tests { let mut cfg = SandboxConfiguration::default(); cfg.set_input_data_size(0x1000); cfg.set_output_data_size(0x1000); - cfg.set_host_function_definition_size(0x1000); - cfg.set_host_exception_size(0x1000); cfg.set_stack_size(0x1000); cfg.set_heap_size(0x1000); cfg.set_max_execution_time(Duration::from_millis(1001));