Skip to content

Commit ecffd72

Browse files
committed
Update memory allocation in Windows
Replace use of VirtualAlloc/VirtualFree with CreateFileMapping/MapViewOFile/UnmapViewOfFile/CloseHandle to allow memry to be mapped into surrogate processes without any copying Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
1 parent 1b0c76b commit ecffd72

File tree

1 file changed

+90
-14
lines changed

1 file changed

+90
-14
lines changed

src/hyperlight_host/src/mem/shared_mem.rs

Lines changed: 90 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,17 @@ use std::sync::{Arc, RwLock};
2323
use hyperlight_common::mem::PAGE_SIZE_USIZE;
2424
use tracing::{instrument, Span};
2525
#[cfg(target_os = "windows")]
26-
use windows::Win32::System::Memory::{VirtualAlloc, MEM_COMMIT, PAGE_EXECUTE_READWRITE};
26+
use windows::core::PCSTR;
27+
#[cfg(target_os = "windows")]
28+
use windows::Win32::Foundation::{CloseHandle, HANDLE, INVALID_HANDLE_VALUE};
29+
#[cfg(target_os = "windows")]
30+
use windows::Win32::System::Memory::{
31+
CreateFileMappingA, MapViewOfFile, UnmapViewOfFile, VirtualProtect, FILE_MAP_ALL_ACCESS,
32+
MEMORY_MAPPED_VIEW_ADDRESS, PAGE_EXECUTE_READWRITE, PAGE_PROTECTION_FLAGS,
33+
};
2734

35+
#[cfg(target_os = "windows")]
36+
use crate::hypervisor::wrappers::HandleWrapper;
2837
#[cfg(target_os = "windows")]
2938
use crate::HyperlightError::{MemoryRequestTooBig, WindowsAPIError};
3039
use crate::{log_then_return, new_error, Result};
@@ -82,6 +91,8 @@ macro_rules! generate_writer {
8291
pub struct HostMapping {
8392
ptr: *mut u8,
8493
size: usize,
94+
#[cfg(target_os = "windows")]
95+
handle: HandleWrapper,
8596
}
8697

8798
impl Drop for HostMapping {
@@ -95,10 +106,19 @@ impl Drop for HostMapping {
95106
}
96107
#[cfg(target_os = "windows")]
97108
fn drop(&mut self) {
98-
use windows::Win32::System::Memory::{VirtualFree, MEM_DECOMMIT};
109+
let mem_mapped_address = MEMORY_MAPPED_VIEW_ADDRESS {
110+
Value: self.ptr as *mut c_void,
111+
};
112+
if let Err(e) = unsafe { UnmapViewOfFile(mem_mapped_address) } {
113+
tracing::error!(
114+
"Failed to drop HostMapping (UnmapViewOfFile failed): {:?}",
115+
e
116+
);
117+
}
99118

100-
if let Err(e) = unsafe { VirtualFree(self.ptr as *mut c_void, self.size, MEM_DECOMMIT) } {
101-
tracing::error!("Failed to free shared memory (VirtualFree failed): {:?}", e);
119+
let file_handle: HANDLE = self.handle.into();
120+
if let Err(e) = unsafe { CloseHandle(file_handle) } {
121+
tracing::error!("Failed to drop HostMapping (CloseHandle failed): {:?}", e);
102122
}
103123
}
104124
}
@@ -360,7 +380,9 @@ impl ExclusiveSharedMemory {
360380
#[cfg(target_os = "windows")]
361381
#[instrument(skip_all, parent = Span::current(), level= "Trace")]
362382
pub fn new(min_size_bytes: usize) -> Result<Self> {
363-
use windows::Win32::System::Memory::PAGE_READWRITE;
383+
#[cfg(inprocess)]
384+
use windows::Win32::System::Memory::FILE_MAP_EXECUTE;
385+
use windows::Win32::System::Memory::{PAGE_NOACCESS, PAGE_PROTECTION_FLAGS};
364386

365387
use crate::HyperlightError::MemoryAllocationFailed;
366388

@@ -385,16 +407,65 @@ impl ExclusiveSharedMemory {
385407
return Err(MemoryRequestTooBig(total_size, isize::MAX as usize));
386408
}
387409

388-
// allocate the memory
410+
let mut dwmaximumsizehigh = 0;
411+
let mut dwmaximumsizelow = 0;
412+
413+
if std::mem::size_of::<usize>() == 8 {
414+
dwmaximumsizehigh = (total_size >> 32) as u32;
415+
dwmaximumsizelow = (total_size & 0xFFFFFFFF) as u32;
416+
}
417+
418+
// Allocate the memory use CreateFileMapping instead of VirtualAlloc
419+
// This allows us to map the memory into the surrogate process using MapViewOfFile2
420+
let handle = unsafe {
421+
CreateFileMappingA(
422+
INVALID_HANDLE_VALUE,
423+
None,
424+
PAGE_EXECUTE_READWRITE,
425+
dwmaximumsizehigh,
426+
dwmaximumsizelow,
427+
PCSTR::null(),
428+
)?
429+
};
430+
431+
#[cfg(inprocess)]
389432
let addr =
390-
unsafe { VirtualAlloc(Some(null_mut()), total_size, MEM_COMMIT, PAGE_READWRITE) };
391-
if addr.is_null() {
433+
unsafe { MapViewOfFile(handle, FILE_MAP_ALL_ACCESS | FILE_MAP_EXECUTE, 0, 0, 0) };
434+
#[cfg(not(inprocess))]
435+
let addr = unsafe { MapViewOfFile(handle, FILE_MAP_ALL_ACCESS, 0, 0, 0) };
436+
if addr.Value.is_null() {
392437
log_then_return!(MemoryAllocationFailed(
393438
Error::last_os_error().raw_os_error()
394439
));
395440
}
396441

397-
// TODO protect the guard pages
442+
// Set the first and last pages to be guard pages
443+
444+
let mut unused_out_old_prot_flags = PAGE_PROTECTION_FLAGS(0);
445+
446+
let first_guard_page_start = addr.Value;
447+
if let Err(e) = unsafe {
448+
VirtualProtect(
449+
first_guard_page_start,
450+
PAGE_SIZE_USIZE,
451+
PAGE_NOACCESS,
452+
&mut unused_out_old_prot_flags,
453+
)
454+
} {
455+
log_then_return!(WindowsAPIError(e.clone()));
456+
}
457+
458+
let last_guard_page_start = unsafe { addr.Value.add(total_size - PAGE_SIZE_USIZE) };
459+
if let Err(e) = unsafe {
460+
VirtualProtect(
461+
last_guard_page_start,
462+
PAGE_SIZE_USIZE,
463+
PAGE_NOACCESS,
464+
&mut unused_out_old_prot_flags,
465+
)
466+
} {
467+
log_then_return!(WindowsAPIError(e.clone()));
468+
}
398469

399470
Ok(Self {
400471
// HostMapping is only non-Send/Sync because raw pointers
@@ -407,17 +478,16 @@ impl ExclusiveSharedMemory {
407478
// is not pointless as the lint suggests.
408479
#[allow(clippy::arc_with_non_send_sync)]
409480
region: Arc::new(HostMapping {
410-
ptr: addr as *mut u8,
481+
ptr: addr.Value as *mut u8,
411482
size: total_size,
483+
handle: HandleWrapper::from(handle),
412484
}),
413485
})
414486
}
415487

416488
pub(super) fn make_memory_executable(&self) -> Result<()> {
417489
#[cfg(target_os = "windows")]
418490
{
419-
use windows::Win32::System::Memory::{VirtualProtect, PAGE_PROTECTION_FLAGS};
420-
421491
let mut _old_flags = PAGE_PROTECTION_FLAGS::default();
422492
if let Err(e) = unsafe {
423493
VirtualProtect(
@@ -463,7 +533,7 @@ impl ExclusiveSharedMemory {
463533
/// must be properly aligned.
464534
///
465535
/// The rules on validity are still somewhat unspecified, but we
466-
/// assume that the result of our calls to mmap/VirtualAlloc may
536+
/// assume that the result of our calls to mmap/CreateFileMappings may
467537
/// be considered a single "allocated object". The use of
468538
/// non-atomic accesses is alright from a Safe Rust standpoint,
469539
/// because SharedMemoryBuilder is not Sync.
@@ -473,7 +543,7 @@ impl ExclusiveSharedMemory {
473543
/// Again, the exact provenance restrictions on what is
474544
/// considered to be initialized values are unclear, but we make
475545
/// sure to use mmap(MAP_ANONYMOUS) and
476-
/// VirtualAlloc(MEM_COMMIT), so the pages in question are
546+
/// CreateFileMapping(SEC_COMMIT), so the pages in question are
477547
/// zero-initialized, which we hope counts for u8.
478548
/// - The memory referenced by the returned slice must not be
479549
/// accessed through any other pointer (not derived from the
@@ -576,6 +646,12 @@ impl ExclusiveSharedMemory {
576646
},
577647
)
578648
}
649+
650+
/// Gets the file handle of the shared memory region for this Sandbox
651+
#[cfg(target_os = "windows")]
652+
pub fn get_mmap_file_handle(&self) -> HandleWrapper {
653+
self.region.handle
654+
}
579655
}
580656

581657
/// A trait that abstracts over the particular kind of SharedMemory,

0 commit comments

Comments
 (0)