From 9825d9f424150e77826fe632154c2af66871ca12 Mon Sep 17 00:00:00 2001 From: Sangho Lee Date: Mon, 6 Jul 2026 20:19:17 +0000 Subject: [PATCH 1/5] prevent module/kexec validation TOCTOU/confused-deputy --- dev_tests/src/ratchet.rs | 2 +- litebox_common_linux/src/vmap.rs | 14 + litebox_platform_lvbs/src/lib.rs | 86 +++- litebox_platform_lvbs/src/mshv/error.rs | 4 + litebox_platform_lvbs/src/mshv/mod.rs | 9 + litebox_platform_lvbs/src/mshv/ringbuffer.rs | 13 +- litebox_platform_lvbs/src/mshv/vsm.rs | 456 +++++++++++++++---- 7 files changed, 484 insertions(+), 100 deletions(-) diff --git a/dev_tests/src/ratchet.rs b/dev_tests/src/ratchet.rs index 0e0b17f1e9..68288b8e39 100644 --- a/dev_tests/src/ratchet.rs +++ b/dev_tests/src/ratchet.rs @@ -37,7 +37,7 @@ fn ratchet_globals() -> Result<()> { ("litebox/", 9), ("litebox_platform_linux_kernel/", 6), ("litebox_platform_linux_userland/", 5), - ("litebox_platform_lvbs/", 23), + ("litebox_platform_lvbs/", 24), ("litebox_platform_multiplex/", 1), ("litebox_platform_windows_userland/", 8), ("litebox_runner_lvbs/", 5), diff --git a/litebox_common_linux/src/vmap.rs b/litebox_common_linux/src/vmap.rs index 30d161fcd5..9a082e21d2 100644 --- a/litebox_common_linux/src/vmap.rs +++ b/litebox_common_linux/src/vmap.rs @@ -42,6 +42,20 @@ pub unsafe trait VmapManager { Err(PhysPointerError::UnsupportedOperation) } + /// Map pages without the platform's mutable-access checks for trusted callers. Ordinary + /// [`Self::vmap`] may delegate here after performing its checks. The default is deny. + /// + /// # Safety + /// + /// This method has the same raw-mapping requirements as [`Self::vmap`]. + unsafe fn vmap_privileged( + &self, + _pages: &PhysPageAddrArray, + _perms: PhysPageMapPermissions, + ) -> Result { + Err(PhysPointerError::UnsupportedOperation) + } + /// Unmap the previously mapped virtually contiguous addresses ([`Self::MapInfo`]). /// /// This function is analogous to Linux kernel's `vunmap()`. diff --git a/litebox_platform_lvbs/src/lib.rs b/litebox_platform_lvbs/src/lib.rs index 6799b9494c..6cb9e60d84 100644 --- a/litebox_platform_lvbs/src/lib.rs +++ b/litebox_platform_lvbs/src/lib.rs @@ -46,15 +46,21 @@ pub mod mshv; pub mod syscall_entry; -/// Mapping info returned by [`LinuxKernel`]'s [`VmapManager::vmap`]. +/// Mapping info returned by LVBS vmap paths. Ordinary writable mappings retain their +/// `protected_frames` guard here until unmap. pub struct LvbsPhysPageMapInfo { base: *mut u8, size: usize, + protected_frames_guard: Option>>, } impl LvbsPhysPageMapInfo { fn new(base: *mut u8, size: usize) -> Self { - Self { base, size } + Self { + base, + size, + protected_frames_guard: None, + } } } @@ -447,6 +453,9 @@ type UserMutPtr = /// Type-level marker for the VTL0 physical-pointer provider. pub enum Vmap {} +/// Type-level marker for MSHV operations authorized to modify protected VTL0 frames. +pub(crate) struct PrivilegedVmap; + impl GlobalVmapManager for Vmap { type Manager = crate::host::LvbsLinuxKernel; fn manager() -> &'static Self::Manager { @@ -454,10 +463,53 @@ impl GlobalVmapManager for Vmap { } } -pub type Vtl0PhysConstPtr = - litebox_common_linux::physical_pointers::PhysConstPtr; -pub type Vtl0PhysMutPtr = - litebox_common_linux::physical_pointers::PhysMutPtr; +impl GlobalVmapManager for PrivilegedVmap { + type Manager = PrivilegedVmap; + fn manager() -> &'static Self::Manager { + &PrivilegedVmap + } +} + +unsafe impl VmapManager for PrivilegedVmap { + type MapInfo = LvbsPhysPageMapInfo; + + unsafe fn vmap( + &self, + pages: &PhysPageAddrArray, + perms: PhysPageMapPermissions, + ) -> Result { + // SAFETY: callers uphold the raw mapping contract. This provider is used only + // for independently authorized HEKI patch and ring-buffer writes. + unsafe { crate::platform_low().vmap_privileged(pages, perms) } + } + + unsafe fn vunmap( + &self, + map_info: Self::MapInfo, + ) -> Result<(), (PhysPointerError, Self::MapInfo)> { + // SAFETY: `map_info` came from the same LVBS platform mapper and has no outstanding uses + // beyond the physical-pointer guard that is dropping it. + unsafe { + >::vunmap( + crate::platform_low(), + map_info, + ) + } + } + + fn validate_unowned(&self, pages: &PhysPageAddrArray) -> Result<(), PhysPointerError> { + crate::platform_low().validate_unowned(pages) + } + + unsafe fn protect( + &self, + pages: &PhysPageAddrArray, + perms: PhysPageMapPermissions, + ) -> Result<(), PhysPointerError> { + // SAFETY: callers uphold `VmapManager::protect`; this forwards unchanged to LVBS. + unsafe { crate::platform_low().protect(pages, perms) } + } +} impl RawPointerProvider for LinuxKernel { type RawConstPointer = UserConstPtr; @@ -1128,6 +1180,28 @@ unsafe impl VmapManager for Linu &self, pages: &PhysPageAddrArray, perms: PhysPageMapPermissions, + ) -> Result { + let protected_frames = if perms.contains(PhysPageMapPermissions::WRITE) { + // This shared guard spans map/copy/unmap. It permits concurrent foreign-memory writes + // but does not support re-entry into a VTL protection change. + let guard = crate::mshv::vsm::protected_frames().read(); + crate::mshv::vsm::validate_mutable_vtl0_pages(&guard, pages)?; + Some(guard) + } else { + None + }; + // SAFETY: ordinary writable mappings were checked against protected and in-flight frames; + // the guard is retained through map, access, and unmap. `vmap_privileged` provides the + // shared raw mapping implementation. + let mut map_info = unsafe { self.vmap_privileged(pages, perms)? }; + map_info.protected_frames_guard = protected_frames; + Ok(map_info) + } + + unsafe fn vmap_privileged( + &self, + pages: &PhysPageAddrArray, + perms: PhysPageMapPermissions, ) -> Result { if pages.is_empty() { return Err(PhysPointerError::InvalidPhysicalAddress(0)); diff --git a/litebox_platform_lvbs/src/mshv/error.rs b/litebox_platform_lvbs/src/mshv/error.rs index bd40d50b28..a9614205c2 100644 --- a/litebox_platform_lvbs/src/mshv/error.rs +++ b/litebox_platform_lvbs/src/mshv/error.rs @@ -83,6 +83,9 @@ pub enum VsmError { #[error("invalid module token")] ModuleTokenInvalid, + #[error("physical frames overlap already-protected or reserved memory")] + ProtectedFrameOverlap, + // Kernel Symbol Table Errors #[error("no kernel symbol table found")] KernelSymbolTableNotFound, @@ -212,6 +215,7 @@ impl From for Errno { | VsmError::ModuleMemoryTypeInvalid | VsmError::ModuleRelocationInvalid | VsmError::ModuleTokenInvalid + | VsmError::ProtectedFrameOverlap | VsmError::KexecTypeInvalid | VsmError::KexecImageSegmentsInvalid | VsmError::SymbolTableEmpty diff --git a/litebox_platform_lvbs/src/mshv/mod.rs b/litebox_platform_lvbs/src/mshv/mod.rs index 3d5fa740e3..0f1601bb03 100644 --- a/litebox_platform_lvbs/src/mshv/mod.rs +++ b/litebox_platform_lvbs/src/mshv/mod.rs @@ -15,6 +15,15 @@ pub mod vsm_intercept; pub mod vtl1_mem_layout; pub mod vtl_switch; +type Vtl0PhysConstPtr = + litebox_common_linux::physical_pointers::PhysConstPtr; + +/// Mutable VTL0 pointer reserved for the HEKI text patching (validated) and log ring +/// buffer (fixed address). It bypasses `protected_frames` rejection and locking. +/// DO NOT USE IT for other VTL0 destinations which could enable confused-deputy writes. +type PrivilegedVtl0PhysMutPtr = + litebox_common_linux::physical_pointers::PhysMutPtr; + use crate::arch::MAX_CORES; use crate::mshv::vtl1_mem_layout::PAGE_SIZE; use modular_bitfield::prelude::*; diff --git a/litebox_platform_lvbs/src/mshv/ringbuffer.rs b/litebox_platform_lvbs/src/mshv/ringbuffer.rs index 355ffc3d90..573bde849c 100644 --- a/litebox_platform_lvbs/src/mshv/ringbuffer.rs +++ b/litebox_platform_lvbs/src/mshv/ringbuffer.rs @@ -3,7 +3,7 @@ //! RingBuffer implementation and functions -use crate::Vtl0PhysMutPtr; +use super::PrivilegedVtl0PhysMutPtr; use core::fmt; use litebox::mm::linux::PAGE_SIZE; use litebox::utils::TruncateExt; @@ -96,7 +96,7 @@ fn write_fast(rb_pa: PhysAddr, size: usize, write_offset: usize, buf: &[u8]) -> span.push(addr); } - let Ok(ptr) = Vtl0PhysMutPtr::::new(&span, in_page_offset) else { + let Ok(ptr) = PrivilegedVtl0PhysMutPtr::::new(&span, in_page_offset) else { return advance_offset(size, write_offset, buf.len()); }; let _ = ptr.write_slice_at_offset(0, buf); @@ -108,9 +108,12 @@ fn write_fast(rb_pa: PhysAddr, size: usize, write_offset: usize, buf: &[u8]) -> /// after attempting the write. fn write_slow(rb_pa: PhysAddr, size: usize, write_offset: usize, buf: &[u8]) -> usize { let write_slice = |pa: PhysAddr, slice: &[u8]| -> bool { - Vtl0PhysMutPtr::::with_contiguous_pages(pa.as_u64().trunc(), slice.len()) - .and_then(|ptr| ptr.write_slice_at_offset(0, slice)) - .is_ok() + PrivilegedVtl0PhysMutPtr::::with_contiguous_pages( + pa.as_u64().trunc(), + slice.len(), + ) + .and_then(|ptr| ptr.write_slice_at_offset(0, slice)) + .is_ok() }; if buf.len() >= size { diff --git a/litebox_platform_lvbs/src/mshv/vsm.rs b/litebox_platform_lvbs/src/mshv/vsm.rs index 29cc0442fa..fdd7f1b3d5 100644 --- a/litebox_platform_lvbs/src/mshv/vsm.rs +++ b/litebox_platform_lvbs/src/mshv/vsm.rs @@ -6,8 +6,9 @@ #[cfg(debug_assertions)] use crate::mshv::mem_integrity::parse_modinfo; use crate::mshv::ringbuffer::set_ringbuffer; +use crate::mshv::{PrivilegedVtl0PhysMutPtr, Vtl0PhysConstPtr}; use crate::{ - Vtl0PhysConstPtr, Vtl0PhysMutPtr, debug_serial_println, + debug_serial_println, host::{ PRK_LEN, bootparam::get_vtl1_memory_info, @@ -41,6 +42,7 @@ use crate::{ vtl1_mem_layout::{PAGE_SHIFT, PAGE_SIZE}, }, }; + use alloc::{boxed::Box, ffi::CString, string::String, vec::Vec}; use core::{ mem, @@ -50,7 +52,8 @@ use core::{ use hashbrown::{HashMap, HashSet}; use litebox::utils::TruncateExt; use litebox_common_linux::{errno::Errno, vmap::PhysPageAddr}; -use spin::Once; +use rangemap::RangeSet; +use spin::{Once, rwlock::RwLock as SpinRwLock}; use thiserror::Error; use x86_64::{ PhysAddr, VirtAddr, @@ -465,6 +468,132 @@ pub fn mshv_vsm_load_kdata(pa: u64, nranges: u64) -> Result { // TODO: save blocklist hashes } +/// RAII reservation over VTL0 physical frames, shared by module load and kexec validation. +/// On drop without `commit`, release of every reserved range is attempted. +struct FrameReservation { + owned_ranges: Vec>, + owned_frames: RangeSet, + committed: bool, +} + +#[derive(Debug, PartialEq, Eq)] +enum ReservationStatus { + New, + AlreadyOwned, +} + +fn classify_reservation( + owned: &RangeSet, + protected: &RangeSet, + range: Range, +) -> Result { + if owned.gaps(&range).next().is_none() { + return Ok(ReservationStatus::AlreadyOwned); + } + if owned.overlaps(&range) || protected.overlaps(&range) { + return Err(VsmError::ProtectedFrameOverlap); + } + Ok(ReservationStatus::New) +} + +impl FrameReservation { + fn new() -> Self { + Self { + owned_ranges: Vec::new(), + owned_frames: RangeSet::new(), + committed: false, + } + } + + /// Reserve `frames`: reject any overlap with VTL1 working memory, a frame already in the registry + /// (a non-writable frame the reservation would downgrade or another live reservation's claim). + /// A range already covered by this reservation is idempotent. Call again to add other frames. + /// + /// The claim is inserted into `protected_frames` atomically under its lock together with the + /// overlap check. Each claimed frame is a "maybe-non-writable" entry that is later resolved by + /// [`apply_vtl_protection`]: promotion to RX/RO keeps it, promotion to RW (or release on failure) + /// removes it. On rejection, this call's inserts are rolled back. The returned status for each + /// input identifies whether that occurrence added a new claim. + fn reserve( + &mut self, + frames: impl IntoIterator>, + ) -> Result, VsmError> { + let vtl1 = crate::platform_low().vtl1_phys_frame_range(); + let vtl1_start = vtl1.start.start_address().as_u64(); + let vtl1_end = vtl1.end.start_address().as_u64(); + + let mut protected = protected_frames().write(); + // Idempotence applies only to ranges owned before this call. + let owned_before = self.owned_frames.clone(); + let mut seen = RangeSet::new(); + let mut statuses = Vec::new(); + // Frames this call adds, so a later overlap rolls back only them. + let rollback_from = self.owned_ranges.len(); + for phys_frame_range in frames { + let start = phys_frame_range.start.start_address().as_u64(); + let end = phys_frame_range.end.start_address().as_u64(); + if start >= end { + statuses.push(ReservationStatus::AlreadyOwned); + continue; + } + // `protected` holds existing non-writable frames, this reservation's earlier claims, and + // any other concurrent reservation's in-flight claims, so this one check rejects + // cross-object downgrade, intra-overlap, and concurrent double-claim alike. + let range = start..end; + let status = if seen.overlaps(&range) || (start < vtl1_end && vtl1_start < end) { + Err(VsmError::ProtectedFrameOverlap) + } else { + classify_reservation(&owned_before, &protected, range.clone()) + }; + seen.insert(range.clone()); + let status = match status { + Ok(status) => status, + Err(error) => { + for undo in &self.owned_ranges[rollback_from..] { + let range = + undo.start.start_address().as_u64()..undo.end.start_address().as_u64(); + protected.remove(range.clone()); + self.owned_frames.remove(range); + } + self.owned_ranges.truncate(rollback_from); + return Err(error); + } + }; + if status == ReservationStatus::AlreadyOwned { + statuses.push(status); + continue; + } + protected.insert(range.clone()); + self.owned_frames.insert(range); + self.owned_ranges.push(phys_frame_range); + statuses.push(status); + } + Ok(statuses) + } + + /// Mark the reserved frames as committed; drop becomes a no-op. + fn commit(&mut self) { + self.committed = true; + } +} + +impl Drop for FrameReservation { + fn drop(&mut self) { + if self.committed { + return; + } + // Rollback: attempt to release every reserved range back to VTL0. Drop cannot report a + // release failure, so debug builds assert it instead. + for &phys_frame_range in &self.owned_ranges { + let result = release_physical_memory_range(phys_frame_range); + debug_assert!( + result.is_ok(), + "Failed to release reserved VTL0 memory protection" + ); + } + } +} + /// VSM function for validating a guest kernel module and applying specified protection to its memory ranges after validation. /// `pa` and `nranges` specify a memory area containing the information about the kernel module to validate or protect. /// `flags` controls the validation process (unused for now). @@ -528,6 +657,19 @@ pub fn mshv_vsm_validate_guest_module(pa: u64, nranges: u64, _flags: u64) -> Res } } + // Reject overlap and reserve this module's frames. Any overlap is adversarial (legitimate + // module frames are never shared). The reservation attempts to release the frames on failure. + let mut frame_guard = FrameReservation::new(); + let _ = frame_guard.reserve(module_memory_metadata.iter().map(|r| r.phys_frame_range))?; + + // Write-protect the frames that will become non-writable (RX/RO) BEFORE copying, so VTL0 cannot + // change them between the copy/validation and the final permission promotion. + for mod_mem_range in &module_memory_metadata { + if !mod_mem_type_to_mem_attr(mod_mem_range.mod_mem_type).contains(MemAttr::MEM_ATTR_WRITE) { + protect_physical_memory_range(mod_mem_range.phys_frame_range, MemAttr::MEM_ATTR_READ)?; + } + } + module_as_elf .write_bytes_from_heki_range() .map_err(|_| VsmError::Vtl0CopyFailed)?; @@ -559,7 +701,15 @@ pub fn mshv_vsm_validate_guest_module(pa: u64, nranges: u64, _flags: u64) -> Res return Err(VsmError::ModuleRelocationInvalid); } - // pre-computed patch data for a module + // once a module is verified and validated, change the permission of its memory ranges based on their types + for mod_mem_range in &module_memory_metadata { + protect_physical_memory_range( + mod_mem_range.phys_frame_range, + mod_mem_type_to_mem_attr(mod_mem_range.mod_mem_type), + )?; + } + + // Commit the module's pre-computed patch data (transactional). if !patch_info_for_module.is_empty() { let patch_info_buf = &patch_info_for_module[..]; crate::platform_low() @@ -569,14 +719,8 @@ pub fn mshv_vsm_validate_guest_module(pa: u64, nranges: u64, _flags: u64) -> Res .map_err(|_| VsmError::Vtl0CopyFailed)?; } - // once a module is verified and validated, change the permission of its memory ranges based on their types - for mod_mem_range in &module_memory_metadata { - protect_physical_memory_range( - mod_mem_range.phys_frame_range, - mod_mem_type_to_mem_attr(mod_mem_range.mod_mem_type), - )?; - } - + // Fully validated and committed: disarm the guard and register the module. + frame_guard.commit(); // register the module memory in the global map and obtain a unique token for it let token = crate::platform_low() .vtl0_kernel_info @@ -600,32 +744,49 @@ pub fn mshv_vsm_free_guest_module_init(token: i64) -> Result { return Err(VsmError::ModuleTokenInvalid); } + let mut result: Result<(), VsmError> = Ok(()); if let Some(entry) = crate::platform_low() .vtl0_kernel_info .module_memory_metadata .iter_entry(token) { for mod_mem_range in entry.iter_mem_ranges() { - match mod_mem_range.mod_mem_type { + let range_result = match mod_mem_range.mod_mem_type { ModMemType::InitText | ModMemType::InitData | ModMemType::InitRoData => { - // make this memory range readable, writable, and non-executable after initialization to let the VTL0 kernel free it - protect_physical_memory_range( - mod_mem_range.phys_frame_range, - MemAttr::MEM_ATTR_READ | MemAttr::MEM_ATTR_WRITE, - )?; + release_physical_memory_range(mod_mem_range.phys_frame_range) } ModMemType::RoAfterInit => { // make this memory range read-only after initialization protect_physical_memory_range( mod_mem_range.phys_frame_range, MemAttr::MEM_ATTR_READ, - )?; + ) } - _ => {} + _ => Ok(()), + }; + if range_result.is_err() { + result = range_result; + break; } } } + // Drop the init ranges from the module's metadata regardless of failures. This is intentional + // since hypercalls shouldn't fail and avoiding double release is more important. + let freed_init_patch_targets = crate::platform_low() + .vtl0_kernel_info + .module_memory_metadata + .remove_init_ranges(token); + // Drop the precomputed patches targeting those freed init frames so a stale init patch cannot + // later be applied to recycled frames. + if !freed_init_patch_targets.is_empty() { + crate::platform_low() + .vtl0_kernel_info + .precomputed_patches + .remove_patch_data(&freed_init_patch_targets); + } + + result?; Ok(0) } @@ -647,12 +808,8 @@ pub fn mshv_vsm_unload_guest_module(token: i64) -> Result { .module_memory_metadata .iter_entry(token) { - // make the memory ranges of a module readable, writable, and non-executable to let the VTL0 kernel unload the module for mod_mem_range in entry.iter_mem_ranges() { - protect_physical_memory_range( - mod_mem_range.phys_frame_range, - MemAttr::MEM_ATTR_READ | MemAttr::MEM_ATTR_WRITE, - )?; + release_physical_memory_range(mod_mem_range.phys_frame_range)?; } } @@ -708,10 +865,7 @@ pub fn mshv_vsm_kexec_validate(pa: u64, nranges: u64, crash: u64) -> Result Result Result KEXEC_SEGMENT_MAX as u64 { return Err(VsmError::KexecImageSegmentsInvalid); } + let mut segment_ranges = Vec::new(); for i in 0..usize::try_from(kimage.nr_segments).unwrap_or(0) { let va = kimage.segment[i].buf; let pa = kimage.segment[i].mem; if let Some(epa) = pa.checked_add(kimage.segment[i].memsz) { - kexec_memory_metadata.insert_memory_range(KexecMemoryRange::new(va, pa, epa)?); + segment_ranges.push(KexecMemoryRange::new(va, pa, epa)?); } else { return Err(VsmError::KexecSegmentRangeInvalid); } } + let reservation_statuses = + frame_guard.reserve(segment_ranges.iter().map(|r| r.phys_frame_range))?; + for (segment_range, status) in segment_ranges.into_iter().zip(reservation_statuses) { + if status == ReservationStatus::New { + protect_physical_memory_range( + segment_range.phys_frame_range, + MemAttr::MEM_ATTR_READ, + )?; + kexec_memory_metadata.insert_memory_range(segment_range); + } + } } - // write protect the kexec memory ranges first to avoid the race condition during verification - for kexec_mem_range in &kexec_memory_metadata { - protect_physical_memory_range(kexec_mem_range.phys_frame_range, MemAttr::MEM_ATTR_READ)?; - } - - // verify the signature of kexec blob - let kexec_kernel_blob_data = &kexec_kernel_blob[..]; - - if let Err(result) = verify_kernel_pe_signature(kexec_kernel_blob_data, certs) { - for kexec_mem_range in &kexec_memory_metadata { - protect_physical_memory_range( - kexec_mem_range.phys_frame_range, - MemAttr::MEM_ATTR_READ | MemAttr::MEM_ATTR_WRITE, - )?; - } + // verify the signature of the kexec blob + if let Err(result) = verify_kernel_pe_signature(&kexec_kernel_blob[..], certs) { return Err(VsmError::SignatureVerificationFailed(result)); } + frame_guard.commit(); // register the protected kexec memory ranges to support possible invalidation in the future kexec_metadata_ref.register_memory(kexec_memory_metadata); @@ -877,8 +1039,8 @@ fn copy_heki_patch_from_vtl0(patch_pa_0: u64, patch_pa_1: u64) -> Result Result<(), VsmError> { // `HekiPatch::is_valid` already validated both physical addresses. let heki_patch_pa_0 = PhysAddr::new(heki_patch.pa[0]); @@ -900,7 +1062,8 @@ fn apply_vtl0_text_patch(heki_patch: HekiPatch) -> Result<(), VsmError> { <= heki_patch_pa_0.align_down(Size4KiB::SIZE).as_u64() + Size4KiB::SIZE, "patch crosses page boundary but pa_1 is null" ); - let ptr = Vtl0PhysMutPtr::::with_contiguous_pages( + // The patch was validated against VTL1's precomputed HEKI patch data. + let ptr = PrivilegedVtl0PhysMutPtr::::with_contiguous_pages( heki_patch_pa_0.as_u64().trunc(), patch.len(), ) @@ -916,7 +1079,8 @@ fn apply_vtl0_text_patch(heki_patch: HekiPatch) -> Result<(), VsmError> { PhysPageAddr::::new(heki_patch_pa_1.as_u64().trunc()) .ok_or(VsmError::Vtl0CopyFailed)?, ]; - let ptr = Vtl0PhysMutPtr::::new( + // The patch was validated against VTL1's precomputed HEKI patch data. + let ptr = PrivilegedVtl0PhysMutPtr::::new( &pages, (heki_patch_pa_0 - heki_patch_pa_0.align_down(Size4KiB::SIZE)).trunc(), ) @@ -928,6 +1092,10 @@ fn apply_vtl0_text_patch(heki_patch: HekiPatch) -> Result<(), VsmError> { } fn mshv_vsm_allocate_ringbuffer_memory(phys_addr: u64, size: usize) -> Result { + if crate::platform_low().vtl0_kernel_info.check_end_of_boot() { + return Err(VsmError::OperationAfterEndOfBoot("ring buffer allocation")); + } + let end = phys_addr .checked_add(size as u64) .ok_or(VsmError::IntegerOverflow) @@ -1333,6 +1501,42 @@ impl ModuleMemoryMetadataMap { map.remove(&key).is_some() } + /// Drop a module's freed init ranges from its metadata after [`mshv_vsm_free_guest_module_init`] + /// hands them back to VTL0, so a later free/unload does not re-release them. Return the patch + /// targets that fell within the freed init frames to drop their precomputed patches. + fn remove_init_ranges(&self, key: i64) -> Vec { + let is_init = |t| { + matches!( + t, + ModMemType::InitText | ModMemType::InitData | ModMemType::InitRoData + ) + }; + let mut map = self.inner.lock(); + let Some(metadata) = map.get_mut(&key) else { + return Vec::new(); + }; + let init_ranges: Vec> = metadata + .ranges + .iter() + .filter(|r| is_init(r.mod_mem_type)) + .map(|r| r.phys_frame_range) + .collect(); + metadata.ranges.retain(|r| !is_init(r.mod_mem_type)); + let mut freed_patch_targets = Vec::new(); + metadata.patch_targets.retain(|&pa| { + let freed = init_ranges + .iter() + .any(|fr| fr.start.start_address() <= pa && fr.end.start_address() > pa); + if freed { + freed_patch_targets.push(pa); + false + } else { + true + } + }); + freed_patch_targets + } + /// Return the addresses of patch targets belonging to a module identified by `key` pub(crate) fn get_patch_targets(&self, key: i64) -> Option> { let guard = self.inner.lock(); @@ -1411,9 +1615,62 @@ fn copy_heki_pages_from_vtl0(pa: u64, nranges: u64) -> Option> { Some(heki_pages) } +/// Registry of VTL0 frames that are non-writable to VTL0 or reserved by an in-flight protection +/// operation. Module and kexec reservations use exclusive access to update it. Ordinary writable +/// LVBS vmaps retain shared access through map, access, and unmap, allowing concurrent foreign-memory +/// writes while excluding protection changes. Privileged HEKI and ring-buffer mappings bypass it. +/// This registry is needed because Hyper-V provides no query for current VTL protection. +pub(crate) fn protected_frames() -> &'static SpinRwLock> { + static PROTECTED_FRAMES: Once>> = Once::new(); + PROTECTED_FRAMES.call_once(|| SpinRwLock::new(RangeSet::new())) +} + +pub(crate) fn validate_mutable_vtl0_pages( + protected: &RangeSet, + pages: &litebox_common_linux::vmap::PhysPageAddrArray, +) -> Result<(), litebox_common_linux::vmap::PhysPointerError> { + for page in pages { + let start = page.as_usize() as u64; + let end = start + .checked_add(ALIGN as u64) + .ok_or(litebox_common_linux::vmap::PhysPointerError::Overflow)?; + if protected.overlaps(&(start..end)) { + return Err( + litebox_common_linux::vmap::PhysPointerError::InvalidPhysicalAddress( + page.as_usize(), + ), + ); + } + } + Ok(()) +} + +/// Update the locked registry after a protection change. Holding the exclusive guard across the +/// hypercall and update synchronizes transitions with ordinary writable mappings; privileged HEKI +/// and ring-buffer mappings intentionally bypass it. +fn record_frame_protection( + set: &mut RangeSet, + phys_frame_range: PhysFrameRange, + protect: bool, +) { + let start = phys_frame_range.start.start_address().as_u64(); + let end = phys_frame_range.end.start_address().as_u64(); + if start >= end { + return; + } + if protect { + set.insert(start..end); + } else { + set.remove(start..end); + } +} + /// Protects a VTL0 physical memory range from potentially compromised VTL0 by restricting its /// access permissions using VTL protection mask (e.g., kernel code integrity). /// +/// The frame is recorded as protected iff the resulting VTL0 permission is non-writable (RO/RX). +/// See [`protected_frames`]. +/// /// If the requested range overlaps with VTL1 working memory, the VTL1 portion is silently /// skipped and only the remaining VTL0 portions are protected. If the range falls entirely /// within VTL1, this function returns `Ok(())` without issuing a hypercall. @@ -1424,22 +1681,47 @@ pub(crate) fn protect_physical_memory_range( phys_frame_range: PhysFrameRange, mem_attr: MemAttr, ) -> Result<(), VsmError> { + apply_vtl_protection(phys_frame_range, mem_attr) +} + +/// Hands a VTL0 physical memory range back to VTL0 (read-write), which also clears its protection. +fn release_physical_memory_range( + phys_frame_range: PhysFrameRange, +) -> Result<(), VsmError> { + apply_vtl_protection( + phys_frame_range, + MemAttr::MEM_ATTR_READ | MemAttr::MEM_ATTR_WRITE, + ) +} + +/// Shared implementation of [`protect_physical_memory_range`] / [`release_physical_memory_range`]. +/// Applies `mem_attr` to the VTL0 portions of `phys_frame_range` (skipping any VTL1 working memory) +/// and updates the registry from the result: a frame non-writable by VTL0 (RO/RX) is recorded as +/// protected; a VTL0-writable frame is removed. +fn apply_vtl_protection( + phys_frame_range: PhysFrameRange, + mem_attr: MemAttr, +) -> Result<(), VsmError> { + let protect = !mem_attr.contains(MemAttr::MEM_ATTR_WRITE); let vtl1_range = crate::platform_low().vtl1_phys_frame_range(); + // Range fully within VTL1 — nothing to protect for VTL0. + if phys_frame_range.start >= vtl1_range.start && phys_frame_range.end <= vtl1_range.end { + return Ok(()); + } + // Fast path: no overlap with VTL1 — protect the entire range directly. let overlaps_vtl1 = phys_frame_range.start < vtl1_range.end && vtl1_range.start < phys_frame_range.end; + let mut protected = protected_frames().write(); + if !overlaps_vtl1 { let pa = phys_frame_range.start.start_address().as_u64(); let num_pages = phys_frame_range.count() as u64; hv_modify_vtl_protection_mask(pa, num_pages, mem_attr_to_hv_page_prot_flags(mem_attr)) .map_err(VsmError::HypercallFailed)?; - return Ok(()); - } - - // Range fully within VTL1 — nothing to protect for VTL0. - if phys_frame_range.start >= vtl1_range.start && phys_frame_range.end <= vtl1_range.end { + record_frame_protection(&mut protected, phys_frame_range, protect); return Ok(()); } @@ -1464,6 +1746,7 @@ pub(crate) fn protect_physical_memory_range( let num_pages = sub_range.count() as u64; hv_modify_vtl_protection_mask(pa, num_pages, mem_attr_to_hv_page_prot_flags(mem_attr)) .map_err(VsmError::HypercallFailed)?; + record_frame_protection(&mut protected, sub_range, protect); } Ok(()) } @@ -1899,7 +2182,8 @@ impl PatchDataMap { if patch_info_buf.len() < core::mem::size_of::() { return Err(PatchDataMapError::InvalidHekiPatchInfo); } - let mut inner = self.inner.write(); + + let mut parsed: Vec<(PhysAddr, HekiPatch)> = Vec::new(); // the buffer looks like below: // [`HekiPatchInfo`, [`HekiPatch`, ...], `HekiPatchInfo`, [`HekiPatch`, ...], ...] @@ -1935,54 +2219,50 @@ impl PatchDataMap { let patch_target_pa_0 = PhysAddr::new(patch.pa[0]); let patch_target_pa_1 = PhysAddr::new(patch.pa[1]); - if let Some(ref mut mod_mem_meta) = module_memory_metadata { - for mod_mem_range in &**mod_mem_meta { + // The second page is used as an additional key when a patch straddles two physical + // pages (see `validate_text_poke_bp_batch`). + let straddles_second_page = !patch_target_pa_1.is_null() + && patch_target_pa_0 + .as_u64() + .checked_add(1) + .and_then(|next| PhysAddr::try_new(next).ok()) + .is_some_and(|next| next.is_aligned(Size4KiB::SIZE)); + + if let Some(ref mod_mem_meta) = module_memory_metadata { + // Only accept patch targets within the module's executable ranges. + let in_executable_range = mod_mem_meta.iter().any(|mod_mem_range| { let in_range = |pa: PhysAddr| { mod_mem_range.phys_frame_range.start.start_address() <= pa && mod_mem_range.phys_frame_range.end.start_address() > pa }; - if matches!( + matches!( mod_mem_range.mod_mem_type, ModMemType::Text | ModMemType::InitText ) && in_range(patch_target_pa_0) && (patch_target_pa_1.is_null() || in_range(patch_target_pa_1)) - { - mod_mem_meta.insert_patch_target(patch_target_pa_0); - inner.insert(patch_target_pa_0, patch); - - // If the first byte of a patch target is in the first (physical) page while the remaining bytes - // are in the second page, we use the second page as an additional key for the patch to deal with - // Step 2 of `text_poke_bp_batch` where we only know the second to last bytes of the patch such - // that cannot know the address of the first page. Details are in `validate_text_poke_bp_batch`. - if !patch_target_pa_1.is_null() - && patch_target_pa_0 - .as_u64() - .checked_add(1) - .and_then(|next| PhysAddr::try_new(next).ok()) - .is_some_and(|next| next.is_aligned(Size4KiB::SIZE)) - { - mod_mem_meta.insert_patch_target(patch_target_pa_1); - inner.insert(patch_target_pa_1, patch); - } - break; - } - } - } else { - inner.insert(patch_target_pa_0, patch); - if !patch_target_pa_1.is_null() - && patch_target_pa_0 - .as_u64() - .checked_add(1) - .and_then(|next| PhysAddr::try_new(next).ok()) - .is_some_and(|next| next.is_aligned(Size4KiB::SIZE)) - { - inner.insert(patch_target_pa_1, patch); + }); + if !in_executable_range { + continue; } } + + parsed.push((patch_target_pa_0, patch)); + if straddles_second_page { + parsed.push((patch_target_pa_1, patch)); + } } index = patches_end; } + // Commit every parsed patch and record its targets for later unload cleanup. + let mut inner = self.inner.write(); + for (target, patch) in parsed { + inner.insert(target, patch); + if let Some(ref mut mod_mem_meta) = module_memory_metadata { + mod_mem_meta.insert_patch_target(target); + } + } + Ok(()) } } From 47815825525f929d58bb290427ab2f985ccfe625 Mon Sep 17 00:00:00 2001 From: Sangho Lee Date: Thu, 16 Jul 2026 17:07:13 +0000 Subject: [PATCH 2/5] make PrivilegedVmap module-private --- litebox_platform_lvbs/src/lib.rs | 51 ----------------------- litebox_platform_lvbs/src/mshv/mod.rs | 58 ++++++++++++++++++++++++++- litebox_platform_lvbs/src/mshv/vsm.rs | 3 +- 3 files changed, 58 insertions(+), 54 deletions(-) diff --git a/litebox_platform_lvbs/src/lib.rs b/litebox_platform_lvbs/src/lib.rs index 6cb9e60d84..494a803250 100644 --- a/litebox_platform_lvbs/src/lib.rs +++ b/litebox_platform_lvbs/src/lib.rs @@ -453,9 +453,6 @@ type UserMutPtr = /// Type-level marker for the VTL0 physical-pointer provider. pub enum Vmap {} -/// Type-level marker for MSHV operations authorized to modify protected VTL0 frames. -pub(crate) struct PrivilegedVmap; - impl GlobalVmapManager for Vmap { type Manager = crate::host::LvbsLinuxKernel; fn manager() -> &'static Self::Manager { @@ -463,54 +460,6 @@ impl GlobalVmapManager for Vmap { } } -impl GlobalVmapManager for PrivilegedVmap { - type Manager = PrivilegedVmap; - fn manager() -> &'static Self::Manager { - &PrivilegedVmap - } -} - -unsafe impl VmapManager for PrivilegedVmap { - type MapInfo = LvbsPhysPageMapInfo; - - unsafe fn vmap( - &self, - pages: &PhysPageAddrArray, - perms: PhysPageMapPermissions, - ) -> Result { - // SAFETY: callers uphold the raw mapping contract. This provider is used only - // for independently authorized HEKI patch and ring-buffer writes. - unsafe { crate::platform_low().vmap_privileged(pages, perms) } - } - - unsafe fn vunmap( - &self, - map_info: Self::MapInfo, - ) -> Result<(), (PhysPointerError, Self::MapInfo)> { - // SAFETY: `map_info` came from the same LVBS platform mapper and has no outstanding uses - // beyond the physical-pointer guard that is dropping it. - unsafe { - >::vunmap( - crate::platform_low(), - map_info, - ) - } - } - - fn validate_unowned(&self, pages: &PhysPageAddrArray) -> Result<(), PhysPointerError> { - crate::platform_low().validate_unowned(pages) - } - - unsafe fn protect( - &self, - pages: &PhysPageAddrArray, - perms: PhysPageMapPermissions, - ) -> Result<(), PhysPointerError> { - // SAFETY: callers uphold `VmapManager::protect`; this forwards unchanged to LVBS. - unsafe { crate::platform_low().protect(pages, perms) } - } -} - impl RawPointerProvider for LinuxKernel { type RawConstPointer = UserConstPtr; type RawMutPointer = UserMutPtr; diff --git a/litebox_platform_lvbs/src/mshv/mod.rs b/litebox_platform_lvbs/src/mshv/mod.rs index 0f1601bb03..898e0056ce 100644 --- a/litebox_platform_lvbs/src/mshv/mod.rs +++ b/litebox_platform_lvbs/src/mshv/mod.rs @@ -15,6 +15,62 @@ pub mod vsm_intercept; pub mod vtl1_mem_layout; pub mod vtl_switch; +use litebox_common_linux::vmap::{ + GlobalVmapManager, PhysPageAddrArray, PhysPageMapPermissions, PhysPointerError, VmapManager, +}; + +/// Provider for MSHV operations authorized to modify protected VTL0 frames. +struct PrivilegedVmap; + +impl GlobalVmapManager for PrivilegedVmap { + type Manager = PrivilegedVmap; + + fn manager() -> &'static Self::Manager { + &PrivilegedVmap + } +} + +unsafe impl VmapManager for PrivilegedVmap { + type MapInfo = crate::LvbsPhysPageMapInfo; + + unsafe fn vmap( + &self, + pages: &PhysPageAddrArray, + perms: PhysPageMapPermissions, + ) -> Result { + // SAFETY: callers uphold the raw mapping contract. This provider is used only for + // independently authorized HEKI patch and ring-buffer writes. + unsafe { crate::platform_low().vmap_privileged(pages, perms) } + } + + unsafe fn vunmap( + &self, + map_info: Self::MapInfo, + ) -> Result<(), (PhysPointerError, Self::MapInfo)> { + // SAFETY: `map_info` came from the same LVBS mapper and has no outstanding uses beyond the + // physical-pointer guard that is dropping it. + unsafe { + >::vunmap( + crate::platform_low(), + map_info, + ) + } + } + + fn validate_unowned(&self, pages: &PhysPageAddrArray) -> Result<(), PhysPointerError> { + crate::platform_low().validate_unowned(pages) + } + + unsafe fn protect( + &self, + pages: &PhysPageAddrArray, + perms: PhysPageMapPermissions, + ) -> Result<(), PhysPointerError> { + // SAFETY: callers uphold `VmapManager::protect`; this forwards unchanged to LVBS. + unsafe { crate::platform_low().protect(pages, perms) } + } +} + type Vtl0PhysConstPtr = litebox_common_linux::physical_pointers::PhysConstPtr; @@ -22,7 +78,7 @@ type Vtl0PhysConstPtr = /// buffer (fixed address). It bypasses `protected_frames` rejection and locking. /// DO NOT USE IT for other VTL0 destinations which could enable confused-deputy writes. type PrivilegedVtl0PhysMutPtr = - litebox_common_linux::physical_pointers::PhysMutPtr; + litebox_common_linux::physical_pointers::PhysMutPtr; use crate::arch::MAX_CORES; use crate::mshv::vtl1_mem_layout::PAGE_SIZE; diff --git a/litebox_platform_lvbs/src/mshv/vsm.rs b/litebox_platform_lvbs/src/mshv/vsm.rs index fdd7f1b3d5..a635789403 100644 --- a/litebox_platform_lvbs/src/mshv/vsm.rs +++ b/litebox_platform_lvbs/src/mshv/vsm.rs @@ -786,8 +786,7 @@ pub fn mshv_vsm_free_guest_module_init(token: i64) -> Result { .remove_patch_data(&freed_init_patch_targets); } - result?; - Ok(0) + result.map(|()| 0) } /// VSM function for supporting the unloading of a guest kernel module. From 5aa8aa09f47492f5e4f0299d8f7c45a01517798d Mon Sep 17 00:00:00 2001 From: Sangho Lee Date: Thu, 16 Jul 2026 17:26:21 +0000 Subject: [PATCH 3/5] rename --- litebox_platform_lvbs/src/lib.rs | 2 +- litebox_platform_lvbs/src/mshv/vsm.rs | 66 +++++++++++++-------------- 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/litebox_platform_lvbs/src/lib.rs b/litebox_platform_lvbs/src/lib.rs index 494a803250..49d9fe667a 100644 --- a/litebox_platform_lvbs/src/lib.rs +++ b/litebox_platform_lvbs/src/lib.rs @@ -1344,7 +1344,7 @@ unsafe impl VmapManager for Linu PhysFrame::::containing_address(x86_64::PhysAddr::new(range.start)), PhysFrame::::containing_address(x86_64::PhysAddr::new(range.end)), ); - crate::mshv::vsm::protect_physical_memory_range(frame_range, mem_attr) + crate::mshv::vsm::set_vtl0_memory_protection(frame_range, mem_attr) .map_err(|_| PhysPointerError::UnsupportedPermissions(perms.bits()))?; } diff --git a/litebox_platform_lvbs/src/mshv/vsm.rs b/litebox_platform_lvbs/src/mshv/vsm.rs index a635789403..83bc8ec8e5 100644 --- a/litebox_platform_lvbs/src/mshv/vsm.rs +++ b/litebox_platform_lvbs/src/mshv/vsm.rs @@ -292,7 +292,7 @@ pub fn mshv_vsm_protect_memory(pa: u64, nranges: u64) -> Result { continue; } - protect_physical_memory_range( + set_vtl0_memory_protection( PhysFrame::range( // `HekiRange::is_valid` already validated both physical addresses. PhysFrame::containing_address(PhysAddr::new(pa)), @@ -425,7 +425,7 @@ pub fn mshv_vsm_load_kdata(pa: u64, nranges: u64) -> Result { // fails, letting kdata load proceed so that heki is not broken. if !kexec_trampoline_insert_failed { for kexec_trampoline_range in &kexec_trampoline_metadata { - protect_physical_memory_range( + set_vtl0_memory_protection( kexec_trampoline_range.phys_frame_range, MemAttr::MEM_ATTR_READ, )?; @@ -511,7 +511,7 @@ impl FrameReservation { /// /// The claim is inserted into `protected_frames` atomically under its lock together with the /// overlap check. Each claimed frame is a "maybe-non-writable" entry that is later resolved by - /// [`apply_vtl_protection`]: promotion to RX/RO keeps it, promotion to RW (or release on failure) + /// [`update_vtl0_memory_protection`]: promotion to RX/RO keeps it, promotion to RW (or rollback) /// removes it. On rejection, this call's inserts are rolled back. The returned status for each /// input identifies whether that occurrence added a new claim. fn reserve( @@ -545,7 +545,6 @@ impl FrameReservation { } else { classify_reservation(&owned_before, &protected, range.clone()) }; - seen.insert(range.clone()); let status = match status { Ok(status) => status, Err(error) => { @@ -559,6 +558,7 @@ impl FrameReservation { return Err(error); } }; + seen.insert(range.clone()); if status == ReservationStatus::AlreadyOwned { statuses.push(status); continue; @@ -585,7 +585,7 @@ impl Drop for FrameReservation { // Rollback: attempt to release every reserved range back to VTL0. Drop cannot report a // release failure, so debug builds assert it instead. for &phys_frame_range in &self.owned_ranges { - let result = release_physical_memory_range(phys_frame_range); + let result = restore_vtl0_memory_access(phys_frame_range); debug_assert!( result.is_ok(), "Failed to release reserved VTL0 memory protection" @@ -662,11 +662,10 @@ pub fn mshv_vsm_validate_guest_module(pa: u64, nranges: u64, _flags: u64) -> Res let mut frame_guard = FrameReservation::new(); let _ = frame_guard.reserve(module_memory_metadata.iter().map(|r| r.phys_frame_range))?; - // Write-protect the frames that will become non-writable (RX/RO) BEFORE copying, so VTL0 cannot - // change them between the copy/validation and the final permission promotion. + // Freeze frames that require immutable copy/validation to avoid TOCTOU. for mod_mem_range in &module_memory_metadata { if !mod_mem_type_to_mem_attr(mod_mem_range.mod_mem_type).contains(MemAttr::MEM_ATTR_WRITE) { - protect_physical_memory_range(mod_mem_range.phys_frame_range, MemAttr::MEM_ATTR_READ)?; + set_vtl0_memory_protection(mod_mem_range.phys_frame_range, MemAttr::MEM_ATTR_READ)?; } } @@ -703,7 +702,7 @@ pub fn mshv_vsm_validate_guest_module(pa: u64, nranges: u64, _flags: u64) -> Res // once a module is verified and validated, change the permission of its memory ranges based on their types for mod_mem_range in &module_memory_metadata { - protect_physical_memory_range( + set_vtl0_memory_protection( mod_mem_range.phys_frame_range, mod_mem_type_to_mem_attr(mod_mem_range.mod_mem_type), )?; @@ -753,11 +752,11 @@ pub fn mshv_vsm_free_guest_module_init(token: i64) -> Result { for mod_mem_range in entry.iter_mem_ranges() { let range_result = match mod_mem_range.mod_mem_type { ModMemType::InitText | ModMemType::InitData | ModMemType::InitRoData => { - release_physical_memory_range(mod_mem_range.phys_frame_range) + restore_vtl0_memory_access(mod_mem_range.phys_frame_range) } ModMemType::RoAfterInit => { // make this memory range read-only after initialization - protect_physical_memory_range( + set_vtl0_memory_protection( mod_mem_range.phys_frame_range, MemAttr::MEM_ATTR_READ, ) @@ -808,7 +807,7 @@ pub fn mshv_vsm_unload_guest_module(token: i64) -> Result { .iter_entry(token) { for mod_mem_range in entry.iter_mem_ranges() { - release_physical_memory_range(mod_mem_range.phys_frame_range)?; + restore_vtl0_memory_access(mod_mem_range.phys_frame_range)?; } } @@ -864,7 +863,7 @@ pub fn mshv_vsm_kexec_validate(pa: u64, nranges: u64, crash: u64) -> Result Result Result Result Vec { let is_init = |t| { matches!( @@ -1644,9 +1643,10 @@ pub(crate) fn validate_mutable_vtl0_pages( Ok(()) } -/// Update the locked registry after a protection change. Holding the exclusive guard across the -/// hypercall and update synchronizes transitions with ordinary writable mappings; privileged HEKI -/// and ring-buffer mappings intentionally bypass it. +/// Update the locked registry after a protection change. +/// +/// This function assumes that the caller holds the exclusive guard across the hypercall and +/// update synchronizes transitions with ordinary writable mappings. fn record_frame_protection( set: &mut RangeSet, phys_frame_range: PhysFrameRange, @@ -1676,28 +1676,26 @@ fn record_frame_protection( /// /// `phys_frame_range` specifies the physical frame range to protect (must belong to VTL0). /// `mem_attr` specifies the memory attributes (VTL0's allowed access) to be applied. -pub(crate) fn protect_physical_memory_range( +pub(crate) fn set_vtl0_memory_protection( phys_frame_range: PhysFrameRange, mem_attr: MemAttr, ) -> Result<(), VsmError> { - apply_vtl_protection(phys_frame_range, mem_attr) + update_vtl0_memory_protection(phys_frame_range, mem_attr) } /// Hands a VTL0 physical memory range back to VTL0 (read-write), which also clears its protection. -fn release_physical_memory_range( - phys_frame_range: PhysFrameRange, -) -> Result<(), VsmError> { - apply_vtl_protection( +fn restore_vtl0_memory_access(phys_frame_range: PhysFrameRange) -> Result<(), VsmError> { + update_vtl0_memory_protection( phys_frame_range, MemAttr::MEM_ATTR_READ | MemAttr::MEM_ATTR_WRITE, ) } -/// Shared implementation of [`protect_physical_memory_range`] / [`release_physical_memory_range`]. +/// Shared implementation of [`set_vtl0_memory_protection`] / [`restore_vtl0_memory_access`]. /// Applies `mem_attr` to the VTL0 portions of `phys_frame_range` (skipping any VTL1 working memory) /// and updates the registry from the result: a frame non-writable by VTL0 (RO/RX) is recorded as /// protected; a VTL0-writable frame is removed. -fn apply_vtl_protection( +fn update_vtl0_memory_protection( phys_frame_range: PhysFrameRange, mem_attr: MemAttr, ) -> Result<(), VsmError> { @@ -1750,8 +1748,8 @@ fn apply_vtl_protection( Ok(()) } -/// This function is a variant of [`protect_physical_memory_range`] to protect a VTL1 physical memory range. -/// Unlike [`protect_physical_memory_range`], this is intended exclusively for securing VTL1's own pages. +/// This function is a variant of [`set_vtl0_memory_protection`] to protect a VTL1 physical memory range. +/// Unlike [`set_vtl0_memory_protection`], this is intended exclusively for securing VTL1's own pages. /// VTL0 should never access VTL1 memory, so the memory attribute is always empty (no read, write, or execute). /// /// Note. This function doesn't check whether `phys_frame_range` belongs to VTL1 because it is called by BSP From 27ff00379499d4e983986b902f4977341733d773 Mon Sep 17 00:00:00 2001 From: Sangho Lee Date: Thu, 16 Jul 2026 18:32:23 +0000 Subject: [PATCH 4/5] refactoring --- litebox_common_linux/src/vmap.rs | 11 +- litebox_platform_lvbs/src/lib.rs | 20 +- litebox_platform_lvbs/src/mshv/mod.rs | 6 +- litebox_platform_lvbs/src/mshv/vsm.rs | 412 ++++++++++++++------------ 4 files changed, 239 insertions(+), 210 deletions(-) diff --git a/litebox_common_linux/src/vmap.rs b/litebox_common_linux/src/vmap.rs index 9a082e21d2..4218ce6b7b 100644 --- a/litebox_common_linux/src/vmap.rs +++ b/litebox_common_linux/src/vmap.rs @@ -42,12 +42,13 @@ pub unsafe trait VmapManager { Err(PhysPointerError::UnsupportedOperation) } - /// Map pages without the platform's mutable-access checks for trusted callers. Ordinary - /// [`Self::vmap`] may delegate here after performing its checks. The default is deny. + /// Map pages while bypassing platform-defined ordinary access checks. Ordinary [`Self::vmap`] + /// may delegate here after performing those checks. The default is deny. /// /// # Safety /// - /// This method has the same raw-mapping requirements as [`Self::vmap`]. + /// In addition to [`Self::vmap`]'s raw-mapping requirements, the caller must independently + /// authorize bypassing the omitted platform checks. unsafe fn vmap_privileged( &self, _pages: &PhysPageAddrArray, @@ -92,7 +93,7 @@ pub unsafe trait VmapManager { /// platform-defined foreign-memory VA ranges, never through LiteBox-owned VA ranges. fn validate_unowned(&self, pages: &PhysPageAddrArray) -> Result<(), PhysPointerError>; - /// Protect the given physical pages to ensure concurrent read or exclusive write access: + /// Protect the given physical pages according to `perms`: /// - Read protection: prevent others from writing to the pages. /// - Read/write protection: prevent others from reading or writing to the pages. /// - No protection: allow others to read and write the pages. @@ -106,7 +107,7 @@ pub unsafe trait VmapManager { /// /// This function relies on hypercalls or other privileged hardware features and assumes those features /// are safe to use. - /// The caller should unprotect the pages when they are no longer needed to access them. + /// Callers should restore ordinary access when protection is no longer needed. unsafe fn protect( &self, pages: &PhysPageAddrArray, diff --git a/litebox_platform_lvbs/src/lib.rs b/litebox_platform_lvbs/src/lib.rs index 49d9fe667a..461cdbddee 100644 --- a/litebox_platform_lvbs/src/lib.rs +++ b/litebox_platform_lvbs/src/lib.rs @@ -46,12 +46,12 @@ pub mod mshv; pub mod syscall_entry; -/// Mapping info returned by LVBS vmap paths. Ordinary writable mappings retain their -/// `protected_frames` guard here until unmap. +/// Mapping metadata. Ordinary writable mappings retain an opaque protected-frame access guard for +/// the mapping's lifetime. pub struct LvbsPhysPageMapInfo { base: *mut u8, size: usize, - protected_frames_guard: Option>>, + protected_frame_access: Option>, } impl LvbsPhysPageMapInfo { @@ -59,7 +59,7 @@ impl LvbsPhysPageMapInfo { Self { base, size, - protected_frames_guard: None, + protected_frame_access: None, } } } @@ -1130,12 +1130,10 @@ unsafe impl VmapManager for Linu pages: &PhysPageAddrArray, perms: PhysPageMapPermissions, ) -> Result { - let protected_frames = if perms.contains(PhysPageMapPermissions::WRITE) { + let protected_frame_access = if perms.contains(PhysPageMapPermissions::WRITE) { // This shared guard spans map/copy/unmap. It permits concurrent foreign-memory writes // but does not support re-entry into a VTL protection change. - let guard = crate::mshv::vsm::protected_frames().read(); - crate::mshv::vsm::validate_mutable_vtl0_pages(&guard, pages)?; - Some(guard) + Some(crate::mshv::vsm::protected_frame_registry().acquire_access_guard(pages)?) } else { None }; @@ -1143,7 +1141,7 @@ unsafe impl VmapManager for Linu // the guard is retained through map, access, and unmap. `vmap_privileged` provides the // shared raw mapping implementation. let mut map_info = unsafe { self.vmap_privileged(pages, perms)? }; - map_info.protected_frames_guard = protected_frames; + map_info.protected_frame_access = protected_frame_access; Ok(map_info) } @@ -1329,7 +1327,7 @@ unsafe impl VmapManager for Linu } let mem_attr = if perms.contains(PhysPageMapPermissions::WRITE) { - // VTL1 wants to write data to the pages, preventing VTL0 from reading/executing the pages. + // VTL1 needs writable access, so deny VTL0 all access. crate::mshv::heki::MemAttr::empty() } else if perms.contains(PhysPageMapPermissions::READ) { // VTL1 wants to read data from the pages, preventing VTL0 from writing to the pages. @@ -1344,7 +1342,7 @@ unsafe impl VmapManager for Linu PhysFrame::::containing_address(x86_64::PhysAddr::new(range.start)), PhysFrame::::containing_address(x86_64::PhysAddr::new(range.end)), ); - crate::mshv::vsm::set_vtl0_memory_protection(frame_range, mem_attr) + crate::mshv::vsm::protect_physical_memory_range(frame_range, mem_attr) .map_err(|_| PhysPointerError::UnsupportedPermissions(perms.bits()))?; } diff --git a/litebox_platform_lvbs/src/mshv/mod.rs b/litebox_platform_lvbs/src/mshv/mod.rs index 898e0056ce..168476a4ad 100644 --- a/litebox_platform_lvbs/src/mshv/mod.rs +++ b/litebox_platform_lvbs/src/mshv/mod.rs @@ -74,9 +74,9 @@ unsafe impl VmapManager for PrivilegedVmap { type Vtl0PhysConstPtr = litebox_common_linux::physical_pointers::PhysConstPtr; -/// Mutable VTL0 pointer reserved for the HEKI text patching (validated) and log ring -/// buffer (fixed address). It bypasses `protected_frames` rejection and locking. -/// DO NOT USE IT for other VTL0 destinations which could enable confused-deputy writes. +/// Mutable VTL0 pointer reserved for validated HEKI text patching and the fixed-address log ring +/// buffer. It bypasses ordinary protected-frame access checks and synchronization. Do not use it for other +/// VTL0 destinations that could enable confused-deputy writes. type PrivilegedVtl0PhysMutPtr = litebox_common_linux::physical_pointers::PhysMutPtr; diff --git a/litebox_platform_lvbs/src/mshv/vsm.rs b/litebox_platform_lvbs/src/mshv/vsm.rs index 83bc8ec8e5..fe9c690a6c 100644 --- a/litebox_platform_lvbs/src/mshv/vsm.rs +++ b/litebox_platform_lvbs/src/mshv/vsm.rs @@ -292,7 +292,7 @@ pub fn mshv_vsm_protect_memory(pa: u64, nranges: u64) -> Result { continue; } - set_vtl0_memory_protection( + protect_physical_memory_range( PhysFrame::range( // `HekiRange::is_valid` already validated both physical addresses. PhysFrame::containing_address(PhysAddr::new(pa)), @@ -425,7 +425,7 @@ pub fn mshv_vsm_load_kdata(pa: u64, nranges: u64) -> Result { // fails, letting kdata load proceed so that heki is not broken. if !kexec_trampoline_insert_failed { for kexec_trampoline_range in &kexec_trampoline_metadata { - set_vtl0_memory_protection( + protect_physical_memory_range( kexec_trampoline_range.phys_frame_range, MemAttr::MEM_ATTR_READ, )?; @@ -469,7 +469,8 @@ pub fn mshv_vsm_load_kdata(pa: u64, nranges: u64) -> Result { } /// RAII reservation over VTL0 physical frames, shared by module load and kexec validation. -/// On drop without `commit`, release of every reserved range is attempted. +/// On drop without `commit`, every newly reserved range is restored to VTL0 read/write, +/// non-executable access. struct FrameReservation { owned_ranges: Vec>, owned_frames: RangeSet, @@ -482,20 +483,6 @@ enum ReservationStatus { AlreadyOwned, } -fn classify_reservation( - owned: &RangeSet, - protected: &RangeSet, - range: Range, -) -> Result { - if owned.gaps(&range).next().is_none() { - return Ok(ReservationStatus::AlreadyOwned); - } - if owned.overlaps(&range) || protected.overlaps(&range) { - return Err(VsmError::ProtectedFrameOverlap); - } - Ok(ReservationStatus::New) -} - impl FrameReservation { fn new() -> Self { Self { @@ -505,15 +492,27 @@ impl FrameReservation { } } - /// Reserve `frames`: reject any overlap with VTL1 working memory, a frame already in the registry - /// (a non-writable frame the reservation would downgrade or another live reservation's claim). - /// A range already covered by this reservation is idempotent. Call again to add other frames. + fn classify( + owned: &RangeSet, + registry: &ProtectedFrameUpdateGuard<'_>, + range: Range, + ) -> Result { + if owned.gaps(&range).next().is_none() { + return Ok(ReservationStatus::AlreadyOwned); + } + if owned.overlaps(&range) || registry.overlaps(&range) { + Err(VsmError::ProtectedFrameOverlap) + } else { + Ok(ReservationStatus::New) + } + } + + /// Reserve `frames`. Ranges fully owned before this call are accepted idempotently. Overlap + /// within this batch, partial overlap with prior ownership, and overlap with VTL1, protected + /// frames, or another reservation are rejected. /// - /// The claim is inserted into `protected_frames` atomically under its lock together with the - /// overlap check. Each claimed frame is a "maybe-non-writable" entry that is later resolved by - /// [`update_vtl0_memory_protection`]: promotion to RX/RO keeps it, promotion to RW (or rollback) - /// removes it. On rejection, this call's inserts are rolled back. The returned status for each - /// input identifies whether that occurrence added a new claim. + /// Validation and insertion are atomic under exclusive registry access. On rejection, only + /// claims added by this call are rolled back. fn reserve( &mut self, frames: impl IntoIterator>, @@ -522,53 +521,53 @@ impl FrameReservation { let vtl1_start = vtl1.start.start_address().as_u64(); let vtl1_end = vtl1.end.start_address().as_u64(); - let mut protected = protected_frames().write(); - // Idempotence applies only to ranges owned before this call. - let owned_before = self.owned_frames.clone(); - let mut seen = RangeSet::new(); - let mut statuses = Vec::new(); - // Frames this call adds, so a later overlap rolls back only them. - let rollback_from = self.owned_ranges.len(); - for phys_frame_range in frames { - let start = phys_frame_range.start.start_address().as_u64(); - let end = phys_frame_range.end.start_address().as_u64(); - if start >= end { - statuses.push(ReservationStatus::AlreadyOwned); - continue; - } - // `protected` holds existing non-writable frames, this reservation's earlier claims, and - // any other concurrent reservation's in-flight claims, so this one check rejects - // cross-object downgrade, intra-overlap, and concurrent double-claim alike. - let range = start..end; - let status = if seen.overlaps(&range) || (start < vtl1_end && vtl1_start < end) { - Err(VsmError::ProtectedFrameOverlap) - } else { - classify_reservation(&owned_before, &protected, range.clone()) - }; - let status = match status { - Ok(status) => status, - Err(error) => { - for undo in &self.owned_ranges[rollback_from..] { - let range = - undo.start.start_address().as_u64()..undo.end.start_address().as_u64(); - protected.remove(range.clone()); - self.owned_frames.remove(range); + protected_frame_registry().with_exclusive(|protected| { + // Idempotence applies only to ranges owned before this call. + let owned_before = self.owned_frames.clone(); + let mut seen = RangeSet::new(); + let mut statuses = Vec::new(); + // Frames this call adds, so a later overlap rolls back only them. + let rollback_from = self.owned_ranges.len(); + for phys_frame_range in frames { + let start = phys_frame_range.start.start_address().as_u64(); + let end = phys_frame_range.end.start_address().as_u64(); + if start >= end { + statuses.push(ReservationStatus::AlreadyOwned); + continue; + } + // `protected` holds existing non-writable frames, this reservation's earlier + // claims, and any other concurrent reservation's in-flight claims. + let range = start..end; + let status = if seen.overlaps(&range) || (start < vtl1_end && vtl1_start < end) { + Err(VsmError::ProtectedFrameOverlap) + } else { + Self::classify(&owned_before, protected, range.clone()) + }; + let status = match status { + Ok(status) => status, + Err(error) => { + for undo in &self.owned_ranges[rollback_from..] { + let range = undo.start.start_address().as_u64() + ..undo.end.start_address().as_u64(); + protected.remove(range.clone()); + self.owned_frames.remove(range); + } + self.owned_ranges.truncate(rollback_from); + return Err(error); } - self.owned_ranges.truncate(rollback_from); - return Err(error); + }; + seen.insert(range.clone()); + if status == ReservationStatus::AlreadyOwned { + statuses.push(status); + continue; } - }; - seen.insert(range.clone()); - if status == ReservationStatus::AlreadyOwned { + protected.insert(range.clone()); + self.owned_frames.insert(range); + self.owned_ranges.push(phys_frame_range); statuses.push(status); - continue; } - protected.insert(range.clone()); - self.owned_frames.insert(range); - self.owned_ranges.push(phys_frame_range); - statuses.push(status); - } - Ok(statuses) + Ok(statuses) + }) } /// Mark the reserved frames as committed; drop becomes a no-op. @@ -582,13 +581,13 @@ impl Drop for FrameReservation { if self.committed { return; } - // Rollback: attempt to release every reserved range back to VTL0. Drop cannot report a - // release failure, so debug builds assert it instead. + // Rollback: restore every newly reserved range to VTL0 read/write, non-executable access. + // Drop cannot report failure, so debug builds assert it. for &phys_frame_range in &self.owned_ranges { - let result = restore_vtl0_memory_access(phys_frame_range); + let result = unprotect_physical_memory_range(phys_frame_range); debug_assert!( result.is_ok(), - "Failed to release reserved VTL0 memory protection" + "Failed to restore VTL0 read/write access for reserved frames" ); } } @@ -657,15 +656,14 @@ pub fn mshv_vsm_validate_guest_module(pa: u64, nranges: u64, _flags: u64) -> Res } } - // Reject overlap and reserve this module's frames. Any overlap is adversarial (legitimate - // module frames are never shared). The reservation attempts to release the frames on failure. + // Reject overlap and reserve this module's frames. Legitimate module frames are never shared. let mut frame_guard = FrameReservation::new(); let _ = frame_guard.reserve(module_memory_metadata.iter().map(|r| r.phys_frame_range))?; // Freeze frames that require immutable copy/validation to avoid TOCTOU. for mod_mem_range in &module_memory_metadata { if !mod_mem_type_to_mem_attr(mod_mem_range.mod_mem_type).contains(MemAttr::MEM_ATTR_WRITE) { - set_vtl0_memory_protection(mod_mem_range.phys_frame_range, MemAttr::MEM_ATTR_READ)?; + protect_physical_memory_range(mod_mem_range.phys_frame_range, MemAttr::MEM_ATTR_READ)?; } } @@ -700,9 +698,10 @@ pub fn mshv_vsm_validate_guest_module(pa: u64, nranges: u64, _flags: u64) -> Res return Err(VsmError::ModuleRelocationInvalid); } - // once a module is verified and validated, change the permission of its memory ranges based on their types + // Once a module is verified and validated, change the permission of its memory ranges based on + // their types. Frozen frames will remain as RO; some of them will turn into RX. for mod_mem_range in &module_memory_metadata { - set_vtl0_memory_protection( + protect_physical_memory_range( mod_mem_range.phys_frame_range, mod_mem_type_to_mem_attr(mod_mem_range.mod_mem_type), )?; @@ -752,11 +751,11 @@ pub fn mshv_vsm_free_guest_module_init(token: i64) -> Result { for mod_mem_range in entry.iter_mem_ranges() { let range_result = match mod_mem_range.mod_mem_type { ModMemType::InitText | ModMemType::InitData | ModMemType::InitRoData => { - restore_vtl0_memory_access(mod_mem_range.phys_frame_range) + unprotect_physical_memory_range(mod_mem_range.phys_frame_range) } ModMemType::RoAfterInit => { // make this memory range read-only after initialization - set_vtl0_memory_protection( + protect_physical_memory_range( mod_mem_range.phys_frame_range, MemAttr::MEM_ATTR_READ, ) @@ -776,8 +775,8 @@ pub fn mshv_vsm_free_guest_module_init(token: i64) -> Result { .vtl0_kernel_info .module_memory_metadata .remove_init_ranges(token); - // Drop the precomputed patches targeting those freed init frames so a stale init patch cannot - // later be applied to recycled frames. + // Remove the precomputed patches targeting those freed init frames so a stale init patch cannot + // later be applied to recycled frames (no patch-after-free). if !freed_init_patch_targets.is_empty() { crate::platform_low() .vtl0_kernel_info @@ -807,7 +806,7 @@ pub fn mshv_vsm_unload_guest_module(token: i64) -> Result { .iter_entry(token) { for mod_mem_range in entry.iter_mem_ranges() { - restore_vtl0_memory_access(mod_mem_range.phys_frame_range)?; + unprotect_physical_memory_range(mod_mem_range.phys_frame_range)?; } } @@ -863,7 +862,7 @@ pub fn mshv_vsm_kexec_validate(pa: u64, nranges: u64, crash: u64) -> Result Result Result Result Option> { Some(heki_pages) } -/// Registry of VTL0 frames that are non-writable to VTL0 or reserved by an in-flight protection -/// operation. Module and kexec reservations use exclusive access to update it. Ordinary writable -/// LVBS vmaps retain shared access through map, access, and unmap, allowing concurrent foreign-memory -/// writes while excluding protection changes. Privileged HEKI and ring-buffer mappings bypass it. -/// This registry is needed because Hyper-V provides no query for current VTL protection. -pub(crate) fn protected_frames() -> &'static SpinRwLock> { - static PROTECTED_FRAMES: Once>> = Once::new(); - PROTECTED_FRAMES.call_once(|| SpinRwLock::new(RangeSet::new())) -} - -pub(crate) fn validate_mutable_vtl0_pages( - protected: &RangeSet, - pages: &litebox_common_linux::vmap::PhysPageAddrArray, -) -> Result<(), litebox_common_linux::vmap::PhysPointerError> { - for page in pages { - let start = page.as_usize() as u64; - let end = start - .checked_add(ALIGN as u64) - .ok_or(litebox_common_linux::vmap::PhysPointerError::Overflow)?; - if protected.overlaps(&(start..end)) { - return Err( - litebox_common_linux::vmap::PhysPointerError::InvalidPhysicalAddress( - page.as_usize(), - ), - ); +/// Registry of VTL0 frames that are non-writable to VTL0 or reserved by in-flight module or kexec +/// validation. Ordinary writable mappings retain shared access for their lifetime; reservations and +/// VTL0 protection updates use exclusive access. Privileged HEKI and ring-buffer mappings bypass +/// the registry. +pub(crate) struct ProtectedFrameRegistry { + frames: SpinRwLock>, +} + +/// Opaque guard that holds shared registry access for an ordinary writable mapping, blocking +/// exclusive protection and reservation updates until dropped. +pub(crate) struct ProtectedFrameAccessGuard<'a> { + _guard: spin::rwlock::RwLockReadGuard<'a, RangeSet>, +} + +struct ProtectedFrameUpdateGuard<'a> { + guard: spin::rwlock::RwLockWriteGuard<'a, RangeSet>, +} + +impl ProtectedFrameUpdateGuard<'_> { + fn overlaps(&self, range: &Range) -> bool { + self.guard.overlaps(range) + } + + fn insert(&mut self, range: Range) { + self.guard.insert(range); + } + + fn remove(&mut self, range: Range) { + self.guard.remove(range); + } + + fn record_protection(&mut self, phys_frame_range: PhysFrameRange, protect: bool) { + let start = phys_frame_range.start.start_address().as_u64(); + let end = phys_frame_range.end.start_address().as_u64(); + if start >= end { + return; + } + if protect { + self.insert(start..end); + } else { + self.remove(start..end); } } - Ok(()) } -/// Update the locked registry after a protection change. -/// -/// This function assumes that the caller holds the exclusive guard across the hypercall and -/// update synchronizes transitions with ordinary writable mappings. -fn record_frame_protection( - set: &mut RangeSet, - phys_frame_range: PhysFrameRange, - protect: bool, -) { - let start = phys_frame_range.start.start_address().as_u64(); - let end = phys_frame_range.end.start_address().as_u64(); - if start >= end { - return; - } - if protect { - set.insert(start..end); - } else { - set.remove(start..end); +impl ProtectedFrameRegistry { + fn new() -> Self { + Self { + frames: SpinRwLock::new(RangeSet::new()), + } } + + /// Validates that no requested page is registered as protected or reserved and returns a shared + /// guard that prevents protection or reservation updates until dropped. + pub(crate) fn acquire_access_guard( + &self, + pages: &litebox_common_linux::vmap::PhysPageAddrArray, + ) -> Result, litebox_common_linux::vmap::PhysPointerError> { + let guard = self.frames.read(); + for page in pages { + let start = page.as_usize() as u64; + let end = start + .checked_add(ALIGN as u64) + .ok_or(litebox_common_linux::vmap::PhysPointerError::Overflow)?; + if guard.overlaps(&(start..end)) { + return Err( + litebox_common_linux::vmap::PhysPointerError::InvalidPhysicalAddress( + page.as_usize(), + ), + ); + } + } + Ok(ProtectedFrameAccessGuard { _guard: guard }) + } + + /// Runs `f` with exclusive registry access. + fn with_exclusive(&self, f: impl FnOnce(&mut ProtectedFrameUpdateGuard<'_>) -> R) -> R { + f(&mut ProtectedFrameUpdateGuard { + guard: self.frames.write(), + }) + } +} + +pub(crate) fn protected_frame_registry() -> &'static ProtectedFrameRegistry { + static REGISTRY: Once = Once::new(); + REGISTRY.call_once(ProtectedFrameRegistry::new) } -/// Protects a VTL0 physical memory range from potentially compromised VTL0 by restricting its -/// access permissions using VTL protection mask (e.g., kernel code integrity). +/// Protect a VTL0 physical memory range using VTL protection mask (e.g., kernel code integrity). /// -/// The frame is recorded as protected iff the resulting VTL0 permission is non-writable (RO/RX). -/// See [`protected_frames`]. +/// The registry tracks non-writable VTL0 ranges and temporary validation reservations. +/// See [`protected_frame_registry`]. /// /// If the requested range overlaps with VTL1 working memory, the VTL1 portion is silently /// skipped and only the remaining VTL0 portions are protected. If the range falls entirely /// within VTL1, this function returns `Ok(())` without issuing a hypercall. /// -/// `phys_frame_range` specifies the physical frame range to protect (must belong to VTL0). +/// `phys_frame_range` specifies the range whose VTL0 permissions are updated; VTL1 working-memory +/// portions are ignored. /// `mem_attr` specifies the memory attributes (VTL0's allowed access) to be applied. -pub(crate) fn set_vtl0_memory_protection( - phys_frame_range: PhysFrameRange, - mem_attr: MemAttr, -) -> Result<(), VsmError> { - update_vtl0_memory_protection(phys_frame_range, mem_attr) -} - -/// Hands a VTL0 physical memory range back to VTL0 (read-write), which also clears its protection. -fn restore_vtl0_memory_access(phys_frame_range: PhysFrameRange) -> Result<(), VsmError> { - update_vtl0_memory_protection( - phys_frame_range, - MemAttr::MEM_ATTR_READ | MemAttr::MEM_ATTR_WRITE, - ) -} - -/// Shared implementation of [`set_vtl0_memory_protection`] / [`restore_vtl0_memory_access`]. -/// Applies `mem_attr` to the VTL0 portions of `phys_frame_range` (skipping any VTL1 working memory) -/// and updates the registry from the result: a frame non-writable by VTL0 (RO/RX) is recorded as -/// protected; a VTL0-writable frame is removed. -fn update_vtl0_memory_protection( +pub(crate) fn protect_physical_memory_range( phys_frame_range: PhysFrameRange, mem_attr: MemAttr, ) -> Result<(), VsmError> { @@ -1711,45 +1731,55 @@ fn update_vtl0_memory_protection( let overlaps_vtl1 = phys_frame_range.start < vtl1_range.end && vtl1_range.start < phys_frame_range.end; - let mut protected = protected_frames().write(); - - if !overlaps_vtl1 { - let pa = phys_frame_range.start.start_address().as_u64(); - let num_pages = phys_frame_range.count() as u64; - hv_modify_vtl_protection_mask(pa, num_pages, mem_attr_to_hv_page_prot_flags(mem_attr)) - .map_err(VsmError::HypercallFailed)?; - record_frame_protection(&mut protected, phys_frame_range, protect); - return Ok(()); - } + protected_frame_registry().with_exclusive(|protected| { + if !overlaps_vtl1 { + let pa = phys_frame_range.start.start_address().as_u64(); + let num_pages = phys_frame_range.count() as u64; + hv_modify_vtl_protection_mask(pa, num_pages, mem_attr_to_hv_page_prot_flags(mem_attr)) + .map_err(VsmError::HypercallFailed)?; + protected.record_protection(phys_frame_range, protect); + return Ok(()); + } - // Partial overlap: split into the portions before and after VTL1, skipping VTL1 pages. - let sub_ranges: [PhysFrameRange; 2] = { - let before = PhysFrame::range( - phys_frame_range.start, - core::cmp::min(phys_frame_range.end, vtl1_range.start), - ); - let after = PhysFrame::range( - core::cmp::max(phys_frame_range.start, vtl1_range.end), - phys_frame_range.end, - ); - [before, after] - }; + // Partial overlap: split into the portions before and after VTL1, skipping VTL1 pages. + let sub_ranges: [PhysFrameRange; 2] = { + let before = PhysFrame::range( + phys_frame_range.start, + core::cmp::min(phys_frame_range.end, vtl1_range.start), + ); + let after = PhysFrame::range( + core::cmp::max(phys_frame_range.start, vtl1_range.end), + phys_frame_range.end, + ); + [before, after] + }; - for sub_range in sub_ranges { - if sub_range.start >= sub_range.end { - continue; + for sub_range in sub_ranges { + if sub_range.start >= sub_range.end { + continue; + } + let pa = sub_range.start.start_address().as_u64(); + let num_pages = sub_range.count() as u64; + hv_modify_vtl_protection_mask(pa, num_pages, mem_attr_to_hv_page_prot_flags(mem_attr)) + .map_err(VsmError::HypercallFailed)?; + protected.record_protection(sub_range, protect); } - let pa = sub_range.start.start_address().as_u64(); - let num_pages = sub_range.count() as u64; - hv_modify_vtl_protection_mask(pa, num_pages, mem_attr_to_hv_page_prot_flags(mem_attr)) - .map_err(VsmError::HypercallFailed)?; - record_frame_protection(&mut protected, sub_range, protect); - } - Ok(()) + Ok(()) + }) +} + +/// Restore VTL0 read/write access while leaving execution disabled, and removes the registry entry. +fn unprotect_physical_memory_range( + phys_frame_range: PhysFrameRange, +) -> Result<(), VsmError> { + protect_physical_memory_range( + phys_frame_range, + MemAttr::MEM_ATTR_READ | MemAttr::MEM_ATTR_WRITE, + ) } -/// This function is a variant of [`set_vtl0_memory_protection`] to protect a VTL1 physical memory range. -/// Unlike [`set_vtl0_memory_protection`], this is intended exclusively for securing VTL1's own pages. +/// This function is a variant of [`protect_physical_memory_range`] to protect a VTL1 physical memory range. +/// Unlike [`protect_physical_memory_range`], this is intended exclusively for securing VTL1's own pages. /// VTL0 should never access VTL1 memory, so the memory attribute is always empty (no read, write, or execute). /// /// Note. This function doesn't check whether `phys_frame_range` belongs to VTL1 because it is called by BSP From 47ff384cdd2b2d14163bbd7d763729b7dbf563b6 Mon Sep 17 00:00:00 2001 From: Sangho Lee Date: Fri, 17 Jul 2026 16:49:17 +0000 Subject: [PATCH 5/5] minor optimization --- litebox_platform_lvbs/src/mshv/vsm.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/litebox_platform_lvbs/src/mshv/vsm.rs b/litebox_platform_lvbs/src/mshv/vsm.rs index fe9c690a6c..7cb97c8e3a 100644 --- a/litebox_platform_lvbs/src/mshv/vsm.rs +++ b/litebox_platform_lvbs/src/mshv/vsm.rs @@ -698,13 +698,18 @@ pub fn mshv_vsm_validate_guest_module(pa: u64, nranges: u64, _flags: u64) -> Res return Err(VsmError::ModuleRelocationInvalid); } - // Once a module is verified and validated, change the permission of its memory ranges based on - // their types. Frozen frames will remain as RO; some of them will turn into RX. + // Both read-only and executable frames have been frozen above. + // Thus, only promote executable frames to RX. for mod_mem_range in &module_memory_metadata { - protect_physical_memory_range( - mod_mem_range.phys_frame_range, - mod_mem_type_to_mem_attr(mod_mem_range.mod_mem_type), - )?; + if matches!( + mod_mem_range.mod_mem_type, + ModMemType::Text | ModMemType::InitText + ) { + protect_physical_memory_range( + mod_mem_range.phys_frame_range, + mod_mem_type_to_mem_attr(mod_mem_range.mod_mem_type), + )?; + } } // Commit the module's pre-computed patch data (transactional).