From c30a467c0ada9e2d90b528ba46f6019815125bb4 Mon Sep 17 00:00:00 2001 From: danbugs Date: Fri, 23 May 2025 17:03:51 +0000 Subject: [PATCH 1/5] [common/mem] remove legacy fields from GuestStackData We no longer have kernel stack and boot stacks, so we can remove these fields. Also, removed some legacy comment. Signed-off-by: danbugs --- src/hyperlight_common/src/mem.rs | 4 ---- src/hyperlight_host/src/lib.rs | 21 +++------------------ 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/src/hyperlight_common/src/mem.rs b/src/hyperlight_common/src/mem.rs index bd94fb692..d9af6dc09 100644 --- a/src/hyperlight_common/src/mem.rs +++ b/src/hyperlight_common/src/mem.rs @@ -46,10 +46,6 @@ pub struct GuestStackData { pub minUserStackAddress: u64, /// This is the user stack pointer pub userStackAddress: u64, - /// This is the stack pointer for the kernel mode stack - pub kernelStackAddress: u64, - /// This is the initial stack pointer when init is called its used before the TSS is set up - pub bootStackAddress: u64, } #[repr(C)] diff --git a/src/hyperlight_host/src/lib.rs b/src/hyperlight_host/src/lib.rs index c18027d63..6a974f23d 100644 --- a/src/hyperlight_host/src/lib.rs +++ b/src/hyperlight_host/src/lib.rs @@ -41,22 +41,7 @@ pub mod hypervisor; /// Functionality to establish and manage an individual sandbox's /// memory. /// -/// The following structs are not used other than to calculate the size of the memory needed -/// and also to illustrate the layout of the memory: -/// -/// - `HostFunctionDefinitions` -/// - `HostExceptionData` -/// - `GuestError` -/// - `CodeAndOutBPointers` -/// - `InputData` -/// - `OutputData` -/// - `GuestHeap` -/// - `GuestStack` -/// -/// the start of the guest memory contains the page tables and is always located at the Virtual Address 0x00200000 when -/// running in a Hypervisor: -/// -/// Virtual Address +/// - Virtual Address /// /// 0x0000 PML4 /// 0x1000 PDPT @@ -64,8 +49,8 @@ pub mod hypervisor; /// 0x3000 The guest PE code (When the code has been loaded using LoadLibrary to debug the guest this will not be /// present and code length will be zero; /// -/// The pointer passed to the Entrypoint in the Guest application is the ize of page table + size of code, -/// at this address structs below are laid out in this order +/// - The pointer passed to the Entrypoint in the Guest application is the size of page table + size of code, +/// at this address structs below are laid out in this order pub mod mem; /// Metric definitions and helpers pub mod metrics; From 307de19da58687bffac36fb8152086a42ebad723 Mon Sep 17 00:00:00 2001 From: danbugs Date: Fri, 23 May 2025 17:07:09 +0000 Subject: [PATCH 2/5] [common/mem] use u64s for PEB pointers If the guest and host are compiled for different architectures, there will be a mismatch in usage of the PEB. Using u64s, just like we do for pCode, makes our PEB portable. Signed-off-by: danbugs --- src/hyperlight_common/src/mem.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/hyperlight_common/src/mem.rs b/src/hyperlight_common/src/mem.rs index d9af6dc09..f01eaaf80 100644 --- a/src/hyperlight_common/src/mem.rs +++ b/src/hyperlight_common/src/mem.rs @@ -20,24 +20,22 @@ pub const PAGE_SHIFT: u64 = 12; pub const PAGE_SIZE: u64 = 1 << 12; pub const PAGE_SIZE_USIZE: usize = 1 << 12; -use core::ffi::{c_char, c_void}; - #[repr(C)] pub struct InputData { pub inputDataSize: u64, - pub inputDataBuffer: *mut c_void, + pub inputDataBuffer: u64, } #[repr(C)] pub struct OutputData { pub outputDataSize: u64, - pub outputDataBuffer: *mut c_void, + pub outputDataBuffer: u64, } #[repr(C)] pub struct GuestHeapData { pub guestHeapSize: u64, - pub guestHeapBuffer: *mut c_void, + pub guestHeapBuffer: u64, } #[repr(C)] @@ -52,7 +50,7 @@ pub struct GuestStackData { pub struct HyperlightPEB { pub security_cookie_seed: u64, pub guest_function_dispatch_ptr: u64, - pub pCode: *mut c_char, + pub pCode: u64, pub inputdata: InputData, pub outputdata: OutputData, pub guestheapData: GuestHeapData, From 5d728d9c19fa706ac2eadeb8a6992469b9f45d54 Mon Sep 17 00:00:00 2001 From: danbugs Date: Fri, 23 May 2025 17:31:01 +0000 Subject: [PATCH 3/5] [common/mem] make peb structures debug, clone, and copy It's useful for a library to provide implementaitons of these traits for its structures. Signed-off-by: danbugs --- src/hyperlight_common/src/mem.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/hyperlight_common/src/mem.rs b/src/hyperlight_common/src/mem.rs index f01eaaf80..e678d6baf 100644 --- a/src/hyperlight_common/src/mem.rs +++ b/src/hyperlight_common/src/mem.rs @@ -20,24 +20,28 @@ pub const PAGE_SHIFT: u64 = 12; pub const PAGE_SIZE: u64 = 1 << 12; pub const PAGE_SIZE_USIZE: usize = 1 << 12; +#[derive(Debug, Clone, Copy)] #[repr(C)] pub struct InputData { pub inputDataSize: u64, pub inputDataBuffer: u64, } +#[derive(Debug, Clone, Copy)] #[repr(C)] pub struct OutputData { pub outputDataSize: u64, pub outputDataBuffer: u64, } +#[derive(Debug, Clone, Copy)] #[repr(C)] pub struct GuestHeapData { pub guestHeapSize: u64, pub guestHeapBuffer: u64, } +#[derive(Debug, Clone, Copy)] #[repr(C)] pub struct GuestStackData { /// This is the top of the user stack @@ -46,6 +50,7 @@ pub struct GuestStackData { pub userStackAddress: u64, } +#[derive(Debug, Clone, Copy)] #[repr(C)] pub struct HyperlightPEB { pub security_cookie_seed: u64, From 49942bb7e29eb639b834d317caa7cabc094fd29f Mon Sep 17 00:00:00 2001 From: danbugs Date: Fri, 23 May 2025 17:41:43 +0000 Subject: [PATCH 4/5] [common,guest,host] refactor PEB-related structs - InputData, OutputData, and GuestHeapData were functionally the same and so they were combined into a single GuestMemoryRegion struct. - removed the allow_non_snake_case attribute. - general housekeeping. Signed-off-by: danbugs --- src/hyperlight_common/src/mem.rs | 46 +++++++------------ src/hyperlight_guest/src/entrypoint.rs | 6 +-- src/hyperlight_guest/src/shared_input_data.rs | 10 ++-- .../src/shared_output_data.rs | 10 ++-- src/hyperlight_host/src/mem/layout.rs | 20 ++++---- 5 files changed, 36 insertions(+), 56 deletions(-) diff --git a/src/hyperlight_common/src/mem.rs b/src/hyperlight_common/src/mem.rs index e678d6baf..36d01e707 100644 --- a/src/hyperlight_common/src/mem.rs +++ b/src/hyperlight_common/src/mem.rs @@ -14,40 +14,28 @@ See the License for the specific language governing permissions and limitations under the License. */ -#![allow(non_snake_case)] - pub const PAGE_SHIFT: u64 = 12; pub const PAGE_SIZE: u64 = 1 << 12; pub const PAGE_SIZE_USIZE: usize = 1 << 12; +/// A memory region in the guest address space #[derive(Debug, Clone, Copy)] #[repr(C)] -pub struct InputData { - pub inputDataSize: u64, - pub inputDataBuffer: u64, -} - -#[derive(Debug, Clone, Copy)] -#[repr(C)] -pub struct OutputData { - pub outputDataSize: u64, - pub outputDataBuffer: u64, -} - -#[derive(Debug, Clone, Copy)] -#[repr(C)] -pub struct GuestHeapData { - pub guestHeapSize: u64, - pub guestHeapBuffer: u64, +pub struct GuestMemoryRegion { + /// The size of the memory region + pub size: u64, + /// The address of the memory region + pub ptr: u64, } +/// A memory region in the guest address space that is used for the stack #[derive(Debug, Clone, Copy)] #[repr(C)] -pub struct GuestStackData { - /// This is the top of the user stack - pub minUserStackAddress: u64, - /// This is the user stack pointer - pub userStackAddress: u64, +pub struct GuestStack { + /// The top of the user stack + pub min_user_stack_address: u64, + /// The user stack pointer + pub user_stack_address: u64, } #[derive(Debug, Clone, Copy)] @@ -55,9 +43,9 @@ pub struct GuestStackData { pub struct HyperlightPEB { pub security_cookie_seed: u64, pub guest_function_dispatch_ptr: u64, - pub pCode: u64, - pub inputdata: InputData, - pub outputdata: OutputData, - pub guestheapData: GuestHeapData, - pub gueststackData: GuestStackData, + pub code_ptr: u64, + pub input_stack: GuestMemoryRegion, + pub output_stack: GuestMemoryRegion, + pub guest_heap: GuestMemoryRegion, + pub guest_stack: GuestStack, } diff --git a/src/hyperlight_guest/src/entrypoint.rs b/src/hyperlight_guest/src/entrypoint.rs index cc559a8d2..891f3da59 100644 --- a/src/hyperlight_guest/src/entrypoint.rs +++ b/src/hyperlight_guest/src/entrypoint.rs @@ -98,7 +98,7 @@ pub extern "C" fn entrypoint(peb_address: u64, seed: u64, ops: u64, max_log_leve // This static is to make it easier to implement the __chkstk function in assembly. // It also means that should we change the layout of the struct in the future, we // don't have to change the assembly code. - MIN_STACK_ADDRESS = (*peb_ptr).gueststackData.minUserStackAddress; + MIN_STACK_ADDRESS = (*peb_ptr).guest_stack.min_user_stack_address; #[cfg(target_arch = "x86_64")] { @@ -107,8 +107,8 @@ pub extern "C" fn entrypoint(peb_address: u64, seed: u64, ops: u64, max_log_leve load_idt(); } - let heap_start = (*peb_ptr).guestheapData.guestHeapBuffer as usize; - let heap_size = (*peb_ptr).guestheapData.guestHeapSize as usize; + let heap_start = (*peb_ptr).guest_heap.ptr as usize; + let heap_size = (*peb_ptr).guest_heap.size as usize; HEAP_ALLOCATOR .try_lock() .expect("Failed to access HEAP_ALLOCATOR") diff --git a/src/hyperlight_guest/src/shared_input_data.rs b/src/hyperlight_guest/src/shared_input_data.rs index 8d943db07..ff7dbb00d 100644 --- a/src/hyperlight_guest/src/shared_input_data.rs +++ b/src/hyperlight_guest/src/shared_input_data.rs @@ -30,14 +30,10 @@ where T: for<'a> TryFrom<&'a [u8]>, { let peb_ptr = unsafe { P_PEB.unwrap() }; - let shared_buffer_size = unsafe { (*peb_ptr).inputdata.inputDataSize as usize }; + let shared_buffer_size = unsafe { (*peb_ptr).input_stack.size as usize }; - let idb = unsafe { - from_raw_parts_mut( - (*peb_ptr).inputdata.inputDataBuffer as *mut u8, - shared_buffer_size, - ) - }; + let idb = + unsafe { from_raw_parts_mut((*peb_ptr).input_stack.ptr as *mut u8, shared_buffer_size) }; if idb.is_empty() { return Err(HyperlightGuestError::new( diff --git a/src/hyperlight_guest/src/shared_output_data.rs b/src/hyperlight_guest/src/shared_output_data.rs index 520a369f3..205f4d06c 100644 --- a/src/hyperlight_guest/src/shared_output_data.rs +++ b/src/hyperlight_guest/src/shared_output_data.rs @@ -26,13 +26,9 @@ use crate::P_PEB; pub fn push_shared_output_data(data: Vec) -> Result<()> { let peb_ptr = unsafe { P_PEB.unwrap() }; - let shared_buffer_size = unsafe { (*peb_ptr).outputdata.outputDataSize as usize }; - let odb = unsafe { - from_raw_parts_mut( - (*peb_ptr).outputdata.outputDataBuffer as *mut u8, - shared_buffer_size, - ) - }; + let shared_buffer_size = unsafe { (*peb_ptr).output_stack.size as usize }; + let odb = + unsafe { from_raw_parts_mut((*peb_ptr).output_stack.ptr as *mut u8, shared_buffer_size) }; if odb.is_empty() { return Err(HyperlightGuestError::new( diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index 7dea36cde..1c1faf561 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -16,7 +16,7 @@ limitations under the License. use std::fmt::Debug; use std::mem::{offset_of, size_of}; -use hyperlight_common::mem::{GuestStackData, HyperlightPEB, PAGE_SIZE_USIZE}; +use hyperlight_common::mem::{GuestStack, HyperlightPEB, PAGE_SIZE_USIZE}; use rand::{rng, RngCore}; use tracing::{instrument, Span}; @@ -225,17 +225,17 @@ 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_code_pointer_offset = peb_offset + offset_of!(HyperlightPEB, pCode); - let peb_input_data_offset = peb_offset + offset_of!(HyperlightPEB, inputdata); - let peb_output_data_offset = peb_offset + offset_of!(HyperlightPEB, outputdata); - let peb_heap_data_offset = peb_offset + offset_of!(HyperlightPEB, guestheapData); - let peb_guest_stack_data_offset = peb_offset + offset_of!(HyperlightPEB, gueststackData); + let peb_code_pointer_offset = peb_offset + offset_of!(HyperlightPEB, code_ptr); + let peb_input_data_offset = peb_offset + offset_of!(HyperlightPEB, input_stack); + let peb_output_data_offset = peb_offset + offset_of!(HyperlightPEB, output_stack); + let peb_heap_data_offset = peb_offset + offset_of!(HyperlightPEB, guest_heap); + let peb_guest_stack_data_offset = peb_offset + offset_of!(HyperlightPEB, guest_stack); // 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 input_data_buffer_offset = round_up_to( - peb_guest_stack_data_offset + size_of::(), + peb_guest_stack_data_offset + size_of::(), PAGE_SIZE_USIZE, ); let output_data_buffer_offset = round_up_to( @@ -332,7 +332,7 @@ impl SandboxMemoryLayout { /// Get the offset in guest memory to the input data size. #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub(super) fn get_input_data_size_offset(&self) -> usize { - // The input data size is the first field in the `InputData` struct + // The input data size is the first field in the input stack's `GuestMemoryRegion` struct self.peb_input_data_offset } @@ -340,7 +340,7 @@ impl SandboxMemoryLayout { #[instrument(skip_all, parent = Span::current(), level= "Trace")] fn get_input_data_pointer_offset(&self) -> usize { // The input data pointer is immediately after the input - // data size field in the `InputData` struct which is a `u64`. + // data size field in the input data `GuestMemoryRegion` struct which is a `u64`. self.get_input_data_size_offset() + size_of::() } @@ -375,7 +375,7 @@ impl SandboxMemoryLayout { #[instrument(skip_all, parent = Span::current(), level= "Trace")] fn get_heap_pointer_offset(&self) -> usize { // The heap pointer is immediately after the - // heap size field in the `GuestHeap` struct which is a `u64`. + // heap size field in the guest heap's `GuestMemoryRegion` struct which is a `u64`. self.get_heap_size_offset() + size_of::() } From 0f91d6c73cb607595ddef8b8edf5096915f6c571 Mon Sep 17 00:00:00 2001 From: danbugs Date: Fri, 23 May 2025 18:32:43 +0000 Subject: [PATCH 5/5] [guest/{input,output}stacks] make input/output stacks portable If we use usize and the guest and host are compiled for different architectures, there will be a mismatch in sizing between the guest and host, so we should use u64s to address that (tested w/ Nanvix already). Signed-off-by: danbugs --- src/hyperlight_guest/src/shared_input_data.rs | 14 +++++++------- src/hyperlight_guest/src/shared_output_data.rs | 17 +++++++++-------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/hyperlight_guest/src/shared_input_data.rs b/src/hyperlight_guest/src/shared_input_data.rs index ff7dbb00d..d4b620ca9 100644 --- a/src/hyperlight_guest/src/shared_input_data.rs +++ b/src/hyperlight_guest/src/shared_input_data.rs @@ -43,10 +43,10 @@ where } // get relative offset to next free address - let stack_ptr_rel: usize = - usize::from_le_bytes(idb[..8].try_into().expect("Shared input buffer too small")); + let stack_ptr_rel: u64 = + u64::from_le_bytes(idb[..8].try_into().expect("Shared input buffer too small")); - if stack_ptr_rel > shared_buffer_size || stack_ptr_rel < 16 { + if stack_ptr_rel as usize > shared_buffer_size || stack_ptr_rel < 16 { return Err(HyperlightGuestError::new( ErrorCode::GuestError, format!( @@ -57,13 +57,13 @@ where } // go back 8 bytes and read. This is the offset to the element on top of stack - let last_element_offset_rel = usize::from_le_bytes( - idb[stack_ptr_rel - 8..stack_ptr_rel] + let last_element_offset_rel = u64::from_le_bytes( + idb[stack_ptr_rel as usize - 8..stack_ptr_rel as usize] .try_into() .expect("Invalid stack pointer in pop_shared_input_data_into"), ); - let buffer = &idb[last_element_offset_rel..]; + let buffer = &idb[last_element_offset_rel as usize..]; // convert the buffer to T let type_t = match T::try_from(buffer) { @@ -80,7 +80,7 @@ where idb[..8].copy_from_slice(&last_element_offset_rel.to_le_bytes()); // zero out popped off buffer - idb[last_element_offset_rel..stack_ptr_rel].fill(0); + idb[last_element_offset_rel as usize..stack_ptr_rel as usize].fill(0); type_t } diff --git a/src/hyperlight_guest/src/shared_output_data.rs b/src/hyperlight_guest/src/shared_output_data.rs index 205f4d06c..6deb34fec 100644 --- a/src/hyperlight_guest/src/shared_output_data.rs +++ b/src/hyperlight_guest/src/shared_output_data.rs @@ -38,13 +38,13 @@ pub fn push_shared_output_data(data: Vec) -> Result<()> { } // get offset to next free address on the stack - let stack_ptr_rel: usize = - usize::from_le_bytes(odb[..8].try_into().expect("Shared output buffer too small")); + let stack_ptr_rel: u64 = + u64::from_le_bytes(odb[..8].try_into().expect("Shared output buffer too small")); // check if the stack pointer is within the bounds of the buffer. // It can be equal to the size, but never greater // It can never be less than 8. An empty buffer's stack pointer is 8 - if stack_ptr_rel > shared_buffer_size || stack_ptr_rel < 8 { + if stack_ptr_rel as usize > shared_buffer_size || stack_ptr_rel < 8 { return Err(HyperlightGuestError::new( ErrorCode::GuestError, format!( @@ -56,7 +56,7 @@ pub fn push_shared_output_data(data: Vec) -> Result<()> { // check if there is enough space in the buffer let size_required = data.len() + 8; // the data plus the pointer pointing to the data - let size_available = shared_buffer_size - stack_ptr_rel; + let size_available = shared_buffer_size - stack_ptr_rel as usize; if size_required > size_available { return Err(HyperlightGuestError::new( ErrorCode::GuestError, @@ -68,14 +68,15 @@ pub fn push_shared_output_data(data: Vec) -> Result<()> { } // write the actual data - odb[stack_ptr_rel..stack_ptr_rel + data.len()].copy_from_slice(&data); + odb[stack_ptr_rel as usize..stack_ptr_rel as usize + data.len()].copy_from_slice(&data); // write the offset to the newly written data, to the top of the stack - let bytes = stack_ptr_rel.to_le_bytes(); - odb[stack_ptr_rel + data.len()..stack_ptr_rel + data.len() + 8].copy_from_slice(&bytes); + let bytes: [u8; 8] = stack_ptr_rel.to_le_bytes(); + odb[stack_ptr_rel as usize + data.len()..stack_ptr_rel as usize + data.len() + 8] + .copy_from_slice(&bytes); // update stack pointer to point to next free address - let new_stack_ptr_rel = stack_ptr_rel + data.len() + 8; + let new_stack_ptr_rel: u64 = (stack_ptr_rel as usize + data.len() + 8) as u64; odb[0..8].copy_from_slice(&(new_stack_ptr_rel).to_le_bytes()); Ok(())