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/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index d6333e456..4c57958e5 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; @@ -48,6 +48,7 @@ use x86_64_target::HyperlightSandboxTarget; use super::InterruptHandle; use crate::hypervisor::regs::{CommonFpu, CommonRegisters}; 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}; @@ -89,6 +90,150 @@ impl From for TargetError { } } +/// 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 { @@ -193,7 +338,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)?; @@ -203,8 +348,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); @@ -218,7 +363,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); @@ -230,20 +375,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; @@ -267,7 +400,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)?; @@ -277,7 +410,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 { @@ -291,7 +424,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); @@ -303,20 +436,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; @@ -441,4 +563,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 80ee0b16b..083a2fb8e 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -55,7 +55,8 @@ use {super::crashdump, std::path::Path}; #[cfg(gdb)] use super::gdb::{ - DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, MshvDebug, VcpuStopReason, + DebugCommChannel, DebugMemoryAccess, DebugMsg, DebugResponse, GuestDebug, MshvDebug, + VcpuStopReason, }; use super::{HyperlightExit, Hypervisor, InterruptHandle, LinuxInterruptHandle, VirtualCPU}; #[cfg(gdb)] @@ -77,13 +78,9 @@ 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}; - use crate::mem::mgr::SandboxMemoryManager; - use crate::mem::shared_mem::HostSharedMemory; + use crate::hypervisor::gdb::{DebugMemoryAccess, DebugMsg, DebugResponse, VcpuStopReason}; use crate::{Result, new_error}; impl HypervLinuxDriver { @@ -114,7 +111,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 { @@ -130,7 +127,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); @@ -157,7 +154,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) @@ -170,13 +168,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)) } @@ -200,7 +192,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); @@ -218,13 +210,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) } @@ -901,7 +887,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]; @@ -945,8 +931,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, @@ -968,6 +957,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 @@ -1011,7 +1005,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(_)) => { @@ -1059,7 +1053,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 490e1f72e..eeb4e6928 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs @@ -31,7 +31,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, }; @@ -58,15 +59,11 @@ 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}; - use crate::mem::mgr::SandboxMemoryManager; - use crate::mem::shared_mem::HostSharedMemory; + use crate::hypervisor::gdb::{DebugMemoryAccess, DebugMsg, DebugResponse, VcpuStopReason}; impl HypervWindowsDriver { /// Resets the debug information to disable debugging @@ -96,7 +93,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 { @@ -112,7 +109,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); @@ -139,7 +136,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) @@ -153,7 +151,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); @@ -182,7 +180,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); @@ -201,7 +199,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); @@ -735,7 +733,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]; @@ -779,8 +777,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, @@ -801,6 +802,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 @@ -844,7 +851,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(_)) => { @@ -893,7 +900,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 a42538b33..a5ee24e21 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -27,7 +27,10 @@ use tracing::{Span, instrument}; use {super::crashdump, std::path::Path}; #[cfg(gdb)] -use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason}; +use super::gdb::{ + DebugCommChannel, DebugMemoryAccess, DebugMsg, DebugResponse, GuestDebug, KvmDebug, + VcpuStopReason, +}; use super::{HyperlightExit, Hypervisor, InterruptHandle, LinuxInterruptHandle, VirtualCPU}; #[cfg(gdb)] use crate::HyperlightError; @@ -70,14 +73,12 @@ 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}; - use crate::mem::mgr::SandboxMemoryManager; - use crate::mem::shared_mem::HostSharedMemory; + use crate::hypervisor::gdb::{ + DebugMemoryAccess, DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason, + }; use crate::{Result, new_error}; impl KVMDriver { @@ -108,7 +109,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 { @@ -124,7 +125,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); @@ -151,7 +152,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) @@ -164,13 +166,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)) } @@ -194,7 +190,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); @@ -212,13 +208,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) } @@ -827,7 +817,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]; @@ -873,8 +863,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 @@ -900,6 +893,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 @@ -943,7 +941,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(_)) => { @@ -991,7 +989,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, diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index 781099134..7ad183ba9 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -298,7 +298,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 diff --git a/src/tests/rust_guests/dummyguest/Cargo.lock b/src/tests/rust_guests/dummyguest/Cargo.lock index 4baf80060..1d9d3b354 100644 --- a/src/tests/rust_guests/dummyguest/Cargo.lock +++ b/src/tests/rust_guests/dummyguest/Cargo.lock @@ -254,9 +254,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.106" +version = "2.0.107" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ede7c438028d4436d71104916910f5bb611972c5cfd7f89b8300a8186e6fada6" +checksum = "2a26dbd934e5451d21ef060c018dae56fc073894c5a7896f882928a76e6d081b" dependencies = [ "proc-macro2", "quote", diff --git a/src/tests/rust_guests/simpleguest/Cargo.lock b/src/tests/rust_guests/simpleguest/Cargo.lock index 076ef55f3..26e526c37 100644 --- a/src/tests/rust_guests/simpleguest/Cargo.lock +++ b/src/tests/rust_guests/simpleguest/Cargo.lock @@ -257,9 +257,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.106" +version = "2.0.107" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ede7c438028d4436d71104916910f5bb611972c5cfd7f89b8300a8186e6fada6" +checksum = "2a26dbd934e5451d21ef060c018dae56fc073894c5a7896f882928a76e6d081b" dependencies = [ "proc-macro2", "quote", diff --git a/src/tests/rust_guests/witguest/Cargo.lock b/src/tests/rust_guests/witguest/Cargo.lock index 281181180..3a0f074ae 100644 --- a/src/tests/rust_guests/witguest/Cargo.lock +++ b/src/tests/rust_guests/witguest/Cargo.lock @@ -499,9 +499,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.106" +version = "2.0.107" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ede7c438028d4436d71104916910f5bb611972c5cfd7f89b8300a8186e6fada6" +checksum = "2a26dbd934e5451d21ef060c018dae56fc073894c5a7896f882928a76e6d081b" dependencies = [ "proc-macro2", "quote",