From 5f016ed45f977fd2f1eb49ff7fde6ba985a2d68b Mon Sep 17 00:00:00 2001 From: Richard Durkee Date: Sat, 11 Apr 2026 17:23:10 -0400 Subject: [PATCH] fix: validate guest address ranges for overlapping regions in map_region Closes #1289 Signed-off-by: Richard Durkee --- .../src/hypervisor/hyperlight_vm/mod.rs | 46 ++++++++ .../src/sandbox/initialized_multi_use.rs | 105 +++++++++++++++++- 2 files changed, 146 insertions(+), 5 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs index 830b856c0..aae4f45e5 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs @@ -243,6 +243,8 @@ pub enum MapRegionError { MapMemory(#[from] MapMemoryError), #[error("Region is not page-aligned (page size: {0:#x})")] NotPageAligned(usize), + #[error("Region [{0:#x}..{1:#x}) overlaps existing region [{2:#x}..{3:#x})")] + Overlapping(usize, usize, usize, usize), } /// Errors that can occur when unmapping a memory region @@ -420,6 +422,50 @@ impl HyperlightVm { return Err(MapRegionError::NotPageAligned(self.page_size)); } + let new_start = region.guest_region.start; + let new_end = region.guest_region.end; + + // Check against existing dynamically mapped regions + for (_, existing) in &self.mmap_regions { + if new_start < existing.guest_region.end && new_end > existing.guest_region.start { + return Err(MapRegionError::Overlapping( + new_start, + new_end, + existing.guest_region.start, + existing.guest_region.end, + )); + } + } + + // Check against the snapshot region + if let Some(ref snapshot) = self.snapshot_memory { + let snap_start = crate::mem::layout::SandboxMemoryLayout::BASE_ADDRESS; + #[cfg(not(unshared_snapshot_mem))] + let snap_end = snap_start + snapshot.guest_mapped_size(); + #[cfg(unshared_snapshot_mem)] + let snap_end = snap_start + snapshot.mem_size(); + if new_start < snap_end && new_end > snap_start { + return Err(MapRegionError::Overlapping( + new_start, new_end, snap_start, snap_end, + )); + } + } + + // Check against the scratch region + if let Some(ref scratch) = self.scratch_memory { + let scratch_start = + hyperlight_common::layout::scratch_base_gpa(scratch.mem_size()) as usize; + let scratch_end = scratch_start + scratch.mem_size(); + if new_start < scratch_end && new_end > scratch_start { + return Err(MapRegionError::Overlapping( + new_start, + new_end, + scratch_start, + scratch_end, + )); + } + } + // Try to reuse a freed slot first, otherwise use next_slot let slot = if let Some(freed_slot) = self.freed_slots.pop() { freed_slot diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 241622cab..af8f82801 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -568,9 +568,10 @@ impl MultiUseSandbox { // writes can be rolled back when necessary. log_then_return!("TODO: Writable mappings not yet supported"); } - // Reset snapshot since we are mutating the sandbox state - self.snapshot = None; + + // Map first so overlaps are rejected before resetting the snapshot unsafe { self.vm.map_region(rgn) }.map_err(HyperlightVmError::MapRegion)?; + self.snapshot = None; self.mem_mgr.mapped_rgns += 1; Ok(()) } @@ -654,13 +655,12 @@ impl MultiUseSandbox { } } - // Reset snapshot since we are mutating the sandbox state - self.snapshot = None; - unsafe { self.vm.map_region(®ion) } .map_err(HyperlightVmError::MapRegion) .map_err(crate::HyperlightError::HyperlightVmError)?; + self.snapshot = None; + let size = prepared.size as u64; // Mark consumed immediately after map_region succeeds. @@ -2588,4 +2588,99 @@ mod tests { } let _ = std::fs::remove_file(&path); } + + #[test] + fn map_region_rejects_overlapping_regions() { + let mut sbox: MultiUseSandbox = { + let path = simple_guest_as_string().unwrap(); + let u_sbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap(); + u_sbox.evolve().unwrap() + }; + + let mem1 = allocate_guest_memory(); + let mem2 = allocate_guest_memory(); + let guest_base: usize = 0x200000000; + let region1 = region_for_memory(&mem1, guest_base, MemoryRegionFlags::READ); + + // First mapping should succeed + unsafe { sbox.map_region(®ion1).unwrap() }; + + // Exact same range should fail + let region2 = region_for_memory(&mem2, guest_base, MemoryRegionFlags::READ); + let err = unsafe { sbox.map_region(®ion2) }.unwrap_err(); + assert!( + format!("{err:?}").contains("Overlapping"), + "Expected Overlapping error, got: {err:?}" + ); + } + + #[test] + fn map_region_rejects_partial_overlap() { + let mut sbox: MultiUseSandbox = { + let path = simple_guest_as_string().unwrap(); + let u_sbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap(); + u_sbox.evolve().unwrap() + }; + + // Use multi-page regions so partial overlap is geometrically possible + let mem1 = page_aligned_memory(&[0xAA; 8192]); // 2 pages + let mem2 = page_aligned_memory(&[0xBB; 8192]); // 2 pages + let guest_base: usize = 0x200000000; + let region1 = region_for_memory(&mem1, guest_base, MemoryRegionFlags::READ); + + unsafe { sbox.map_region(®ion1).unwrap() }; + + // region2 starts one page before region1, overlapping by one page + let overlap_base = guest_base - 0x1000; + let region2 = region_for_memory(&mem2, overlap_base, MemoryRegionFlags::READ); + let err = unsafe { sbox.map_region(®ion2) }.unwrap_err(); + assert!( + format!("{err:?}").contains("verlap"), + "Expected overlap error for partial overlap, got: {err:?}" + ); + } + + #[test] + fn map_region_allows_adjacent_non_overlapping() { + let mut sbox: MultiUseSandbox = { + let path = simple_guest_as_string().unwrap(); + let u_sbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap(); + u_sbox.evolve().unwrap() + }; + + let mem1 = allocate_guest_memory(); + let mem2 = allocate_guest_memory(); + let guest_base: usize = 0x200000000; + let region1 = region_for_memory(&mem1, guest_base, MemoryRegionFlags::READ); + let region_size = mem1.mem_size(); + + unsafe { sbox.map_region(®ion1).unwrap() }; + + // Adjacent region (starts right after the first one ends) should succeed + let adjacent_base = guest_base + region_size; + let region2 = region_for_memory(&mem2, adjacent_base, MemoryRegionFlags::READ); + unsafe { sbox.map_region(®ion2).unwrap() }; + } + + #[test] + fn map_region_rejects_overlap_with_snapshot() { + let mut sbox: MultiUseSandbox = { + let path = simple_guest_as_string().unwrap(); + let u_sbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap(); + u_sbox.evolve().unwrap() + }; + + // Try to map at BASE_ADDRESS (0x1000) which overlaps the snapshot region + let mem = allocate_guest_memory(); + let region = region_for_memory( + &mem, + crate::mem::layout::SandboxMemoryLayout::BASE_ADDRESS, + MemoryRegionFlags::READ, + ); + let err = unsafe { sbox.map_region(®ion) }.unwrap_err(); + assert!( + format!("{err:?}").contains("Overlapping"), + "Expected Overlapping error for snapshot overlap, got: {err:?}" + ); + } }