hyperlight-dev
diff --git a/‎Justfile‎
Lines changed: 8 additions & 6 deletions b/‎Justfile‎
Lines changed: 8 additions & 6 deletions
diff --git a/‎src/hyperlight_host/src/hypervisor/surrogate_process.rs‎
Lines changed: 88 additions & 30 deletions b/‎src/hyperlight_host/src/hypervisor/surrogate_process.rs‎
Lines changed: 88 additions & 30 deletions
diff --git a/‎src/hyperlight_host/src/hypervisor/surrogate_process_manager.rs‎
Lines changed: 124 additions & 0 deletions b/‎src/hyperlight_host/src/hypervisor/surrogate_process_manager.rs‎
Lines changed: 124 additions & 0 deletions
@@ -43,10 +43,10 @@ build target=default-target:
 guests: build-and-move-rust-guests build-and-move-c-guests
 
 ensure-cargo-hyperlight:
-    command -v cargo-hyperlight >/dev/null 2>&1 || cargo install --locked cargo-hyperlight
+    {{ if os() == "windows" { "if (-not (Get-Command cargo-hyperlight -ErrorAction SilentlyContinue)) { cargo install --locked cargo-hyperlight }" } else { "command -v cargo-hyperlight >/dev/null 2>&1 || cargo install --locked cargo-hyperlight" } }}
 
 witguest-wit:
-    command -v wasm-tools >/dev/null 2>&1 || cargo install --locked wasm-tools
+    {{ if os() == "windows" { "if (-not (Get-Command wasm-tools -ErrorAction SilentlyContinue)) { cargo install --locked wasm-tools }" } else { "command -v wasm-tools >/dev/null 2>&1 || cargo install --locked wasm-tools" } }}
     cd src/tests/rust_guests/witguest && wasm-tools component wit guest.wit -w -o interface.wasm
     cd src/tests/rust_guests/witguest && wasm-tools component wit two_worlds.wit -w -o twoworlds.wasm
 
@@ -287,19 +287,21 @@ check:
     {{ cargo-cmd }} check -p hyperlight-host --features nanvix-unstable  {{ target-triple-flag }}
     {{ cargo-cmd }} check -p hyperlight-host --features nanvix-unstable,executable_heap  {{ target-triple-flag }}
 
-fmt-check:
-    rustup +nightly component list | grep -q "rustfmt.*installed" || rustup component add rustfmt --toolchain nightly
+fmt-check: (ensure-nightly-fmt)
     cargo +nightly fmt --all -- --check
     cargo +nightly fmt --manifest-path src/tests/rust_guests/simpleguest/Cargo.toml -- --check
     cargo +nightly fmt --manifest-path src/tests/rust_guests/dummyguest/Cargo.toml -- --check
     cargo +nightly fmt --manifest-path src/tests/rust_guests/witguest/Cargo.toml -- --check
     cargo +nightly fmt --manifest-path src/hyperlight_guest_capi/Cargo.toml -- --check
 
+[private]
+ensure-nightly-fmt:
+    {{ if os() == "windows" { "if (-not (rustup +nightly component list | Select-String 'rustfmt.*installed')) { rustup component add rustfmt --toolchain nightly }" } else { "rustup +nightly component list | grep -q 'rustfmt.*installed' || rustup component add rustfmt --toolchain nightly" } }}
+
 check-license-headers:
     ./dev/check-license-headers.sh
 
-fmt-apply:
-    rustup +nightly component list | grep -q "rustfmt.*installed" || rustup component add rustfmt --toolchain nightly
+fmt-apply: (ensure-nightly-fmt)
     cargo +nightly fmt --all
     cargo +nightly fmt --manifest-path src/tests/rust_guests/simpleguest/Cargo.toml
     cargo +nightly fmt --manifest-path src/tests/rust_guests/dummyguest/Cargo.toml
 
@@ -23,19 +23,23 @@ use tracing::{Span, instrument};
 use windows::Win32::Foundation::HANDLE;
 use windows::Win32::System::Memory::{
     MEMORY_MAPPED_VIEW_ADDRESS, MapViewOfFileNuma2, PAGE_NOACCESS, PAGE_PROTECTION_FLAGS,
-    PAGE_READWRITE, UNMAP_VIEW_OF_FILE_FLAGS, UnmapViewOfFile2, VirtualProtectEx,
+    PAGE_READONLY, PAGE_READWRITE, UNMAP_VIEW_OF_FILE_FLAGS, UnmapViewOfFile2, VirtualProtectEx,
 };
 use windows::Win32::System::SystemServices::NUMA_NO_PREFERRED_NODE;
 
 use super::surrogate_process_manager::get_surrogate_process_manager;
 use super::wrappers::HandleWrapper;
 use crate::HyperlightError::WindowsAPIError;
+use crate::mem::memory_region::SurrogateMapping;
 use crate::{Result, log_then_return};
 
 #[derive(Debug)]
 pub(crate) struct HandleMapping {
     pub(crate) use_count: u64,
     pub(crate) surrogate_base: *mut c_void,
+    /// The mapping type used when this entry was first created.
+    /// Used for debug assertions to catch conflicting re-maps.
+    pub(crate) mapping_type: SurrogateMapping,
 }
 
 /// Contains details of a surrogate process to be used by a Sandbox for providing memory to a HyperV VM on Windows.
@@ -57,18 +61,45 @@ impl SurrogateProcess {
         }
     }
 
+    /// Maps a file mapping handle into the surrogate process.
+    ///
+    /// The `mapping` parameter controls the page protection and guard page
+    /// behaviour:
+    /// - [`SurrogateMapping::SandboxMemory`]: uses `PAGE_READWRITE` and sets
+    ///   guard pages (`PAGE_NOACCESS`) on the first and last pages.
+    /// - [`SurrogateMapping::ReadOnlyFile`]: uses `PAGE_READONLY` with no
+    ///   guard pages.
+    ///
+    /// If `host_base` was already mapped, the existing mapping is reused
+    /// and the reference count is incremented (the `mapping` parameter is
+    /// ignored in that case).
     pub(super) fn map(
         &mut self,
         handle: HandleWrapper,
         host_base: usize,
         host_size: usize,
+        mapping: &SurrogateMapping,
     ) -> Result<*mut c_void> {
         match self.mappings.entry(host_base) {
             Entry::Occupied(mut oe) => {
+                if oe.get().mapping_type != *mapping {
+                    tracing::warn!(
+                        "Conflicting SurrogateMapping for host_base {host_base:#x}: \
+                         existing={:?}, requested={:?}",
+                        oe.get().mapping_type,
+                        mapping
+                    );
+                }
                 oe.get_mut().use_count += 1;
                 Ok(oe.get().surrogate_base)
             }
             Entry::Vacant(ve) => {
+                // Derive the page protection from the mapping type
+                let page_protection = match mapping {
+                    SurrogateMapping::SandboxMemory => PAGE_READWRITE,
+                    SurrogateMapping::ReadOnlyFile => PAGE_READONLY,
+                };
+
                 // Use MapViewOfFile2 to map memory 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)
@@ -80,43 +111,60 @@ impl SurrogateProcess {
                         None,
                         host_size,
                         0,
-                        PAGE_READWRITE.0,
+                        page_protection.0,
                         NUMA_NO_PREFERRED_NODE,
                     )
                 };
-                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 = surrogate_base.Value;
-                if let Err(e) = unsafe {
-                    VirtualProtectEx(
-                        self.process_handle.into(),
-                        first_guard_page_start,
-                        PAGE_SIZE_USIZE,
-                        PAGE_NOACCESS,
-                        &mut unused_out_old_prot_flags,
-                    )
-                } {
-                    log_then_return!(WindowsAPIError(e.clone()));
+                if surrogate_base.Value.is_null() {
+                    log_then_return!(
+                        "MapViewOfFileNuma2 failed: {:?}",
+                        std::io::Error::last_os_error()
+                    );
                 }
 
-                // the last page of the raw_size is the guard page
-                let last_guard_page_start =
-                    unsafe { first_guard_page_start.add(host_size - PAGE_SIZE_USIZE) };
-                if let Err(e) = unsafe {
-                    VirtualProtectEx(
-                        self.process_handle.into(),
-                        last_guard_page_start,
-                        PAGE_SIZE_USIZE,
-                        PAGE_NOACCESS,
-                        &mut unused_out_old_prot_flags,
-                    )
-                } {
-                    log_then_return!(WindowsAPIError(e.clone()));
+                // Only set guard pages for SandboxMemory mappings.
+                // File-backed read-only mappings do not need guard pages
+                // because the host does not write to them.
+                if *mapping == SurrogateMapping::SandboxMemory {
+                    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 = surrogate_base.Value;
+                    if let Err(e) = unsafe {
+                        VirtualProtectEx(
+                            self.process_handle.into(),
+                            first_guard_page_start,
+                            PAGE_SIZE_USIZE,
+                            PAGE_NOACCESS,
+                            &mut unused_out_old_prot_flags,
+                        )
+                    } {
+                        self.unmap_helper(surrogate_base.Value);
+                        log_then_return!(WindowsAPIError(e.clone()));
+                    }
+
+                    // the last page of the raw_size is the guard page
+                    let last_guard_page_start =
+                        unsafe { first_guard_page_start.add(host_size - PAGE_SIZE_USIZE) };
+                    if let Err(e) = unsafe {
+                        VirtualProtectEx(
+                            self.process_handle.into(),
+                            last_guard_page_start,
+                            PAGE_SIZE_USIZE,
+                            PAGE_NOACCESS,
+                            &mut unused_out_old_prot_flags,
+                        )
+                    } {
+                        self.unmap_helper(surrogate_base.Value);
+                        log_then_return!(WindowsAPIError(e.clone()));
+                    }
                 }
+
                 ve.insert(HandleMapping {
                     use_count: 1,
                     surrogate_base: surrogate_base.Value,
+                    mapping_type: *mapping,
                 });
                 Ok(surrogate_base.Value)
             }
@@ -126,15 +174,25 @@ impl SurrogateProcess {
     pub(super) fn unmap(&mut self, host_base: usize) {
         match self.mappings.entry(host_base) {
             Entry::Occupied(mut oe) => {
-                oe.get_mut().use_count -= 1;
+                oe.get_mut().use_count = oe.get().use_count.checked_sub(1).unwrap_or_else(|| {
+                    tracing::error!(
+                        "Surrogate unmap ref count underflow for host_base {:#x}",
+                        host_base
+                    );
+                    0
+                });
                 if oe.get().use_count == 0 {
                     let entry = oe.remove();
                     self.unmap_helper(entry.surrogate_base);
                 }
             }
             Entry::Vacant(_) => {
+                tracing::error!(
+                    "Attempted to unmap from surrogate a region at host_base {:#x} that was never mapped",
+                    host_base
+                );
                 #[cfg(debug_assertions)]
-                panic!("Attempted to unmap from surrogate a region that was never mapped")
+                panic!("Attempted to unmap from surrogate a region that was never mapped");
             }
         }
     }
 
@@ -459,6 +459,7 @@ mod tests {
                 HandleWrapper::from(mem.get_mmap_file_handle()),
                 mem.raw_ptr() as usize,
                 mem.raw_mem_size(),
+                &crate::mem::memory_region::SurrogateMapping::SandboxMemory,
             )
             .unwrap();
 
@@ -498,4 +499,127 @@ mod tests {
             assert!(success.is_err());
         }
     }
+
+    /// Tests that [`SurrogateMapping::ReadOnlyFile`] skips guard pages entirely.
+    ///
+    /// When mapping with `ReadOnlyFile`, the first and last pages should be
+    /// accessible (no `PAGE_NOACCESS` guard pages set), unlike `SandboxMemory`
+    /// which marks them as guard pages.
+    #[test]
+    fn readonly_file_mapping_skips_guard_pages() {
+        const SIZE: usize = 4096;
+        let mgr = get_surrogate_process_manager().unwrap();
+        let mem = ExclusiveSharedMemory::new(SIZE).unwrap();
+
+        let mut process = mgr.get_surrogate_process().unwrap();
+        let surrogate_address = process
+            .map(
+                HandleWrapper::from(mem.get_mmap_file_handle()),
+                mem.raw_ptr() as usize,
+                mem.raw_mem_size(),
+                &crate::mem::memory_region::SurrogateMapping::ReadOnlyFile,
+            )
+            .unwrap();
+
+        let buffer = vec![0u8; SIZE];
+        let bytes_read: Option<*mut usize> = None;
+        let process_handle: HANDLE = process.process_handle.into();
+
+        unsafe {
+            // read the first page — should succeed (no guard page for ReadOnlyFile)
+            let success = windows::Win32::System::Diagnostics::Debug::ReadProcessMemory(
+                process_handle,
+                surrogate_address,
+                buffer.as_ptr() as *mut c_void,
+                SIZE,
+                bytes_read,
+            );
+            assert!(
+                success.is_ok(),
+                "First page should be readable with ReadOnlyFile (no guard page)"
+            );
+
+            // read the middle page — should also succeed
+            let success = windows::Win32::System::Diagnostics::Debug::ReadProcessMemory(
+                process_handle,
+                surrogate_address.wrapping_add(SIZE),
+                buffer.as_ptr() as *mut c_void,
+                SIZE,
+                bytes_read,
+            );
+            assert!(
+                success.is_ok(),
+                "Middle page should be readable with ReadOnlyFile"
+            );
+
+            // read the last page — should succeed (no guard page for ReadOnlyFile)
+            let success = windows::Win32::System::Diagnostics::Debug::ReadProcessMemory(
+                process_handle,
+                surrogate_address.wrapping_add(2 * SIZE),
+                buffer.as_ptr() as *mut c_void,
+                SIZE,
+                bytes_read,
+            );
+            assert!(
+                success.is_ok(),
+                "Last page should be readable with ReadOnlyFile (no guard page)"
+            );
+        }
+    }
+
+    /// Tests that the reference counting in [`SurrogateProcess::map`] works
+    /// correctly — repeated maps to the same `host_base` increment the count
+    /// and return the same surrogate address, regardless of the mapping type
+    /// passed on subsequent calls.
+    #[test]
+    fn surrogate_map_ref_counting() {
+        let mgr = get_surrogate_process_manager().unwrap();
+        let mem = ExclusiveSharedMemory::new(4096).unwrap();
+
+        let mut process = mgr.get_surrogate_process().unwrap();
+        let handle = HandleWrapper::from(mem.get_mmap_file_handle());
+        let host_base = mem.raw_ptr() as usize;
+        let host_size = mem.raw_mem_size();
+
+        // First map — creates the mapping
+        let addr1 = process
+            .map(
+                handle,
+                host_base,
+                host_size,
+                &crate::mem::memory_region::SurrogateMapping::SandboxMemory,
+            )
+            .unwrap();
+
+        // Second map — should reuse (ref count incremented)
+        let addr2 = process
+            .map(
+                handle,
+                host_base,
+                host_size,
+                &crate::mem::memory_region::SurrogateMapping::SandboxMemory,
+            )
+            .unwrap();
+
+        assert_eq!(
+            addr1, addr2,
+            "Repeated map should return the same surrogate address"
+        );
+
+        // First unmap — decrements ref count but should NOT actually unmap
+        process.unmap(host_base);
+
+        // The mapping should still be present (ref count was 2, now 1)
+        assert!(
+            process.mappings.contains_key(&host_base),
+            "Mapping should still exist after first unmap (ref count > 0)"
+        );
+
+        // Second unmap — ref count hits 0, actually unmaps
+        process.unmap(host_base);
+        assert!(
+            !process.mappings.contains_key(&host_base),
+            "Mapping should be removed after ref count reaches 0"
+        );
+    }
 }