From f0dc67743c0be7cc6082e8003a972a1fc11f044d Mon Sep 17 00:00:00 2001 From: Weiteng Chen Date: Tue, 1 Sep 2026 13:58:02 -0700 Subject: [PATCH 1/3] Fix Windows page reservation permissions --- litebox_platform_windows_userland/src/lib.rs | 61 ++++++++++++++------ 1 file changed, 44 insertions(+), 17 deletions(-) diff --git a/litebox_platform_windows_userland/src/lib.rs b/litebox_platform_windows_userland/src/lib.rs index 2cc274021..47a5ccddf 100644 --- a/litebox_platform_windows_userland/src/lib.rs +++ b/litebox_platform_windows_userland/src/lib.rs @@ -1645,15 +1645,16 @@ impl litebox::platform::PageManagementProvider for Wi debug_assert!(ALIGN.is_multiple_of(self.sys_info.read().unwrap().dwPageSize as usize)); debug_assert_alignment!(suggested_range, ALIGN); - // A helper closure to reserve and commit memory in one go. + // A helper closure to reserve memory and commit it when accessible + // permissions are requested. // // Note that MEM_RESERVE requires the base address to be aligned to system allocation granularity, // while MEM_COMMIT only requires page-aligned address. // // To ensure future MEM_COMMIT calls on sub-ranges succeed, we always reserve the entire aligned range // (i.e., MEM_RESERVE size is also made aligned to system allocation granularity). - let reserve_and_commit = |r: core::ops::Range, - flags: Win32_Memory::PAGE_PROTECTION_FLAGS| + let reserve_and_maybe_commit = |r: core::ops::Range, + flags: Win32_Memory::PAGE_PROTECTION_FLAGS| -> *mut c_void { let aligned_start_addr = self.round_down_to_granu(r.start); let aligned_end_addr = self.round_up_to_granu(r.end); @@ -1670,6 +1671,8 @@ impl litebox::platform::PageManagementProvider for Wi }; if ptr.is_null() { core::ptr::null_mut() + } else if flags == Win32_Memory::PAGE_NOACCESS { + ptr } else { unsafe { VirtualAlloc2( @@ -1745,6 +1748,9 @@ impl litebox::platform::PageManagementProvider for Wi unsafe { GetLastError() } ); } + if initial_permissions.is_empty() { + return Ok(true); + } let ptr = unsafe { VirtualAlloc2( GetCurrentProcess(), @@ -1760,8 +1766,10 @@ impl litebox::platform::PageManagementProvider for Wi } // In case the region is free, we need to reserve and commit it. Win32_Memory::MEM_FREE => { - let ptr = - reserve_and_commit(r.clone(), prot_flags(initial_permissions)); + let ptr = reserve_and_maybe_commit( + r.clone(), + prot_flags(initial_permissions), + ); !ptr.is_null() } _ => unimplemented!( @@ -1782,7 +1790,7 @@ impl litebox::platform::PageManagementProvider for Wi } debug_assert!(base_addr.is_null()); - let ptr = reserve_and_commit(0..size, prot_flags(initial_permissions)); + let ptr = reserve_and_maybe_commit(0..size, prot_flags(initial_permissions)); assert!( !ptr.is_null(), "VirtualAlloc2(RESERVE|COMMIT size=0x{:x}) failed: {}", @@ -1831,17 +1839,36 @@ impl litebox::platform::PageManagementProvider for Wi process_memory_range_by_regions( range, |r, state| -> Result { - debug_assert_eq!( - state, - Win32_Memory::MEM_COMMIT, - "Trying to change permissions on a non-committed region: {:p}-{:p}", - r.start as *mut c_void, - r.end as *mut c_void - ); - let mut old_protect: u32 = 0; - Ok(unsafe { - VirtualProtect(r.start as *mut c_void, r.len(), flags, &raw mut old_protect) - } != 0) + match state { + Win32_Memory::MEM_RESERVE if flags == Win32_Memory::PAGE_NOACCESS => Ok(true), + Win32_Memory::MEM_RESERVE => Ok(!unsafe { + VirtualAlloc2( + GetCurrentProcess(), + r.start as *mut c_void, + r.len(), + Win32_Memory::MEM_COMMIT, + flags, + core::ptr::null_mut(), + 0, + ) + } + .is_null()), + Win32_Memory::MEM_COMMIT => { + let mut old_protect: u32 = 0; + Ok(unsafe { + VirtualProtect( + r.start as *mut c_void, + r.len(), + flags, + &raw mut old_protect, + ) + } != 0) + } + _ => panic!( + "Trying to change permissions on an unallocated region: {:p}-{:p}", + r.start as *mut c_void, r.end as *mut c_void + ), + } }, ) .expect("update_permissions failed"); From fa4a7e5ae637fbee953e295e15d0be1457645548 Mon Sep 17 00:00:00 2001 From: Weiteng Chen Date: Tue, 1 Sep 2026 16:03:23 -0700 Subject: [PATCH 2/3] add comment --- litebox_platform_windows_userland/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/litebox_platform_windows_userland/src/lib.rs b/litebox_platform_windows_userland/src/lib.rs index 47a5ccddf..7cd936828 100644 --- a/litebox_platform_windows_userland/src/lib.rs +++ b/litebox_platform_windows_userland/src/lib.rs @@ -1653,6 +1653,10 @@ impl litebox::platform::PageManagementProvider for Wi // // To ensure future MEM_COMMIT calls on sub-ranges succeed, we always reserve the entire aligned range // (i.e., MEM_RESERVE size is also made aligned to system allocation granularity). + // + // TODO: Empty permissions cannot distinguish a reserve-only mapping from Windows + // MEM_COMMIT | PAGE_NOACCESS. Represent commitment separately from permissions before + // supporting committed inaccessible pages. let reserve_and_maybe_commit = |r: core::ops::Range, flags: Win32_Memory::PAGE_PROTECTION_FLAGS| -> *mut c_void { From 31c3eb5b8fa83adab720490a7ecd0e622e95c7a3 Mon Sep 17 00:00:00 2001 From: Weiteng Chen Date: Fri, 4 Sep 2026 16:35:12 -0700 Subject: [PATCH 3/3] Fix reserve-only Windows mappings --- Cargo.lock | 5 +- litebox_platform_windows_userland/Cargo.toml | 1 + litebox_platform_windows_userland/src/lib.rs | 216 +++++++++++++++++-- 3 files changed, 205 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 49f222b72..0ed0800af 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1666,6 +1666,7 @@ dependencies = [ "getrandom 0.3.4", "litebox", "litebox_common_linux", + "rangemap", "windows-sys 0.60.2", "zerocopy", ] @@ -2441,9 +2442,9 @@ dependencies = [ [[package]] name = "rangemap" -version = "1.6.0" +version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f93e7e49bb0bf967717f7bd674458b3d6b0c5f48ec7e3038166026a69fc22223" +checksum = "f60fcc7d6849342eff22c4350c8b9a989ee8ceabc4b481253e8946b9fe83d684" [[package]] name = "raw-cpuid" diff --git a/litebox_platform_windows_userland/Cargo.toml b/litebox_platform_windows_userland/Cargo.toml index ed637f9a1..8daeb9dbf 100644 --- a/litebox_platform_windows_userland/Cargo.toml +++ b/litebox_platform_windows_userland/Cargo.toml @@ -7,6 +7,7 @@ edition = "2024" getrandom = "0.3.4" litebox = { path = "../litebox/", version = "0.1.0" } litebox_common_linux = { path = "../litebox_common_linux", version = "0.1.0" } +rangemap = "1.5.1" windows-sys = { version = "0.60.2", features = [ "Win32_System_Console", diff --git a/litebox_platform_windows_userland/src/lib.rs b/litebox_platform_windows_userland/src/lib.rs index 7cd936828..5c650fc27 100644 --- a/litebox_platform_windows_userland/src/lib.rs +++ b/litebox_platform_windows_userland/src/lib.rs @@ -53,6 +53,7 @@ thread_local! { /// traits. pub struct WindowsUserland { reserved_pages: alloc::vec::Vec>, + reserve_only_mappings: std::sync::RwLock>, sys_info: std::sync::RwLock, } @@ -242,6 +243,7 @@ impl WindowsUserland { let platform = Self { reserved_pages, + reserve_only_mappings: std::sync::RwLock::new(rangemap::RangeSet::new()), sys_info: std::sync::RwLock::new(sys_info), }; @@ -1644,6 +1646,7 @@ impl litebox::platform::PageManagementProvider for Wi ) -> Result, AllocationError> { debug_assert!(ALIGN.is_multiple_of(self.sys_info.read().unwrap().dwPageSize as usize)); debug_assert_alignment!(suggested_range, ALIGN); + let mut reserve_only_mappings = self.reserve_only_mappings.write().unwrap(); // A helper closure to reserve memory and commit it when accessible // permissions are requested. @@ -1653,10 +1656,6 @@ impl litebox::platform::PageManagementProvider for Wi // // To ensure future MEM_COMMIT calls on sub-ranges succeed, we always reserve the entire aligned range // (i.e., MEM_RESERVE size is also made aligned to system allocation granularity). - // - // TODO: Empty permissions cannot distinguish a reserve-only mapping from Windows - // MEM_COMMIT | PAGE_NOACCESS. Represent commitment separately from permissions before - // supporting committed inaccessible pages. let reserve_and_maybe_commit = |r: core::ops::Range, flags: Win32_Memory::PAGE_PROTECTION_FLAGS| -> *mut c_void { @@ -1707,6 +1706,7 @@ impl litebox::platform::PageManagementProvider for Wi assert!(suggested_range.end <= >:: TASK_ADDR_MAX); + let has_reserve_only_mapping = reserve_only_mappings.overlaps(&suggested_range); let has_committed_page = process_memory_range_by_regions(suggested_range.clone(), |_r, state| { if state == Win32_Memory::MEM_COMMIT { @@ -1716,17 +1716,19 @@ impl litebox::platform::PageManagementProvider for Wi } }) .is_err(); - if has_committed_page && fixed_address_behavior == FixedAddressBehavior::Hint { - // If any page in the suggested range is already committed, and the caller - // did not request a fixed address, we ask the OS to allocate a new region. + if (has_reserve_only_mapping || has_committed_page) + && fixed_address_behavior == FixedAddressBehavior::Hint + { + // If any page in the suggested range is already mapped, and the caller did not + // request a fixed address, we ask the OS to allocate a new region. base_addr = core::ptr::null_mut(); - } else if has_committed_page + } else if (has_reserve_only_mapping || has_committed_page) && fixed_address_behavior == FixedAddressBehavior::NoReplace { return Err(AllocationError::AddressInUse); } else { process_memory_range_by_regions( - suggested_range, + suggested_range.clone(), |r, state| -> Result { let ok = match state { // In case the region is already reserved, we just need to commit it. @@ -1781,14 +1783,19 @@ impl litebox::platform::PageManagementProvider for Wi state ), }; - // Prefetch the memory range if requested - if ok && populate_pages_immediately { + // Prefetch only committed, accessible ranges. + if ok && populate_pages_immediately && !initial_permissions.is_empty() { do_prefetch_on_range(r.start, r.len()); } Ok(ok) }, ) .unwrap(); + if initial_permissions.is_empty() { + reserve_only_mappings.insert(suggested_range); + } else { + reserve_only_mappings.remove(suggested_range); + } return Ok(UserMutPtr::from_ptr(base_addr.cast())); } } @@ -1802,10 +1809,13 @@ impl litebox::platform::PageManagementProvider for Wi std::io::Error::last_os_error() ); - // Prefetch the memory range if requested - if populate_pages_immediately { + // Prefetch only committed, accessible ranges. + if populate_pages_immediately && !initial_permissions.is_empty() { do_prefetch_on_range(ptr as usize, size); } + if initial_permissions.is_empty() { + reserve_only_mappings.insert(ptr as usize..ptr as usize + size); + } Ok(UserMutPtr::from_ptr(ptr.cast::())) } @@ -1814,8 +1824,9 @@ impl litebox::platform::PageManagementProvider for Wi range: core::ops::Range, ) -> Result<(), litebox::platform::page_mgmt::DeallocationError> { debug_assert_alignment!(range, ALIGN); + let mut reserve_only_mappings = self.reserve_only_mappings.write().unwrap(); process_memory_range_by_regions( - range, + range.clone(), |r, state| -> Result { debug_assert_ne!( state, @@ -1830,6 +1841,7 @@ impl litebox::platform::PageManagementProvider for Wi }, ) .expect("deallocate_pages failed"); + reserve_only_mappings.remove(range); Ok(()) } @@ -1839,9 +1851,10 @@ impl litebox::platform::PageManagementProvider for Wi new_permissions: MemoryRegionPermissions, ) -> Result<(), litebox::platform::page_mgmt::PermissionUpdateError> { debug_assert_alignment!(range, ALIGN); + let mut reserve_only_mappings = self.reserve_only_mappings.write().unwrap(); let flags = prot_flags(new_permissions); process_memory_range_by_regions( - range, + range.clone(), |r, state| -> Result { match state { Win32_Memory::MEM_RESERVE if flags == Win32_Memory::PAGE_NOACCESS => Ok(true), @@ -1876,6 +1889,9 @@ impl litebox::platform::PageManagementProvider for Wi }, ) .expect("update_permissions failed"); + if !new_permissions.is_empty() { + reserve_only_mappings.remove(range); + } Ok(()) } @@ -2115,15 +2131,18 @@ impl litebox::mm::linux::VmemPageFaultHandler for WindowsUserland { #[cfg(test)] mod tests { use core::sync::atomic::AtomicU32; + use std::os::raw::c_void; use std::thread::sleep; use crate::WindowsUserland; + use crate::do_query_on_region; use crate::process_memory_range_by_regions; use litebox::platform::PageManagementProvider; use litebox::platform::RawConstPointer; use litebox::platform::RawMutex; use litebox::platform::page_mgmt::FixedAddressBehavior; use litebox::platform::page_mgmt::MemoryRegionPermissions; + use windows_sys::Win32::System::Memory as Win32_Memory; #[test] fn test_raw_mutex() { @@ -2190,6 +2209,14 @@ mod tests { ) .unwrap() .as_usize(); + // Accessible mappings must not be tracked as reserve-only. + assert!( + !platform + .reserve_only_mappings + .read() + .unwrap() + .overlaps(&(addr..addr + 0x1000)) + ); assert_eq!( collect_regions(addr..addr + system_allocation_granularity), vec![ @@ -2249,5 +2276,164 @@ mod tests { .unwrap() .as_usize(); assert_ne!(addr3, addr + 0x4000); + + // Find a free allocation-granularity-sized region so this allocation + // deterministically exercises the MEM_FREE path. + let free_addr = { + let mut address = + >::TASK_ADDR_MIN as *mut c_void; + loop { + let mut mbi = Win32_Memory::MEMORY_BASIC_INFORMATION::default(); + do_query_on_region(&mut mbi, address); + if mbi.State == Win32_Memory::MEM_FREE + && mbi.RegionSize >= system_allocation_granularity + { + break mbi.BaseAddress as usize; + } + address = (mbi.BaseAddress as usize + mbi.RegionSize) as *mut c_void; + } + }; + // MAP_POPULATE with no permissions should reserve the hinted range + // without committing or prefetching it. + let suggested_populated_inaccessible_addr = + >::allocate_pages( + platform, + free_addr..free_addr + 0x1000, + MemoryRegionPermissions::empty(), + false, + true, + FixedAddressBehavior::Hint, + ) + .unwrap() + .as_usize(); + assert_eq!(suggested_populated_inaccessible_addr, free_addr); + assert_eq!( + collect_regions(free_addr..free_addr + 0x1000), + vec![( + free_addr..free_addr + 0x1000, + windows_sys::Win32::System::Memory::MEM_RESERVE + )] + ); + + // The same behavior applies when Windows chooses the base address. + let populated_inaccessible_addr = + >::allocate_pages( + platform, + 0..0x1000, + MemoryRegionPermissions::empty(), + false, + true, + FixedAddressBehavior::Hint, + ) + .unwrap() + .as_usize(); + assert_eq!( + collect_regions(populated_inaccessible_addr..populated_inaccessible_addr + 0x1000), + vec![( + populated_inaccessible_addr..populated_inaccessible_addr + 0x1000, + windows_sys::Win32::System::Memory::MEM_RESERVE + )] + ); + // Making the range accessible commits it and clears reserve-only metadata. + unsafe { + >::update_permissions( + platform, + populated_inaccessible_addr..populated_inaccessible_addr + 0x1000, + MemoryRegionPermissions::WRITE, + ) + } + .unwrap(); + assert!( + !platform + .reserve_only_mappings + .read() + .unwrap() + .overlaps(&(populated_inaccessible_addr..populated_inaccessible_addr + 0x1000)) + ); + assert_eq!( + collect_regions(populated_inaccessible_addr..populated_inaccessible_addr + 0x1000), + vec![( + populated_inaccessible_addr..populated_inaccessible_addr + 0x1000, + windows_sys::Win32::System::Memory::MEM_COMMIT + )] + ); + + // A reserve-only mapping is live even though Windows reports it as MEM_RESERVE. + let inaccessible_addr = >::allocate_pages( + platform, + 0..0x1000, + MemoryRegionPermissions::empty(), + false, + false, + FixedAddressBehavior::Hint, + ) + .unwrap() + .as_usize(); + assert!( + platform + .reserve_only_mappings + .read() + .unwrap() + .overlaps(&(inaccessible_addr..inaccessible_addr + 0x1000)) + ); + assert_eq!( + collect_regions(inaccessible_addr..inaccessible_addr + 0x1000), + vec![( + inaccessible_addr..inaccessible_addr + 0x1000, + windows_sys::Win32::System::Memory::MEM_RESERVE + )] + ); + + // Hint must relocate around a live reserve-only mapping, while + // MAP_FIXED_NOREPLACE must reject the collision. + let relocated_addr = >::allocate_pages( + platform, + inaccessible_addr..inaccessible_addr + 0x1000, + MemoryRegionPermissions::empty(), + false, + false, + FixedAddressBehavior::Hint, + ) + .unwrap() + .as_usize(); + assert_ne!(relocated_addr, inaccessible_addr); + assert!(matches!( + >::allocate_pages( + platform, + inaccessible_addr..inaccessible_addr + 0x1000, + MemoryRegionPermissions::empty(), + false, + false, + FixedAddressBehavior::NoReplace, + ), + Err(litebox::platform::page_mgmt::AllocationError::AddressInUse) + )); + + // Deallocation clears the metadata, making the address reusable. + unsafe { + >::deallocate_pages( + platform, + inaccessible_addr..inaccessible_addr + 0x1000, + ) + } + .unwrap(); + assert!( + !platform + .reserve_only_mappings + .read() + .unwrap() + .overlaps(&(inaccessible_addr..inaccessible_addr + 0x1000)) + ); + let reused_addr = >::allocate_pages( + platform, + inaccessible_addr..inaccessible_addr + 0x1000, + MemoryRegionPermissions::empty(), + false, + false, + FixedAddressBehavior::Hint, + ) + .unwrap() + .as_usize(); + assert_eq!(reused_addr, inaccessible_addr); } }