Use CreateFileMapping\MapViewOfFile and UnmapViewOfFile\CloseHandle instead of VitualAllocEx and VirtualFreeEx on Windows

src/hyperlight_host/Cargo.toml

@@ -62,6 +62,7 @@ windows = { version = "0.59", features = [
     "Win32_System_Memory",
     "Win32_System_Threading",
     "Win32_System_JobObjects",
+    "Win32_System_SystemServices",
 ] }
 windows-sys = { version = "0.59", features = ["Win32"] }
 windows-result = "0.3"

src/hyperlight_host/src/hypervisor/hyperv_windows.rs

@@ -21,7 +21,6 @@ use std::string::String;
 
 use hyperlight_common::mem::PAGE_SIZE_USIZE;
 use tracing::{instrument, Span};
-use windows::Win32::Foundation::HANDLE;
 use windows::Win32::System::Hypervisor::{
     WHvX64RegisterCr0, WHvX64RegisterCr3, WHvX64RegisterCr4, WHvX64RegisterCs, WHvX64RegisterEfer,
     WHV_MEMORY_ACCESS_TYPE, WHV_PARTITION_HANDLE, WHV_REGISTER_VALUE, WHV_RUN_VP_EXIT_CONTEXT,
@@ -33,7 +32,7 @@ use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper};
 use super::surrogate_process::SurrogateProcess;
 use super::surrogate_process_manager::*;
 use super::windows_hypervisor_platform::{VMPartition, VMProcessor};
-use super::wrappers::WHvFPURegisters;
+use super::wrappers::{HandleWrapper, WHvFPURegisters};
 use super::{
     HyperlightExit, Hypervisor, VirtualCPU, CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP,
     CR4_OSFXSR, CR4_OSXMMEXCPT, CR4_PAE, EFER_LMA, EFER_LME, EFER_NX, EFER_SCE,
@@ -43,15 +42,14 @@ use crate::hypervisor::hypervisor_handler::HypervisorHandler;
 use crate::hypervisor::wrappers::WHvGeneralRegisters;
 use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags};
 use crate::mem::ptr::{GuestPtr, RawPtr};
-use crate::HyperlightError::WindowsAPIError;
-use crate::{debug, log_then_return, new_error, Result};
+use crate::{debug, new_error, Result};
 
 /// A Hypervisor driver for HyperV-on-Windows.
 pub(crate) struct HypervWindowsDriver {
     size: usize, // this is the size of the memory region, excluding the 2 surrounding guard pages
     processor: VMProcessor,
-    surrogate_process: SurrogateProcess,
-    source_address: *mut c_void, // this points into the first guard page
+    _surrogate_process: SurrogateProcess, // we need to keep a reference to the SurrogateProcess for the duration of the driver since otherwise it will dropped and the memory mapping will be unmapped and the surrogate process will be returned to the pool
+    source_address: *mut c_void,          // this points into the first guard page
     entrypoint: u64,
     orig_rsp: GuestPtr,
     mem_regions: Vec<MemoryRegion>,
@@ -73,6 +71,7 @@ impl HypervWindowsDriver {
         pml4_address: u64,
         entrypoint: u64,
         rsp: u64,
+        mmap_file_handle: HandleWrapper,
     ) -> Result<Self> {
         // create and setup hypervisor partition
         let mut partition = VMPartition::new(1)?;
@@ -81,7 +80,7 @@ impl HypervWindowsDriver {
         // with guard pages setup
         let surrogate_process = {
             let mgr = get_surrogate_process_manager()?;
-            mgr.get_surrogate_process(raw_size, raw_source_address)
+            mgr.get_surrogate_process(raw_size, raw_source_address, mmap_file_handle)
         }?;
 
         partition.map_gpa_range(&mem_regions, surrogate_process.process_handle)?;
@@ -95,7 +94,7 @@ impl HypervWindowsDriver {
         Ok(Self {
             size: mem_size,
             processor: proc,
-            surrogate_process,
+            _surrogate_process: surrogate_process,
             source_address: raw_source_address,
             entrypoint,
             orig_rsp: GuestPtr::try_from(RawPtr::from(rsp))?,
@@ -402,54 +401,8 @@ impl Hypervisor for HypervWindowsDriver {
 
     #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")]
     fn run(&mut self) -> Result<super::HyperlightExit> {
-        let bytes_written: Option<*mut usize> = None;
-        let bytes_read: Option<*mut usize> = None;
-        let handle: HANDLE = self.surrogate_process.process_handle.into();
-
-        // TODO optimise this
-        // the following write to and read from process memory is required as we need to use
-        // surrogate processes to allow more than one WHP Partition per process
-        // see HyperVSurrogateProcessManager
-        // this needs updating so that
-        // 1. it only writes to memory that changes between usage
-        // 2. memory is allocated in the process once and then only freed and reallocated if the
-        // memory needs to grow.
-
-        // - copy stuff to surrogate process
-
-        if let Err(e) = unsafe {
-            windows::Win32::System::Diagnostics::Debug::WriteProcessMemory(
-                handle,
-                self.surrogate_process
-                    .allocated_address
-                    .add(PAGE_SIZE_USIZE),
-                self.source_address.add(PAGE_SIZE_USIZE),
-                self.size,
-                bytes_written,
-            )
-        } {
-            log_then_return!(WindowsAPIError(e.clone()));
-        }
-
-        // - call WHvRunVirtualProcessor
         let exit_context: WHV_RUN_VP_EXIT_CONTEXT = self.processor.run()?;
 
-        // - call read-process memory
-
-        if let Err(e) = unsafe {
-            windows::Win32::System::Diagnostics::Debug::ReadProcessMemory(
-                handle,
-                self.surrogate_process
-                    .allocated_address
-                    .add(PAGE_SIZE_USIZE),
-                self.source_address.add(PAGE_SIZE_USIZE),
-                self.size,
-                bytes_read,
-            )
-        } {
-            log_then_return!(WindowsAPIError(e.clone()));
-        }
-
         let result = match exit_context.ExitReason {
             // WHvRunVpExitReasonX64IoPortAccess
             WHV_RUN_VP_EXIT_REASON(2i32) => {

src/hyperlight_host/src/hypervisor/hypervisor_handler.rs

@@ -38,6 +38,8 @@ use windows::Win32::System::Hypervisor::{WHvCancelRunVirtualProcessor, WHV_PARTI
 #[cfg(feature = "function_call_metrics")]
 use crate::histogram_vec_observe;
 use crate::hypervisor::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper};
+#[cfg(target_os = "windows")]
+use crate::hypervisor::wrappers::HandleWrapper;
 use crate::hypervisor::Hypervisor;
 use crate::mem::layout::SandboxMemoryLayout;
 use crate::mem::mgr::SandboxMemoryManager;
@@ -905,13 +907,17 @@ fn set_up_hypervisor_partition(
 
             #[cfg(target_os = "windows")]
             Some(HypervisorType::Whp) => {
+                let mmap_file_handle = mgr
+                    .shared_mem
+                    .with_exclusivity(|e| e.get_mmap_file_handle())?;
                 let hv = crate::hypervisor::hyperv_windows::HypervWindowsDriver::new(
                     regions,
                     mgr.shared_mem.raw_mem_size(), // we use raw_* here because windows driver requires 64K aligned addresses,
                     mgr.shared_mem.raw_ptr() as *mut c_void, // and instead convert it to base_addr where needed in the driver itself
                     pml4_ptr.absolute()?,
                     entrypoint_ptr.absolute()?,
                     rsp_ptr.absolute()?,
+                    HandleWrapper::from(mmap_file_handle),
                 )?;
                 Ok(Box::new(hv))
             }

src/hyperlight_host/src/hypervisor/surrogate_process.rs

@@ -18,7 +18,9 @@ use core::ffi::c_void;
 
 use tracing::{instrument, Span};
 use windows::Win32::Foundation::HANDLE;
-use windows::Win32::System::Memory::{VirtualFreeEx, MEM_RELEASE};
+use windows::Win32::System::Memory::{
+    UnmapViewOfFile2, MEMORY_MAPPED_VIEW_ADDRESS, UNMAP_VIEW_OF_FILE_FLAGS,
+};
 
 use super::surrogate_process_manager::get_surrogate_process_manager;
 use super::wrappers::HandleWrapper;
@@ -54,10 +56,16 @@ impl Default for SurrogateProcess {
 impl Drop for SurrogateProcess {
     #[instrument(skip_all, parent = Span::current(), level= "Trace")]
     fn drop(&mut self) {
-        let handle: HANDLE = self.process_handle.into();
-        if let Err(e) = unsafe { VirtualFreeEx(handle, self.allocated_address, 0, MEM_RELEASE) } {
+        let process_handle: HANDLE = self.process_handle.into();
+        let memory_mapped_view_address = MEMORY_MAPPED_VIEW_ADDRESS {
+            Value: self.allocated_address,
+        };
+        let flags = UNMAP_VIEW_OF_FILE_FLAGS(0);
+        if let Err(e) =
+            unsafe { UnmapViewOfFile2(process_handle, memory_mapped_view_address, flags) }
+        {
             tracing::error!(
-                "Failed to free surrogate process resources (VirtualFreeEx failed): {:?}",
+                "Failed to free surrogate process resources (UnmapViewOfFile2 failed): {:?}",
                 e
             );
         }
@@ -66,9 +74,21 @@ impl Drop for SurrogateProcess {
         // of the SurrogateProcess being dropped. this is ok to
         // do because we are in the process of dropping ourselves
         // anyway.
-        get_surrogate_process_manager()
-            .unwrap()
-            .return_surrogate_process(self.process_handle)
-            .unwrap();
+        match get_surrogate_process_manager() {
+            Ok(manager) => match manager.return_surrogate_process(self.process_handle) {
+                Ok(_) => (),
+                Err(e) => {
+                    tracing::error!("Failed to return surrogate process to surrogate process manager when dropping : {:?}", e);
+                    return;
+                }
+            },
+            Err(e) => {
+                tracing::error!(
+                    "Failed to get surrogate process manager when dropping SurrogateProcess: {:?}",
+                    e
+                );
+                return;
+            }
+        }
     }
 }

src/hyperlight_host/src/hypervisor/surrogate_process_manager.rs

@@ -33,9 +33,9 @@ use windows::Win32::System::JobObjects::{
     JOBOBJECT_EXTENDED_LIMIT_INFORMATION, JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE,
 };
 use windows::Win32::System::Memory::{
-    VirtualAllocEx, VirtualProtectEx, MEM_COMMIT, MEM_RESERVE, PAGE_NOACCESS,
-    PAGE_PROTECTION_FLAGS, PAGE_READWRITE,
+    MapViewOfFileNuma2, VirtualProtectEx, PAGE_NOACCESS, PAGE_PROTECTION_FLAGS, PAGE_READWRITE,
 };
+use windows::Win32::System::SystemServices::NUMA_NO_PREFERRED_NODE;
 use windows::Win32::System::Threading::{
     CreateProcessA, CREATE_SUSPENDED, PROCESS_INFORMATION, STARTUPINFOA,
 };
@@ -79,7 +79,7 @@ const NUMBER_OF_SURROGATE_PROCESSES: usize = 512;
 /// There is, however, another API (WHvMapGpaRange2) that has a second
 /// parameter which is a handle to a process. This process merely has to exist,
 /// the memory being mapped from the host to the virtual machine is
-/// allocated/freed  in this process using VirtualAllocEx/VirtualFreeEx.
+/// allocated/freed  in this process using CreateFileMapping/MapViewOfFile.
 /// Memory for the HyperVisor partition is copied to and from the host process
 /// from/into the surrogate process in Sandbox before and after the VCPU is run.
 ///
@@ -145,35 +145,55 @@ impl SurrogateProcessManager {
         &self,
         raw_size: usize,
         raw_source_address: *const c_void,
+        mmap_file_handle: HandleWrapper,
     ) -> Result<SurrogateProcess> {
-        let process_handle: HANDLE = self.process_receiver.recv()?.into();
+        let surrogate_process_handle: HANDLE = self.process_receiver.recv()?.into();
+        let mapping_file_handle: HANDLE = mmap_file_handle.into();
 
-        // allocate memory
+        // Allocate the memory by creating a view over the memory mapped file
+
+        // Use MapViewOfFile2 to map memoy into the surrogate process, the MapViewOfFile2 API is implemented in as an inline function in a windows header file
+        // (see https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-mapviewoffile2#remarks) so we use the same API it uses in the header file here instead of
+        // MapViewOfFile2 which does not exist in the rust crate (see https://github.com/microsoft/windows-rs/issues/2595)
         let allocated_address = unsafe {
-            VirtualAllocEx(
-                process_handle,
+            MapViewOfFileNuma2(
+                mapping_file_handle,
+                surrogate_process_handle,
+                0,
                 Some(raw_source_address),
                 raw_size,
-                MEM_COMMIT | MEM_RESERVE,
-                PAGE_READWRITE,
+                0,
+                PAGE_READWRITE.0,
+                NUMA_NO_PREFERRED_NODE,
             )
         };
-        if allocated_address.is_null() {
+
+        if allocated_address.Value.is_null() {
             log_then_return!(
-                "VirtualAllocEx failed for mem address {:?}",
+                "MapViewOfFileNuma2 failed for mem address {:?}",
                 raw_source_address
             );
         }
 
-        // set up guard page
+        if allocated_address.Value as *const c_void != raw_source_address {
+            log_then_return!(
+                "Address Mismatch Allocated: {:?} Requested: {:?}",
+                allocated_address.Value,
+                raw_source_address
+            );
+        }
+
+        // set up guard pages
+
+        // If the following calls to VirtualProtectEx are changed make sure to update the calls to VirtualProtect in shared_mem.rs
 
         let mut unused_out_old_prot_flags = PAGE_PROTECTION_FLAGS(0);
 
         // the first page of the raw_size is the guard page
         let first_guard_page_start = raw_source_address;
         if let Err(e) = unsafe {
             VirtualProtectEx(
-                process_handle,
+                surrogate_process_handle,
                 first_guard_page_start,
                 PAGE_SIZE_USIZE,
                 PAGE_NOACCESS,
@@ -187,7 +207,7 @@ impl SurrogateProcessManager {
         let last_guard_page_start = unsafe { raw_source_address.add(raw_size - PAGE_SIZE_USIZE) };
         if let Err(e) = unsafe {
             VirtualProtectEx(
-                process_handle,
+                surrogate_process_handle,
                 last_guard_page_start,
                 PAGE_SIZE_USIZE,
                 PAGE_NOACCESS,
@@ -197,7 +217,10 @@ impl SurrogateProcessManager {
             log_then_return!(WindowsAPIError(e.clone()));
         }
 
-        Ok(SurrogateProcess::new(allocated_address, process_handle))
+        Ok(SurrogateProcess::new(
+            allocated_address.Value,
+            surrogate_process_handle,
+        ))
     }
 
     /// Returns a surrogate process to the pool of surrogate processes.
@@ -391,15 +414,17 @@ mod tests {
     use std::thread;
     use std::time::{Duration, Instant};
 
+    use hyperlight_common::mem::PAGE_SIZE_USIZE;
     use rand::{thread_rng, Rng};
     use serial_test::serial;
-    use windows::Win32::Foundation::BOOL;
+    use windows::Win32::Foundation::{CloseHandle, BOOL, HANDLE, INVALID_HANDLE_VALUE};
     use windows::Win32::System::Diagnostics::ToolHelp::{
         CreateToolhelp32Snapshot, Process32First, Process32Next, PROCESSENTRY32, TH32CS_SNAPPROCESS,
     };
     use windows::Win32::System::JobObjects::IsProcessInJob;
     use windows::Win32::System::Memory::{
-        VirtualAlloc, VirtualFree, MEM_COMMIT, MEM_RELEASE, MEM_RESERVE, PAGE_READWRITE,
+        CreateFileMappingA, MapViewOfFile, UnmapViewOfFile, FILE_MAP_ALL_ACCESS, PAGE_READWRITE,
+        SEC_COMMIT,
     };
 
     use super::*;
@@ -423,13 +448,29 @@ mod tests {
                 // surrogate process, make sure we actually got one,
                 // then put it back
                 for p in 0..NUMBER_OF_SURROGATE_PROCESSES {
-                    let allocated_address = unsafe {
-                        VirtualAlloc(None, size, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE)
+                    let dwmaximumsizehigh = 0;
+                    let dwmaximumsizelow = (size & 0xFFFFFFFF) as u32;
+                    let handle = unsafe {
+                        CreateFileMappingA(
+                            INVALID_HANDLE_VALUE, // Causes the page file to be used as the backing store
+                            None,
+                            PAGE_READWRITE | SEC_COMMIT,
+                            dwmaximumsizehigh,
+                            dwmaximumsizelow,
+                            PCSTR::null(),
+                        )
+                        .unwrap()
                     };
+
+                    let addr = unsafe { MapViewOfFile(handle, FILE_MAP_ALL_ACCESS, 0, 0, 0) };
+
                     let timer = Instant::now();
                     let surrogate_process = {
-                        let res = surrogate_process_manager
-                            .get_surrogate_process(size, allocated_address)?;
+                        let res = surrogate_process_manager.get_surrogate_process(
+                            size,
+                            addr.Value,
+                            HandleWrapper::from(handle),
+                        )?;
                         let elapsed = timer.elapsed();
                         // Print out the time it took to get the process if its greater than 150ms (this is just to allow us to see that threads are blocking on the process queue)
                         if (elapsed.as_millis() as u64) > 150 {
@@ -454,10 +495,11 @@ mod tests {
                     // dropping the surrogate process, as we do in the line
                     // below, will return it to the surrogate process manager
                     drop(surrogate_process);
-                    unsafe {
-                        let res = VirtualFree(allocated_address, 0, MEM_RELEASE);
-                        assert!(res.is_ok())
-                    }
+                    let res = unsafe { UnmapViewOfFile(addr) };
+                    assert!(res.is_ok(), "Failed to UnmapViewOfFile: {:?}", res.err());
+
+                    let res = unsafe { CloseHandle(handle) };
+                    assert!(res.is_ok(), "Failed to CloseHandle: {:?}", res.err());
                 }
                 Ok(())
             });
@@ -516,7 +558,11 @@ mod tests {
         let mem = ExclusiveSharedMemory::new(SIZE).unwrap();
 
         let process = mgr
-            .get_surrogate_process(mem.raw_mem_size(), mem.raw_ptr() as *mut c_void)
+            .get_surrogate_process(
+                mem.raw_mem_size(),
+                mem.raw_ptr() as *mut c_void,
+                HandleWrapper::from(mem.get_mmap_file_handle()),
+            )
             .unwrap();
 
         let buffer = vec![0u8; SIZE];