Map file cow windows by simongdavies · Pull Request #1296 · hyperlight-dev/hyperlight

simongdavies · 2026-03-10T14:54:43Z

This pull request introduces an implementation of map_file_cow on Windows and adds Windows specific and additional tests for file mapping.

Introduce SurrogateMapping enum (SandboxMemory/ReadOnlyFile) to HostRegionBase on Windows. This allows the surrogate process pipeline to distinguish between standard sandbox memory (guard pages, PAGE_READWRITE) and file-backed read-only mappings (no guard pages, PAGE_READONLY). - Add SurrogateMapping enum to memory_region.rs - Add surrogate_mapping field to HostRegionBase - Update Hash, MemoryRegionKind::add impls to include new field - Update host_region_base() in shared_mem.rs to set SandboxMemory - Update crashdump test to include surrogate_mapping field - Add unit tests for SurrogateMapping enum behaviour - Fix Justfile fmt-apply/fmt-check/witguest-wit for Windows (PowerShell) Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

Pass SurrogateMapping through SurrogateProcess::map and WhpVm::map_memory so that file-backed read-only mappings (ReadOnlyFile) use PAGE_READONLY and skip guard pages, while sandbox memory (SandboxMemory) retains the existing PAGE_READWRITE + guard page behaviour. - Add mapping parameter to SurrogateProcess::map - Derive page protection and guard page logic from SurrogateMapping variant - Update WhpVm::map_memory to extract and forward surrogate_mapping - Update existing test call site in surrogate_process_manager - Add test: readonly_file_mapping_skips_guard_pages - Add test: surrogate_map_ref_counting Signed-off-by: Simon Davies <simon.davies@microsoft.com> Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

Introduce OwnedFileMapping to track host-side file mapping resources (MapViewOfFile view + CreateFileMappingW handle) with RAII cleanup. Add file_mappings Vec to MultiUseSandbox (Windows-only, positioned after vm field for correct drop order). - OwnedFileMapping::Drop calls UnmapViewOfFile then CloseHandle - unsafe impl Send + Sync (raw pointer only used during Drop) - Add test: owned_file_mapping_drop_releases_handles Signed-off-by: Simon Davies <simon.davies@microsoft.com> Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

Replace the Windows stub with a full implementation that maps a file into the guest address space via the surrogate process pipeline: CreateFileMappingW -> MapViewOfFile -> SurrogateProcess::map -> WHvMapGpaRange2. Key details: - Opens file read-only, creates PAGE_READONLY mapping - Uses SurrogateMapping::ReadOnlyFile (no guard pages, PAGE_READONLY) - Guest gets READ|EXECUTE access via WHvMapGpaRange2 flags - Tracks host resources in OwnedFileMapping for RAII cleanup - Error path cleans up MapViewOfFile + CloseHandle on failure - CreateFileMappingW uses 0,0 for size (file's actual size, Windows rounds to page boundaries internally) Tests (264 passed, 0 failed): - test_map_file_cow_basic: map file, read from guest, verify content - test_map_file_cow_read_only_enforcement: write triggers violation - test_map_file_cow_poisoned: PoisonedSandbox check + restore - test_map_file_cow_multi_vm_same_file: two sandboxes, same file - test_map_file_cow_multi_vm_threaded: 5 threads, concurrent mapping - test_map_file_cow_cleanup_no_handle_leak: file deletable after drop Signed-off-by: Simon Davies <simon.davies@microsoft.com> Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

Clean up host-side file mapping resources (OwnedFileMapping) when restore() unmaps regions. For each unmapped region, remove the corresponding OwnedFileMapping entry, whose Drop impl calls UnmapViewOfFile + CloseHandle. Also remove the dead_code allow now that guest_base is read. Tests (266 passed, 0 failed): - test_map_file_cow_snapshot_restore: map, snapshot, restore, verify data readable from snapshot memory - test_map_file_cow_snapshot_remapping_cycle: snapshot1 (empty) -> map file -> snapshot2 -> restore1 (unmapped) -> restore2 (folded) Signed-off-by: Simon Davies <simon.davies@microsoft.com> Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

H1: Add null check after MapViewOfFileNuma2 in SurrogateProcess::map H2: Store SurrogateMapping in HandleMapping, add debug_assert_eq on reuse M1: Clean up surrogate mapping on VirtualProtectEx failure (pre-existing) M2: Use checked_sub for ref count, log error on underflow M5: Early return with clear error for empty (0-byte) files L1: Fix incorrect Send/Sync safety comment on OwnedFileMapping L3: Rename _fp/_guest_base to file_path/guest_base L4: Use usize::try_from(file_size) instead of silent truncation N2: Use page_size::get() in test helper instead of magic 4096 N3: Change SurrogateMapping and surrogate_mapping field to pub(crate) N4: Replace low-value derive trait test with meaningful variant test Also: Change MemoryRegionType::Heap to Code for file mappings, add tracing::error in release-mode vacant unmap path. All 266 tests pass, 0 failures. Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

…ate), fix clippy - Add MemoryRegionType::MappedFile variant for map_file_cow regions - Downgrade MemoryRegionType from pub to pub(crate) (not part of public API) - Use MappedFile consistently on both Windows and Linux in map_file_cow - Replace disallowed debug_assert_eq! with tracing::warn! in surrogate_process - Fix Justfile merge conflict Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

Copilot

Pull request overview

This PR implements map_file_cow on Windows using CreateFileMappingW + MapViewOfFile, adds a SurrogateMapping enum to control how regions are projected through the surrogate process, and introduces comprehensive tests for both the new Windows path and cross-platform behavior.

Changes:

Windows implementation of map_file_cow using Win32 file mapping APIs, with RAII cleanup via OwnedFileMapping.
New SurrogateMapping enum in memory_region.rs to distinguish SandboxMemory vs ReadOnlyFile surrogate projections, propagated through the surrogate process pipeline.
Justfile updates for Windows-compatible shell commands (ensure-cargo-hyperlight, ensure-nightly-fmt, witguest-wit).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`initialized_multi_use.rs`	Adds `OwnedFileMapping` RAII struct, Windows implementation of `map_file_cow`, and new integration/unit tests.
`memory_region.rs`	Adds `SurrogateMapping` enum, new `MappedFile` variant to `MemoryRegionType`, and unit tests.
`surrogate_process.rs`	Propagates `SurrogateMapping` through `map`, conditionally applies guard pages, and improves unmap error handling.
`surrogate_process_manager.rs`	Passes `SurrogateMapping` at call sites and adds new surrogate mapping tests.
`shared_mem.rs`	Passes `SurrogateMapping::SandboxMemory` at the existing `HostRegionBase` construction site.
`virtual_machine/whp.rs`	Passes `surrogate_mapping` field to the surrogate `map` call.
`Justfile`	Adds Windows-compatible shell commands for tool checks and formatting.

Comments suppressed due to low confidence (1)

src/hyperlight_host/src/sandbox/initialized_multi_use.rs:1

This test is in surrogate_process_manager.rs (not initialized_multi_use.rs), but the issue is that SIZE is only 4096 bytes (one page), yet the test reads at surrogate_address.wrapping_add(SIZE) (line 545) and surrogate_address.wrapping_add(2 * SIZE) (line 558), which are one and two pages past the end of the mapped region. These reads will access memory that was never mapped, making the "middle page" and "last page" assertions either unreliable or likely to fail/AV. The mapped size should be at least 3 * SIZE to cover the three pages being read, or the read offsets should stay within the mapped range.

/*

src/hyperlight_host/src/sandbox/initialized_multi_use.rs

src/hyperlight_host/src/hypervisor/surrogate_process.rs

src/hyperlight_host/src/hypervisor/surrogate_process_manager.rs

Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

jsturtevant · 2026-03-10T19:03:57Z

src/hyperlight_host/src/mem/memory_region.rs

    pub flags: MemoryRegionFlags,
    /// the type of memory region
-    pub region_type: MemoryRegionType,
+    pub(crate) region_type: MemoryRegionType,


This makes this type not usable outside the crate? which negates these changes I just made: https://github.com/hyperlight-dev/hyperlight/pull/1293/changes for HL-wasm's load_module_by_mapping.

we use map_region currnetly. Is there a different way we are suppose to map these files in hl-wasm?

OK I will clean this up, I only did it because I was adding a new MemoryRegionType member which technically is breaking and I didn't think there was anyone using it

ludfjig · 2026-03-10T21:48:55Z

src/hyperlight_host/src/mem/memory_region.rs

+        fn surrogate_mapping_equality() {
+            assert_eq!(
+                SurrogateMapping::SandboxMemory,
+                SurrogateMapping::SandboxMemory
+            );
+            assert_eq!(
+                SurrogateMapping::ReadOnlyFile,
+                SurrogateMapping::ReadOnlyFile
+            );
+            assert_ne!(
+                SurrogateMapping::SandboxMemory,
+                SurrogateMapping::ReadOnlyFile
+            );
+        }
+
+        #[test]
+        fn surrogate_mapping_variants_are_distinct() {
+            // Verify the two variants are distinct values that can be
+            // used to key different behaviour in the surrogate pipeline
+            let sandbox = SurrogateMapping::SandboxMemory;
+            let readonly = SurrogateMapping::ReadOnlyFile;
+            assert_ne!(sandbox, readonly);
+
+            // Verify Copy semantics work (enum is Copy + Eq)
+            let copy = sandbox;
+            assert_eq!(sandbox, copy);
+        }
+
+        #[test]
+        fn host_region_base_different_surrogate_mapping_not_equal() {
+            let sandbox = make_host_region_base(SurrogateMapping::SandboxMemory);
+            let readonly = make_host_region_base(SurrogateMapping::ReadOnlyFile);
+            assert_ne!(sandbox, readonly);
+        }
+
+        #[test]
+        fn host_region_base_different_surrogate_mapping_different_hash() {
+            let sandbox = make_host_region_base(SurrogateMapping::SandboxMemory);
+            let readonly = make_host_region_base(SurrogateMapping::ReadOnlyFile);
+            assert_ne!(hash_of(&sandbox), hash_of(&readonly));
+        }
+
+        #[test]
+        fn host_region_base_same_surrogate_mapping_equal_and_same_hash() {
+            let a = make_host_region_base(SurrogateMapping::SandboxMemory);
+            let b = make_host_region_base(SurrogateMapping::SandboxMemory);
+            assert_eq!(a, b);
+            assert_eq!(hash_of(&a), hash_of(&b));
+        }


These tests seem unnecessary

ludfjig · 2026-03-10T21:56:23Z

src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs

                region.host_region.start.from_handle,
                region.host_region.start.handle_base,
                region.host_region.start.handle_size,
+                &region.host_region.start.surrogate_mapping,


I think we can get rid of the entire SurrogateMapping from this PR and instead just pass region.flags here. I think this would make this PR a lot smaller

if contains WRITE -> PAGE_READWRITE and guardpages , otherwise PAGE_READONLY and no guardapges

ludfjig · 2026-03-10T22:03:31Z

src/hyperlight_host/src/sandbox/initialized_multi_use.rs

+    /// Tracks host-side file mappings created by [`Self::map_file_cow`] for
+    /// RAII cleanup. Declared **after** `vm` so that Rust's field drop order
+    /// releases hypervisor mappings before closing host-side file handles.
+    #[cfg(target_os = "windows")]
+    file_mappings: Vec<OwnedFileMapping>,


Are you certain this order/comment is necessary? Even if OwnedFileMapping is dropped firest, I suspect the surrgoate process would keep it alive until SuroogateProcess::drop calls UnmapViewOfFile2

I'm also wondering if this is preferable to just doing the unmap in Whpvm (in drop and unmap_region), based on the MemoryRegionType?

ludfjig · 2026-03-10T22:14:00Z

Also, could memmap2 crate be useful here instead of our own OwnedFileMapping? I'm not sure if it has accessors we need for the raw windows calls we need, but worth investigating

simongdavies requested review from danbugs, dblnz, devigned, jprendes, ludfjig, marosset and syntactically as code owners March 10, 2026 14:54

simongdavies added the kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. label Mar 10, 2026

simongdavies requested a review from Copilot March 10, 2026 14:56

simongdavies added 7 commits March 10, 2026 14:57

simongdavies force-pushed the map-file-cow-windows branch from 088dd22 to c5f8f5c Compare March 10, 2026 14:58

Copilot AI reviewed Mar 10, 2026

View reviewed changes

Fix clippy

253fc42

Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

jsturtevant reviewed Mar 10, 2026

View reviewed changes

ludfjig requested changes Mar 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Map file cow windows#1296

Map file cow windows#1296
simongdavies wants to merge 8 commits intohyperlight-dev:mainfrom
simongdavies:map-file-cow-windows

simongdavies commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsturtevant Mar 10, 2026

Uh oh!

simongdavies Mar 10, 2026

Uh oh!

ludfjig Mar 10, 2026

Uh oh!

ludfjig Mar 10, 2026 •

edited

Loading

Uh oh!

ludfjig Mar 10, 2026

Uh oh!

ludfjig Mar 10, 2026

Uh oh!

syntactically Mar 10, 2026

Uh oh!

ludfjig commented Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

simongdavies commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsturtevant Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

simongdavies Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

ludfjig Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

ludfjig Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ludfjig Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

ludfjig Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

syntactically Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

ludfjig commented Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ludfjig Mar 10, 2026 •

edited

Loading