From b9c36ef3308036bb8c48a2bd3e099b90ec27d5a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 7 Oct 2025 14:39:23 +0300 Subject: [PATCH 1/2] [dbg] Add debug support for memory mapped regions in Hypervisors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- src/hyperlight_host/src/hypervisor/gdb/mod.rs | 368 ++++++++++++++++-- .../src/hypervisor/hyperv_linux.rs | 45 +-- .../src/hypervisor/hyperv_windows.rs | 34 +- src/hyperlight_host/src/hypervisor/kvm.rs | 46 +-- 4 files changed, 392 insertions(+), 101 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index e8daf5aa9..a3f524c04 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -27,9 +27,9 @@ mod x86_64_target; use std::io::{self, ErrorKind}; use std::net::TcpListener; use std::sync::{Arc, Mutex}; -use std::thread; +use std::{slice, thread}; -use arch::{SW_BP, SW_BP_SIZE}; +pub(crate) use arch::{SW_BP, SW_BP_SIZE}; use crossbeam_channel::{Receiver, Sender, TryRecvError}; use event_loop::event_loop_thread; use gdbstub::conn::ConnectionExt; @@ -47,6 +47,7 @@ use x86_64_target::HyperlightSandboxTarget; use super::InterruptHandle; use crate::mem::layout::SandboxMemoryLayout; +use crate::mem::memory_region::MemoryRegion; use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::HostSharedMemory; use crate::{HyperlightError, new_error}; @@ -113,6 +114,150 @@ pub(crate) struct X86_64Regs { pub(crate) mxcsr: u32, } +/// This abstracts the memory access functions that debugging needs from a sandbox +pub(crate) struct DebugMemoryAccess { + /// Memory manager that provides access to the guest memory + pub(crate) dbg_mem_access_fn: Arc>>, + /// Guest mapped memory regions + pub(crate) guest_mmap_regions: Vec, +} + +impl DebugMemoryAccess { + /// Reads memory from the guest's address space with a maximum length of a PAGE_SIZE + /// + /// # Arguments + /// * `data` - Buffer to store the read data + /// * `gpa` - Guest physical address to read from. + /// This address is shall be translated before calling this function + /// # Returns + /// * `Result<(), HyperlightError>` - Ok if successful, Err otherwise + fn read(&self, data: &mut [u8], gpa: u64) -> crate::Result<()> { + let read_len = data.len(); + + let mem_offset = (gpa as usize) + .checked_sub(SandboxMemoryLayout::BASE_ADDRESS) + .ok_or_else(|| { + log::warn!( + "gpa={:#X} causes subtract with underflow: \"gpa - BASE_ADDRESS={:#X}-{:#X}\"", + gpa, + gpa, + SandboxMemoryLayout::BASE_ADDRESS + ); + HyperlightError::TranslateGuestAddress(gpa) + })?; + + // First check the mapped memory regions to see if the address is within any of them + let mut region_found = false; + for reg in self.guest_mmap_regions.iter() { + if reg.guest_region.contains(&mem_offset) { + log::debug!("Found mapped region containing {:X}: {:#?}", gpa, reg); + + // Region found - calculate the offset within the region + let region_offset = mem_offset.checked_sub(reg.guest_region.start).ok_or_else(|| { + log::warn!( + "Cannot calculate offset in memory region: mem_offset={:#X}, base={:#X}", + mem_offset, + reg.guest_region.start, + ); + HyperlightError::TranslateGuestAddress(mem_offset as u64) + })?; + + let bytes: &[u8] = unsafe { + slice::from_raw_parts(reg.host_region.start as *const u8, reg.host_region.len()) + }; + data[..read_len].copy_from_slice(&bytes[region_offset..region_offset + read_len]); + + region_found = true; + break; + } + } + + if !region_found { + log::debug!( + "No mapped region found containing {:X}. Trying shared memory ...", + gpa + ); + + self.dbg_mem_access_fn + .try_lock() + .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? + .get_shared_mem_mut() + .copy_to_slice(&mut data[..read_len], mem_offset)?; + } + + Ok(()) + } + + /// Writes memory from the guest's address space with a maximum length of a PAGE_SIZE + /// + /// # Arguments + /// * `data` - Buffer containing the data to write + /// * `gpa` - Guest physical address to write to. + /// This address is shall be translated before calling this function + /// # Returns + /// * `Result<(), HyperlightError>` - Ok if successful, Err otherwise + fn write(&self, data: &[u8], gpa: u64) -> crate::Result<()> { + let write_len = data.len(); + + let mem_offset = (gpa as usize) + .checked_sub(SandboxMemoryLayout::BASE_ADDRESS) + .ok_or_else(|| { + log::warn!( + "gpa={:#X} causes subtract with underflow: \"gpa - BASE_ADDRESS={:#X}-{:#X}\"", + gpa, + gpa, + SandboxMemoryLayout::BASE_ADDRESS + ); + HyperlightError::TranslateGuestAddress(gpa) + })?; + + // First check the mapped memory regions to see if the address is within any of them + let mut region_found = false; + for reg in self.guest_mmap_regions.iter() { + if reg.guest_region.contains(&mem_offset) { + log::debug!("Found mapped region containing {:X}: {:#?}", gpa, reg); + + // Region found - calculate the offset within the region + let region_offset = mem_offset.checked_sub(reg.guest_region.start).ok_or_else(|| { + log::warn!( + "Cannot calculate offset in memory region: mem_offset={:#X}, base={:#X}", + mem_offset, + reg.guest_region.start, + ); + HyperlightError::TranslateGuestAddress(mem_offset as u64) + })?; + + let bytes: &mut [u8] = unsafe { + slice::from_raw_parts_mut( + reg.host_region.start as *mut u8, + reg.host_region.len(), + ) + }; + bytes[region_offset..region_offset + write_len].copy_from_slice(&data[..write_len]); + + region_found = true; + break; + } + } + + if !region_found { + log::debug!( + "No mapped region found containing {:X}. Trying shared memory at offset {:X} ...", + gpa, + mem_offset + ); + + self.dbg_mem_access_fn + .try_lock() + .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? + .get_shared_mem_mut() + .copy_from_slice(&data[..write_len], mem_offset)?; + } + + Ok(()) + } +} + /// Defines the possible reasons for which a vCPU can be stopped when debugging #[derive(Debug)] pub enum VcpuStopReason { @@ -212,7 +357,7 @@ pub(super) trait GuestDebug { &mut self, vcpu_fd: &Self::Vcpu, addr: u64, - dbg_mem_access_fn: Arc>>, + mem_access: &DebugMemoryAccess, ) -> crate::Result<()> { let addr = self.translate_gva(vcpu_fd, addr)?; @@ -222,8 +367,8 @@ pub(super) trait GuestDebug { // Write breakpoint OP code to write to guest memory let mut save_data = [0; SW_BP_SIZE]; - self.read_addrs(vcpu_fd, addr, &mut save_data[..], dbg_mem_access_fn.clone())?; - self.write_addrs(vcpu_fd, addr, &SW_BP, dbg_mem_access_fn)?; + self.read_addrs(vcpu_fd, addr, &mut save_data[..], mem_access)?; + self.write_addrs(vcpu_fd, addr, &SW_BP, mem_access)?; // Save guest memory to restore when breakpoint is removed self.save_sw_breakpoint_data(addr, save_data); @@ -237,7 +382,7 @@ pub(super) trait GuestDebug { vcpu_fd: &Self::Vcpu, mut gva: u64, mut data: &mut [u8], - dbg_mem_access_fn: Arc>>, + mem_access: &DebugMemoryAccess, ) -> crate::Result<()> { let data_len = data.len(); log::debug!("Read addr: {:X} len: {:X}", gva, data_len); @@ -249,20 +394,8 @@ pub(super) trait GuestDebug { data.len(), (PAGE_SIZE - (gpa & (PAGE_SIZE - 1))).try_into().unwrap(), ); - let offset = (gpa as usize) - .checked_sub(SandboxMemoryLayout::BASE_ADDRESS) - .ok_or_else(|| { - log::warn!( - "gva=0x{:#X} causes subtract with underflow: \"gpa - BASE_ADDRESS={:#X}-{:#X}\"", - gva, gpa, SandboxMemoryLayout::BASE_ADDRESS); - HyperlightError::TranslateGuestAddress(gva) - })?; - dbg_mem_access_fn - .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .get_shared_mem_mut() - .copy_to_slice(&mut data[..read_len], offset)?; + mem_access.read(&mut data[..read_len], gpa)?; data = &mut data[read_len..]; gva += read_len as u64; @@ -286,7 +419,7 @@ pub(super) trait GuestDebug { &mut self, vcpu_fd: &Self::Vcpu, addr: u64, - dbg_mem_access_fn: Arc>>, + mem_access: &DebugMemoryAccess, ) -> crate::Result<()> { let addr = self.translate_gva(vcpu_fd, addr)?; @@ -296,7 +429,7 @@ pub(super) trait GuestDebug { .ok_or_else(|| new_error!("Expected to contain the sw breakpoint address"))?; // Restore saved data to the guest's memory - self.write_addrs(vcpu_fd, addr, &save_data, dbg_mem_access_fn)?; + self.write_addrs(vcpu_fd, addr, &save_data, mem_access)?; Ok(()) } else { @@ -310,7 +443,7 @@ pub(super) trait GuestDebug { vcpu_fd: &Self::Vcpu, mut gva: u64, mut data: &[u8], - dbg_mem_access_fn: Arc>>, + mem_access: &DebugMemoryAccess, ) -> crate::Result<()> { let data_len = data.len(); log::debug!("Write addr: {:X} len: {:X}", gva, data_len); @@ -322,20 +455,9 @@ pub(super) trait GuestDebug { data.len(), (PAGE_SIZE - (gpa & (PAGE_SIZE - 1))).try_into().unwrap(), ); - let offset = (gpa as usize) - .checked_sub(SandboxMemoryLayout::BASE_ADDRESS) - .ok_or_else(|| { - log::warn!( - "gva=0x{:#X} causes subtract with underflow: \"gpa - BASE_ADDRESS={:#X}-{:#X}\"", - gva, gpa, SandboxMemoryLayout::BASE_ADDRESS); - HyperlightError::TranslateGuestAddress(gva) - })?; - dbg_mem_access_fn - .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .get_shared_mem_mut() - .copy_from_slice(&data[..write_len], offset)?; + // Use the memory access to write to guest memory + mem_access.write(&data[..write_len], gpa)?; data = &data[write_len..]; gva += write_len as u64; @@ -457,4 +579,180 @@ mod tests { let res = gdb_conn.recv(); assert!(res.is_ok()); } + + #[cfg(target_os = "linux")] + mod mem_access_tests { + use std::os::fd::AsRawFd; + use std::os::linux::fs::MetadataExt; + use std::sync::{Arc, Mutex}; + + use hyperlight_testing::dummy_guest_as_string; + + use super::*; + use crate::mem::memory_region::{MemoryRegionFlags, MemoryRegionType}; + use crate::sandbox::UninitializedSandbox; + use crate::sandbox::uninitialized::GuestBinary; + use crate::{log_then_return, new_error}; + + #[cfg(target_os = "linux")] + const BASE_VIRT: usize = 0x10000000 + SandboxMemoryLayout::BASE_ADDRESS; + + /// Dummy memory region to test memory access + /// This maps a file into memory and uses it as guest memory + fn get_mem_access() -> crate::Result { + let filename = dummy_guest_as_string().map_err(|e| new_error!("{}", e))?; + + let file = std::fs::File::options() + .read(true) + .write(true) + .open(&filename)?; + let file_size = file.metadata()?.st_size(); + let page_size = page_size::get(); + let size = (file_size as usize).div_ceil(page_size) * page_size; + let mapped_mem = unsafe { + libc::mmap( + std::ptr::null_mut(), + size, + libc::PROT_READ | libc::PROT_WRITE | libc::PROT_EXEC, + libc::MAP_PRIVATE, + file.as_raw_fd(), + 0, + ) + }; + if mapped_mem == libc::MAP_FAILED { + log_then_return!("mmap error: {:?}", std::io::Error::last_os_error()); + } + + // Create a sandbox memory manager with the mapped memory region + let sandbox = UninitializedSandbox::new(GuestBinary::FilePath(filename.clone()), None) + .inspect_err(|_| unsafe { + libc::munmap(mapped_mem, size); + })?; + let (mem_mgr, _) = sandbox.mgr.build(); + + // Create the memory access struct + let mem_access = DebugMemoryAccess { + dbg_mem_access_fn: Arc::new(Mutex::new(mem_mgr)), + guest_mmap_regions: vec![MemoryRegion { + host_region: mapped_mem as usize..mapped_mem.wrapping_add(size) as usize, + guest_region: BASE_VIRT..BASE_VIRT + size, + flags: MemoryRegionFlags::READ | MemoryRegionFlags::EXECUTE, + region_type: MemoryRegionType::Heap, + }], + }; + + Ok(mem_access) + } + + /// Gets a slice to the mapped memory region to be able to modify it + /// + /// NOTE: By returning a mutable slice from a mutable reference, we ensure + /// that the memory is not deallocated while the slice is in use. + unsafe fn get_mmap_slice(mem_access: &mut DebugMemoryAccess) -> &mut [u8] { + unsafe { + std::slice::from_raw_parts_mut( + mem_access.guest_mmap_regions[0].host_region.start as *mut u8, + mem_access.guest_mmap_regions[0].host_region.end + - mem_access.guest_mmap_regions[0].host_region.start, + ) + } + } + + /// Drops the mapped memory region + fn drop_mem_access(mem_access: DebugMemoryAccess) { + let mapped_mem = + mem_access.guest_mmap_regions[0].host_region.start as *mut libc::c_void; + let size = mem_access.guest_mmap_regions[0].host_region.end + - mem_access.guest_mmap_regions[0].host_region.start; + + unsafe { + libc::munmap(mapped_mem, size); + } + } + + #[test] + fn test_mem_access_read_single_byte() -> crate::Result<()> { + let mut mem_access = get_mem_access()?; + let offset = 2000; + + // Modify the memory directly to have a known value to read + { + let slice = unsafe { get_mmap_slice(&mut mem_access) }; + slice[offset] = 0xAA; + } + + let mut read_data = [0u8; 1]; + mem_access.read(&mut read_data, (BASE_VIRT + offset) as u64)?; + + assert_eq!(read_data[0], 0xAA); + + drop_mem_access(mem_access); + + Ok(()) + } + + #[test] + fn test_mem_access_read_multiple_bytes() -> crate::Result<()> { + let mut mem_access = get_mem_access()?; + let offset = 20; + + // Modify the memory directly to have a known value to read + { + let slice = unsafe { get_mmap_slice(&mut mem_access) }; + for i in 0..16 { + slice[offset + i] = i as u8; + } + } + + let mut read_data = [0u8; 16]; + mem_access.read(&mut read_data, (BASE_VIRT + offset) as u64)?; + + assert_eq!( + read_data, + [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15] + ); + drop_mem_access(mem_access); + Ok(()) + } + + #[test] + fn test_mem_access_write_single_byte() -> crate::Result<()> { + let mut mem_access = get_mem_access()?; + let offset = 3000; + { + let slice = unsafe { get_mmap_slice(&mut mem_access) }; + slice[offset] = 0xBB; + } + + let write_data = [0xCCu8; 1]; + mem_access.write(&write_data, (BASE_VIRT + offset) as u64)?; + + let slice = unsafe { get_mmap_slice(&mut mem_access) }; + assert_eq!(slice[offset], write_data[0]); + drop_mem_access(mem_access); + + Ok(()) + } + + #[test] + fn test_mem_access_write_multiple_bytes() -> crate::Result<()> { + let mut mem_access = get_mem_access()?; + let offset = 56; + { + let slice = unsafe { get_mmap_slice(&mut mem_access) }; + for i in 0..16 { + slice[offset + i] = i as u8; + } + } + + let write_data = [0xAAu8; 16]; + mem_access.write(&write_data, (BASE_VIRT + offset) as u64)?; + + let slice = unsafe { get_mmap_slice(&mut mem_access) }; + assert_eq!(slice[offset..offset + 16], write_data); + drop_mem_access(mem_access); + + Ok(()) + } + } } diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index 5a009823e..f0c425a46 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -63,7 +63,8 @@ use super::TraceRegister; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] use super::gdb::{ - DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, MshvDebug, VcpuStopReason, + DebugCommChannel, DebugMemoryAccess, DebugMsg, DebugResponse, GuestDebug, MshvDebug, + VcpuStopReason, }; #[cfg(feature = "init-paging")] use super::{ @@ -89,13 +90,11 @@ use crate::{Result, log_then_return, new_error}; #[cfg(gdb)] mod debug { - use std::sync::{Arc, Mutex}; - use super::mshv_bindings::hv_x64_exception_intercept_message; use super::{HypervLinuxDriver, *}; - use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs}; - use crate::mem::mgr::SandboxMemoryManager; - use crate::mem::shared_mem::HostSharedMemory; + use crate::hypervisor::gdb::{ + DebugMemoryAccess, DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs, + }; use crate::{Result, new_error}; impl HypervLinuxDriver { @@ -126,7 +125,7 @@ mod debug { pub(crate) fn process_dbg_request( &mut self, req: DebugMsg, - dbg_mem_access_fn: Arc>>, + mem_access: &DebugMemoryAccess, ) -> Result { if let Some(debug) = self.debug.as_mut() { match req { @@ -142,7 +141,7 @@ mod debug { )), DebugMsg::AddSwBreakpoint(addr) => Ok(DebugResponse::AddSwBreakpoint( debug - .add_sw_breakpoint(&self.vcpu_fd, addr, dbg_mem_access_fn) + .add_sw_breakpoint(&self.vcpu_fd, addr, mem_access) .map_err(|e| { log::error!("Failed to add sw breakpoint: {:?}", e); @@ -169,7 +168,8 @@ mod debug { Ok(DebugResponse::DisableDebug) } DebugMsg::GetCodeSectionOffset => { - let offset = dbg_mem_access_fn + let offset = mem_access + .dbg_mem_access_fn .try_lock() .map_err(|e| { new_error!("Error locking at {}:{}: {}", file!(), line!(), e) @@ -182,13 +182,7 @@ mod debug { DebugMsg::ReadAddr(addr, len) => { let mut data = vec![0u8; len]; - debug - .read_addrs(&self.vcpu_fd, addr, &mut data, dbg_mem_access_fn) - .map_err(|e| { - log::error!("Failed to read from address: {:?}", e); - - e - })?; + debug.read_addrs(&self.vcpu_fd, addr, &mut data, mem_access)?; Ok(DebugResponse::ReadAddr(data)) } @@ -216,7 +210,7 @@ mod debug { )), DebugMsg::RemoveSwBreakpoint(addr) => Ok(DebugResponse::RemoveSwBreakpoint( debug - .remove_sw_breakpoint(&self.vcpu_fd, addr, dbg_mem_access_fn) + .remove_sw_breakpoint(&self.vcpu_fd, addr, mem_access) .map_err(|e| { log::error!("Failed to remove sw breakpoint: {:?}", e); @@ -234,13 +228,7 @@ mod debug { Ok(DebugResponse::Step) } DebugMsg::WriteAddr(addr, data) => { - debug - .write_addrs(&self.vcpu_fd, addr, &data, dbg_mem_access_fn) - .map_err(|e| { - log::error!("Failed to write to address: {:?}", e); - - e - })?; + debug.write_addrs(&self.vcpu_fd, addr, &data, mem_access)?; Ok(DebugResponse::WriteAddr) } @@ -1027,6 +1015,11 @@ impl Hypervisor for HypervLinuxDriver { return Err(new_error!("Debugging is not enabled")); } + let mem_access = DebugMemoryAccess { + dbg_mem_access_fn, + guest_mmap_regions: self.mmap_regions.to_vec(), + }; + match stop_reason { // If the vCPU stopped because of a crash, we need to handle it differently // We do not want to allow resuming execution or placing breakpoints @@ -1070,7 +1063,7 @@ impl Hypervisor for HypervLinuxDriver { // For all other requests, we will process them normally _ => { - let result = self.process_dbg_request(req, dbg_mem_access_fn.clone()); + let result = self.process_dbg_request(req, &mem_access); match result { Ok(response) => response, Err(HyperlightError::TranslateGuestAddress(_)) => { @@ -1118,7 +1111,7 @@ impl Hypervisor for HypervLinuxDriver { // Wait for a message from gdb let req = self.recv_dbg_msg()?; - let result = self.process_dbg_request(req, dbg_mem_access_fn.clone()); + let result = self.process_dbg_request(req, &mem_access); let response = match result { Ok(response) => response, diff --git a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs index e9aa45703..7f6bb06d7 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs @@ -36,7 +36,8 @@ use {super::crashdump, std::path::Path}; #[cfg(gdb)] use { super::gdb::{ - DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, HypervDebug, VcpuStopReason, + DebugCommChannel, DebugMemoryAccess, DebugMsg, DebugResponse, GuestDebug, HypervDebug, + VcpuStopReason, }, crate::HyperlightError, }; @@ -71,15 +72,13 @@ use crate::{Result, debug, log_then_return, new_error}; #[cfg(gdb)] mod debug { - use std::sync::{Arc, Mutex}; - use windows::Win32::System::Hypervisor::WHV_VP_EXCEPTION_CONTEXT; use super::{HypervWindowsDriver, *}; use crate::Result; - use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs}; - use crate::mem::mgr::SandboxMemoryManager; - use crate::mem::shared_mem::HostSharedMemory; + use crate::hypervisor::gdb::{ + DebugMemoryAccess, DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs, + }; impl HypervWindowsDriver { /// Resets the debug information to disable debugging @@ -109,7 +108,7 @@ mod debug { pub(crate) fn process_dbg_request( &mut self, req: DebugMsg, - dbg_mem_access_fn: Arc>>, + mem_access: &DebugMemoryAccess, ) -> Result { if let Some(debug) = self.debug.as_mut() { match req { @@ -125,7 +124,7 @@ mod debug { )), DebugMsg::AddSwBreakpoint(addr) => Ok(DebugResponse::AddSwBreakpoint( debug - .add_sw_breakpoint(&self.processor, addr, dbg_mem_access_fn) + .add_sw_breakpoint(&self.processor, addr, mem_access) .map_err(|e| { log::error!("Failed to add sw breakpoint: {:?}", e); @@ -152,7 +151,8 @@ mod debug { Ok(DebugResponse::DisableDebug) } DebugMsg::GetCodeSectionOffset => { - let offset = dbg_mem_access_fn + let offset = mem_access + .dbg_mem_access_fn .try_lock() .map_err(|e| { new_error!("Error locking at {}:{}: {}", file!(), line!(), e) @@ -166,7 +166,7 @@ mod debug { let mut data = vec![0u8; len]; debug - .read_addrs(&self.processor, addr, &mut data, dbg_mem_access_fn) + .read_addrs(&self.processor, addr, &mut data, mem_access) .map_err(|e| { log::error!("Failed to read from address: {:?}", e); @@ -199,7 +199,7 @@ mod debug { )), DebugMsg::RemoveSwBreakpoint(addr) => Ok(DebugResponse::RemoveSwBreakpoint( debug - .remove_sw_breakpoint(&self.processor, addr, dbg_mem_access_fn) + .remove_sw_breakpoint(&self.processor, addr, mem_access) .map_err(|e| { log::error!("Failed to remove sw breakpoint: {:?}", e); @@ -218,7 +218,7 @@ mod debug { } DebugMsg::WriteAddr(addr, data) => { debug - .write_addrs(&self.processor, addr, &data, dbg_mem_access_fn) + .write_addrs(&self.processor, addr, &data, mem_access) .map_err(|e| { log::error!("Failed to write to address: {:?}", e); @@ -960,6 +960,12 @@ impl Hypervisor for HypervWindowsDriver { if self.debug.is_none() { return Err(new_error!("Debugging is not enabled")); } + + let mem_access = DebugMemoryAccess { + dbg_mem_access_fn, + guest_mmap_regions: self.mmap_regions.to_vec(), + }; + match stop_reason { // If the vCPU stopped because of a crash, we need to handle it differently // We do not want to allow resuming execution or placing breakpoints @@ -1003,7 +1009,7 @@ impl Hypervisor for HypervWindowsDriver { // For all other requests, we will process them normally _ => { - let result = self.process_dbg_request(req, dbg_mem_access_fn.clone()); + let result = self.process_dbg_request(req, &mem_access); match result { Ok(response) => response, Err(HyperlightError::TranslateGuestAddress(_)) => { @@ -1052,7 +1058,7 @@ impl Hypervisor for HypervWindowsDriver { // Wait for a message from gdb let req = self.recv_dbg_msg()?; - let result = self.process_dbg_request(req, dbg_mem_access_fn.clone()); + let result = self.process_dbg_request(req, &mem_access); let response = match result { Ok(response) => response, diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 36a7066b6..11fce074b 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -31,7 +31,10 @@ use {super::crashdump, std::path::Path}; use super::TraceRegister; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] -use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason}; +use super::gdb::{ + DebugCommChannel, DebugMemoryAccess, DebugMsg, DebugResponse, GuestDebug, KvmDebug, + VcpuStopReason, +}; #[cfg(feature = "init-paging")] use super::{ CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, CR4_OSFXSR, CR4_OSXMMEXCPT, CR4_PAE, @@ -78,16 +81,13 @@ pub(crate) fn is_hypervisor_present() -> bool { #[cfg(gdb)] mod debug { - use std::sync::{Arc, Mutex}; - use kvm_bindings::kvm_debug_exit_arch; use super::KVMDriver; use crate::hypervisor::gdb::{ - DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason, X86_64Regs, + DebugMemoryAccess, DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason, + X86_64Regs, }; - use crate::mem::mgr::SandboxMemoryManager; - use crate::mem::shared_mem::HostSharedMemory; use crate::{Result, new_error}; impl KVMDriver { @@ -118,7 +118,7 @@ mod debug { pub(crate) fn process_dbg_request( &mut self, req: DebugMsg, - dbg_mem_access_fn: Arc>>, + mem_access: &DebugMemoryAccess, ) -> Result { if let Some(debug) = self.debug.as_mut() { match req { @@ -134,7 +134,7 @@ mod debug { )), DebugMsg::AddSwBreakpoint(addr) => Ok(DebugResponse::AddSwBreakpoint( debug - .add_sw_breakpoint(&self.vcpu_fd, addr, dbg_mem_access_fn) + .add_sw_breakpoint(&self.vcpu_fd, addr, mem_access) .map_err(|e| { log::error!("Failed to add sw breakpoint: {:?}", e); @@ -161,7 +161,8 @@ mod debug { Ok(DebugResponse::DisableDebug) } DebugMsg::GetCodeSectionOffset => { - let offset = dbg_mem_access_fn + let offset = mem_access + .dbg_mem_access_fn .try_lock() .map_err(|e| { new_error!("Error locking at {}:{}: {}", file!(), line!(), e) @@ -174,13 +175,7 @@ mod debug { DebugMsg::ReadAddr(addr, len) => { let mut data = vec![0u8; len]; - debug - .read_addrs(&self.vcpu_fd, addr, &mut data, dbg_mem_access_fn) - .map_err(|e| { - log::error!("Failed to read from address: {:?}", e); - - e - })?; + debug.read_addrs(&self.vcpu_fd, addr, &mut data, mem_access)?; Ok(DebugResponse::ReadAddr(data)) } @@ -208,7 +203,7 @@ mod debug { )), DebugMsg::RemoveSwBreakpoint(addr) => Ok(DebugResponse::RemoveSwBreakpoint( debug - .remove_sw_breakpoint(&self.vcpu_fd, addr, dbg_mem_access_fn) + .remove_sw_breakpoint(&self.vcpu_fd, addr, mem_access) .map_err(|e| { log::error!("Failed to remove sw breakpoint: {:?}", e); @@ -226,13 +221,7 @@ mod debug { Ok(DebugResponse::Step) } DebugMsg::WriteAddr(addr, data) => { - debug - .write_addrs(&self.vcpu_fd, addr, &data, dbg_mem_access_fn) - .map_err(|e| { - log::error!("Failed to write to address: {:?}", e); - - e - })?; + debug.write_addrs(&self.vcpu_fd, addr, &data, mem_access)?; Ok(DebugResponse::WriteAddr) } @@ -907,6 +896,11 @@ impl Hypervisor for KVMDriver { return Err(new_error!("Debugging is not enabled")); } + let mem_access = DebugMemoryAccess { + dbg_mem_access_fn, + guest_mmap_regions: self.mmap_regions.iter().map(|(r, _)| r.clone()).collect(), + }; + match stop_reason { // If the vCPU stopped because of a crash, we need to handle it differently // We do not want to allow resuming execution or placing breakpoints @@ -950,7 +944,7 @@ impl Hypervisor for KVMDriver { // For all other requests, we will process them normally _ => { - let result = self.process_dbg_request(req, dbg_mem_access_fn.clone()); + let result = self.process_dbg_request(req, &mem_access); match result { Ok(response) => response, Err(HyperlightError::TranslateGuestAddress(_)) => { @@ -998,7 +992,7 @@ impl Hypervisor for KVMDriver { // Wait for a message from gdb let req = self.recv_dbg_msg()?; - let result = self.process_dbg_request(req, dbg_mem_access_fn.clone()); + let result = self.process_dbg_request(req, &mem_access); let response = match result { Ok(response) => response, From d4fc726da64452b0d5669892f67eb8eeecce298f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 7 Oct 2025 20:17:05 +0300 Subject: [PATCH 2/2] [crashdump] Fix mmaped memory not included in dump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- src/hyperlight_host/src/hypervisor/crashdump.rs | 14 +++++++------- src/hyperlight_host/src/hypervisor/hyperv_linux.rs | 7 +++++-- .../src/hypervisor/hyperv_windows.rs | 7 +++++-- src/hyperlight_host/src/hypervisor/kvm.rs | 7 +++++-- src/hyperlight_host/src/hypervisor/mod.rs | 2 +- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/crashdump.rs b/src/hyperlight_host/src/hypervisor/crashdump.rs index ae865ef56..86dce75ff 100644 --- a/src/hyperlight_host/src/hypervisor/crashdump.rs +++ b/src/hyperlight_host/src/hypervisor/crashdump.rs @@ -43,8 +43,8 @@ const CORE_DUMP_PAGE_SIZE: usize = 0x1000; /// Structure to hold the crash dump context /// This structure contains the information needed to create a core dump #[derive(Debug)] -pub(crate) struct CrashDumpContext<'a> { - regions: &'a [MemoryRegion], +pub(crate) struct CrashDumpContext { + regions: Vec, regs: [u64; 27], xsave: Vec, entry: u64, @@ -52,9 +52,9 @@ pub(crate) struct CrashDumpContext<'a> { filename: Option, } -impl<'a> CrashDumpContext<'a> { +impl CrashDumpContext { pub(crate) fn new( - regions: &'a [MemoryRegion], + regions: Vec, regs: [u64; 27], xsave: Vec, entry: u64, @@ -208,7 +208,7 @@ struct GuestMemReader { impl GuestMemReader { fn new(ctx: &CrashDumpContext) -> Self { Self { - regions: ctx.regions.to_vec(), + regions: ctx.regions.clone(), } } } @@ -440,7 +440,7 @@ mod test { fn test_crashdump_write_fails_when_no_regions() { // Create a dummy context let ctx = CrashDumpContext::new( - &[], + vec![], [0; 27], vec![], 0, @@ -471,7 +471,7 @@ mod test { }]; // Create a dummy context let ctx = CrashDumpContext::new( - ®ions, + regions, [0; 27], vec![], 0x1000, diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index f0c425a46..0aeb3d775 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -948,7 +948,7 @@ impl Hypervisor for HypervLinuxDriver { } #[cfg(crashdump)] - fn crashdump_context(&self) -> Result>> { + fn crashdump_context(&self) -> Result> { if self.rt_cfg.guest_core_dump { let mut regs = [0; 27]; @@ -992,8 +992,11 @@ impl Hypervisor for HypervLinuxDriver { .and_then(|name| name.to_os_string().into_string().ok()) }); + // Include both initial sandbox regions and dynamically mapped regions + let mut regions: Vec = self.sandbox_regions.clone(); + regions.extend(self.mmap_regions.iter().cloned()); Ok(Some(crashdump::CrashDumpContext::new( - &self.sandbox_regions, + regions, regs, xsave.buffer.to_vec(), self.entrypoint, diff --git a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs index 7f6bb06d7..8c5ff26cd 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs @@ -894,7 +894,7 @@ impl Hypervisor for HypervWindowsDriver { } #[cfg(crashdump)] - fn crashdump_context(&self) -> Result>> { + fn crashdump_context(&self) -> Result> { if self.rt_cfg.guest_core_dump { let mut regs = [0; 27]; @@ -938,8 +938,11 @@ impl Hypervisor for HypervWindowsDriver { .and_then(|name| name.to_os_string().into_string().ok()) }); + // Include both initial sandbox regions and dynamically mapped regions + let mut regions: Vec = self.sandbox_regions.clone(); + regions.extend(self.mmap_regions.iter().cloned()); Ok(Some(crashdump::CrashDumpContext::new( - &self.sandbox_regions, + regions, regs, xsave, self.entrypoint, diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 11fce074b..dbbacae09 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -823,7 +823,7 @@ impl Hypervisor for KVMDriver { } #[cfg(crashdump)] - fn crashdump_context(&self) -> Result>> { + fn crashdump_context(&self) -> Result> { if self.rt_cfg.guest_core_dump { let mut regs = [0; 27]; @@ -869,8 +869,11 @@ impl Hypervisor for KVMDriver { // The [`CrashDumpContext`] accepts xsave as a vector of u8, so we need to convert the // xsave region to a vector of u8 + // Also include mapped regions in addition to the initial sandbox regions + let mut regions: Vec = self.sandbox_regions.clone(); + regions.extend(self.mmap_regions.iter().map(|(r, _)| r.clone())); Ok(Some(crashdump::CrashDumpContext::new( - &self.sandbox_regions, + regions, regs, xsave .region diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index 9c04ad3a0..62c75b5e9 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -230,7 +230,7 @@ pub(crate) trait Hypervisor: Debug + Send { fn as_mut_hypervisor(&mut self) -> &mut dyn Hypervisor; #[cfg(crashdump)] - fn crashdump_context(&self) -> Result>>; + fn crashdump_context(&self) -> Result>; #[cfg(gdb)] /// handles the cases when the vCPU stops due to a Debug event