diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index de357b7ca..e08e256af 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -385,24 +385,6 @@ impl SandboxMemoryLayout { self.stack_size } - /// Get the offset in guest memory to the OutB pointer. - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - #[allow(dead_code)] - pub(super) fn get_outb_pointer_offset(&self) -> usize { - // The outb pointer is immediately after the code pointer - // in the `CodeAndOutBPointers` struct which is a u64 - self.peb_code_pointer_offset + size_of::() - } - - /// Get the offset in guest memory to the OutB context. - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - #[allow(dead_code)] - pub(super) fn get_outb_context_offset(&self) -> usize { - // The outb context is immediately after the outb pointer - // in the `CodeAndOutBPointers` struct which is a u64 - self.get_outb_pointer_offset() + size_of::() - } - /// Get the offset in guest memory to the output data pointer. #[instrument(skip_all, parent = Span::current(), level= "Trace")] fn get_output_data_pointer_offset(&self) -> usize { @@ -420,11 +402,8 @@ impl SandboxMemoryLayout { } /// Get the offset in guest memory to the start of output data. - /// - /// This function exists to accommodate the macro that generates C API - /// compatible functions. #[instrument(skip_all, parent = Span::current(), level= "Trace")] - #[allow(dead_code)] + #[cfg(test)] pub(crate) fn get_output_data_offset(&self) -> usize { self.output_data_buffer_offset } @@ -459,13 +438,6 @@ impl SandboxMemoryLayout { self.peb_guest_dispatch_function_ptr_offset } - /// Get the offset in guest memory to the PEB address - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - #[allow(dead_code)] - pub(super) fn get_in_process_peb_offset(&self) -> usize { - self.peb_offset - } - /// Get the offset in guest memory to the heap size #[instrument(skip_all, parent = Span::current(), level= "Trace")] fn get_heap_size_offset(&self) -> usize { @@ -494,13 +466,6 @@ impl SandboxMemoryLayout { self.get_min_guest_stack_address_offset() + size_of::() } - /// Get the offset to the guest guard page - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - #[allow(dead_code)] - pub fn get_guard_page_offset(&self) -> usize { - self.guard_page_offset - } - /// Get the total size of guest memory in `self`'s memory /// layout. #[instrument(skip_all, parent = Span::current(), level= "Trace")] diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index e33c4b08d..787f095e2 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -281,21 +281,6 @@ where snapshot.restore_from_snapshot(&mut self.shared_mem)?; Ok(()) } - - /// Sets `addr` to the correct offset in the memory referenced by - /// `shared_mem` to indicate the address of the outb pointer and context - /// for calling outb function - #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - #[allow(dead_code)] - pub(crate) fn set_outb_address_and_context(&mut self, addr: u64, context: u64) -> Result<()> { - let pointer_offset = self.layout.get_outb_pointer_offset(); - let context_offset = self.layout.get_outb_context_offset(); - self.shared_mem.with_exclusivity(|excl| -> Result<()> { - excl.write_u64(pointer_offset, addr)?; - excl.write_u64(context_offset, context)?; - Ok(()) - })? - } } impl SandboxMemoryManager { diff --git a/src/hyperlight_host/src/mem/shared_mem.rs b/src/hyperlight_host/src/mem/shared_mem.rs index bd39a73fd..84cbb4c8e 100644 --- a/src/hyperlight_host/src/mem/shared_mem.rs +++ b/src/hyperlight_host/src/mem/shared_mem.rs @@ -30,7 +30,7 @@ use windows::Win32::System::Memory::PAGE_READWRITE; #[cfg(target_os = "windows")] use windows::Win32::System::Memory::{ CreateFileMappingA, FILE_MAP_ALL_ACCESS, MEMORY_MAPPED_VIEW_ADDRESS, MapViewOfFile, - PAGE_EXECUTE_READWRITE, PAGE_NOACCESS, PAGE_PROTECTION_FLAGS, UnmapViewOfFile, VirtualProtect, + PAGE_NOACCESS, PAGE_PROTECTION_FLAGS, UnmapViewOfFile, VirtualProtect, }; #[cfg(target_os = "windows")] use windows::core::PCSTR; @@ -502,46 +502,6 @@ impl ExclusiveSharedMemory { }) } - #[allow(dead_code)] - pub(super) fn make_memory_executable(&self) -> Result<()> { - #[cfg(target_os = "windows")] - { - let mut _old_flags = PAGE_PROTECTION_FLAGS::default(); - if let Err(e) = unsafe { - VirtualProtect( - self.region.ptr as *const c_void, - self.region.size, - PAGE_EXECUTE_READWRITE, - &mut _old_flags as *mut PAGE_PROTECTION_FLAGS, - ) - } { - log_then_return!(WindowsAPIError(e.clone())); - } - } - - // make the memory executable on Linux - #[cfg(target_os = "linux")] - { - use libc::{PROT_EXEC, PROT_READ, PROT_WRITE, mprotect}; - - let res = unsafe { - mprotect( - self.region.ptr as *mut c_void, - self.region.size, - PROT_READ | PROT_WRITE | PROT_EXEC, - ) - }; - - if res != 0 { - return Err(new_error!( - "Failed to make memory executable: {:#?}", - Error::last_os_error().raw_os_error() - )); - } - } - Ok(()) - } - /// Internal helper method to get the backing memory as a mutable slice. /// /// # Safety @@ -614,15 +574,6 @@ impl ExclusiveSharedMemory { Ok(()) } - /// Return the address of memory at an offset to this `SharedMemory` checking - /// that the memory is within the bounds of the `SharedMemory`. - #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - #[allow(dead_code)] - pub(crate) fn calculate_address(&self, offset: usize) -> Result { - bounds_check!(offset, 0, self.mem_size()); - Ok(self.base_addr() + offset) - } - generate_reader!(read_u8, u8); generate_reader!(read_i8, i8); generate_reader!(read_u16, u16); diff --git a/src/hyperlight_host/src/mem/shared_mem_snapshot.rs b/src/hyperlight_host/src/mem/shared_mem_snapshot.rs index 49af2d99d..dd44422df 100644 --- a/src/hyperlight_host/src/mem/shared_mem_snapshot.rs +++ b/src/hyperlight_host/src/mem/shared_mem_snapshot.rs @@ -50,15 +50,6 @@ impl SharedMemorySnapshot { }) } - /// Take another snapshot of the internally-stored `SharedMemory`, - /// then store it internally. - #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - #[allow(dead_code)] - pub(super) fn replace_snapshot(&mut self, shared_mem: &mut S) -> Result<()> { - self.snapshot = shared_mem.with_exclusivity(|e| e.copy_all_to_vec())??; - Ok(()) - } - /// Copy the memory from the internally-stored memory snapshot /// into the internally-stored `SharedMemory`. #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] @@ -91,36 +82,58 @@ mod tests { use crate::mem::shared_mem::ExclusiveSharedMemory; #[test] - fn restore_replace() { - let mut data1 = vec![b'a', b'b', b'c']; - data1.resize_with(PAGE_SIZE_USIZE, || 0); - let data2 = data1.iter().map(|b| b + 1).collect::>(); + fn restore() { + // Simplified version of the original test + let data1 = vec![b'a'; PAGE_SIZE_USIZE]; + let data2 = vec![b'b'; PAGE_SIZE_USIZE]; + let mut gm = ExclusiveSharedMemory::new(PAGE_SIZE_USIZE).unwrap(); - gm.copy_from_slice(data1.as_slice(), 0).unwrap(); - let mut snap = super::SharedMemorySnapshot::new(&mut gm, 0, Vec::new()).unwrap(); - { - // after the first snapshot is taken, make sure gm has the equivalent - // of data1 - assert_eq!(data1, gm.copy_all_to_vec().unwrap()); - } - - { - // modify gm with data2 rather than data1 and restore from - // snapshot. we should have the equivalent of data1 again - gm.copy_from_slice(data2.as_slice(), 0).unwrap(); - assert_eq!(data2, gm.copy_all_to_vec().unwrap()); - snap.restore_from_snapshot(&mut gm).unwrap(); - assert_eq!(data1, gm.copy_all_to_vec().unwrap()); - } - { - // modify gm with data2, then retake the snapshot and restore - // from the new snapshot. we should have the equivalent of data2 - gm.copy_from_slice(data2.as_slice(), 0).unwrap(); - assert_eq!(data2, gm.copy_all_to_vec().unwrap()); - snap.replace_snapshot(&mut gm).unwrap(); - assert_eq!(data2, gm.copy_all_to_vec().unwrap()); - snap.restore_from_snapshot(&mut gm).unwrap(); - assert_eq!(data2, gm.copy_all_to_vec().unwrap()); - } + gm.copy_from_slice(&data1, 0).unwrap(); + + // Take snapshot of data1 + let snapshot = super::SharedMemorySnapshot::new(&mut gm, 0, Vec::new()).unwrap(); + + // Modify memory to data2 + gm.copy_from_slice(&data2, 0).unwrap(); + assert_eq!(gm.as_slice(), &data2[..]); + + // Restore should bring back data1 + snapshot.restore_from_snapshot(&mut gm).unwrap(); + assert_eq!(gm.as_slice(), &data1[..]); + } + + #[test] + fn snapshot_mem_size() { + let size = PAGE_SIZE_USIZE * 2; + let mut gm = ExclusiveSharedMemory::new(size).unwrap(); + + let snapshot = super::SharedMemorySnapshot::new(&mut gm, 0, Vec::new()).unwrap(); + assert_eq!(snapshot.mem_size(), size); + } + + #[test] + fn multiple_snapshots_independent() { + let mut gm = ExclusiveSharedMemory::new(PAGE_SIZE_USIZE).unwrap(); + + // Create first snapshot with pattern A + let pattern_a = vec![0xAA; PAGE_SIZE_USIZE]; + gm.copy_from_slice(&pattern_a, 0).unwrap(); + let snapshot_a = super::SharedMemorySnapshot::new(&mut gm, 1, Vec::new()).unwrap(); + + // Create second snapshot with pattern B + let pattern_b = vec![0xBB; PAGE_SIZE_USIZE]; + gm.copy_from_slice(&pattern_b, 0).unwrap(); + let snapshot_b = super::SharedMemorySnapshot::new(&mut gm, 2, Vec::new()).unwrap(); + + // Clear memory + gm.copy_from_slice(&[0; PAGE_SIZE_USIZE], 0).unwrap(); + + // Restore snapshot A + snapshot_a.restore_from_snapshot(&mut gm).unwrap(); + assert_eq!(gm.as_slice(), &pattern_a[..]); + + // Restore snapshot B + snapshot_b.restore_from_snapshot(&mut gm).unwrap(); + assert_eq!(gm.as_slice(), &pattern_b[..]); } } diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index dfdcc9668..b1b41fff2 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -333,7 +333,6 @@ impl MultiUseSandbox { /// Map the contents of a file into the guest at a particular address /// /// Returns the length of the mapping in bytes. - #[allow(dead_code)] #[instrument(err(Debug), skip(self, _fp, _guest_base), parent = Span::current())] pub fn map_file_cow(&mut self, _fp: &Path, _guest_base: u64) -> Result { #[cfg(windows)]