From e14ac69b7f648ada87b8ded42d5e696a99f79f90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Mon, 28 Apr 2025 16:56:27 +0300 Subject: [PATCH 1/2] crashdump: create core dump file when a guest crashes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - the core dump file is an ELF file with special segments that describe the guest's memory when it crashed, the CPU register's values and other special notes that tell the debugger how to set up a debugging session starting from the core dump Signed-off-by: Doru Blânzeanu --- Cargo.lock | 25 ++ src/hyperlight_host/Cargo.toml | 1 + .../src/hypervisor/crashdump.rs | 265 ++++++++++++++++-- .../src/hypervisor/hyperv_linux.rs | 46 ++- .../src/hypervisor/hyperv_windows.rs | 46 ++- .../src/hypervisor/inprocess.rs | 8 +- src/hyperlight_host/src/hypervisor/kvm.rs | 53 +++- src/hyperlight_host/src/hypervisor/mod.rs | 2 +- .../hypervisor/windows_hypervisor_platform.rs | 55 ++++ 9 files changed, 464 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4a552ce14..0854e81e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -666,6 +666,19 @@ version = "1.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "48c757948c5ede0e46177b7add2e67155f70e33c07fea8284df6576da70b3719" +[[package]] +name = "elfcore" +version = "1.1.5" +source = "git+https://github.com/dblnz/elfcore.git?branch=split-linux-impl-from-elfcore#d3152cb3cbeee5f72b1b2ee8fd3b4c1ecf9ed5a7" +dependencies = [ + "libc", + "nix", + "smallvec", + "thiserror 1.0.69", + "tracing", + "zerocopy 0.7.35", +] + [[package]] name = "endian-type" version = "0.1.2" @@ -1207,6 +1220,7 @@ dependencies = [ "crossbeam", "crossbeam-channel", "crossbeam-queue", + "elfcore", "env_logger", "flatbuffers", "gdbstub", @@ -1902,6 +1916,17 @@ dependencies = [ "smallvec", ] +[[package]] +name = "nix" +version = "0.26.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "598beaf3cc6fdd9a5dfb1630c2800c7acd31df7aaf0f565796fba2b53ca1af1b" +dependencies = [ + "bitflags 1.3.2", + "cfg-if", + "libc", +] + [[package]] name = "nom" version = "7.1.3" diff --git a/src/hyperlight_host/Cargo.toml b/src/hyperlight_host/Cargo.toml index 5fdb74cc0..6fdf42b81 100644 --- a/src/hyperlight_host/Cargo.toml +++ b/src/hyperlight_host/Cargo.toml @@ -46,6 +46,7 @@ tempfile = { version = "3.19", optional = true } serde_yaml = "0.9" anyhow = "1.0" metrics = "0.24.2" +elfcore = { git = "https://github.com/dblnz/elfcore.git", branch = "split-linux-impl-from-elfcore" } [target.'cfg(windows)'.dependencies] windows = { version = "0.61", features = [ diff --git a/src/hyperlight_host/src/hypervisor/crashdump.rs b/src/hyperlight_host/src/hypervisor/crashdump.rs index a70dc34c1..72673c67a 100644 --- a/src/hyperlight_host/src/hypervisor/crashdump.rs +++ b/src/hyperlight_host/src/hypervisor/crashdump.rs @@ -1,44 +1,257 @@ -use std::io::Write; +use std::cmp::min; +use elfcore::{ + ArchComponentState, ArchState, CoreDumpBuilder, CoreError, Elf64_Auxv, ProcessInfoSource, + ReadProcessMemory, ThreadView, VaProtection, VaRegion, +}; use tempfile::NamedTempFile; use super::Hypervisor; +use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; use crate::{new_error, Result}; -/// Dump registers + memory regions + raw memory to a tempfile -#[cfg(crashdump)] -pub(crate) fn crashdump_to_tempfile(hv: &dyn Hypervisor) -> Result<()> { - let mut temp_file = NamedTempFile::with_prefix("mem")?; - let hv_details = format!("{:#x?}", hv); +const NT_X86_XSTATE: u32 = 0x202; +const AT_ENTRY: u64 = 9; +const AT_NULL: u64 = 0; + +/// 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], + regs: [u64; 27], + xsave: Vec, + entry: u64, +} + +impl<'a> CrashDumpContext<'a> { + pub(crate) fn new( + regions: &'a [MemoryRegion], + regs: [u64; 27], + xsave: Vec, + entry: u64, + ) -> Self { + Self { + regions, + regs, + xsave, + entry, + } + } +} + +/// Structure that contains the process information for the core dump +/// This serves as a source of information for `elfcore`'s [`CoreDumpBuilder`] +struct GuestView { + regions: Vec, + threads: Vec, + aux_vector: Vec, +} - // write hypervisor details such as registers, info about mapped memory regions, etc. - temp_file.write_all(hv_details.as_bytes())?; - temp_file.write_all(b"================ MEMORY DUMP =================\n")?; +impl GuestView { + fn new(ctx: &CrashDumpContext) -> Self { + // Map the regions to the format `CoreDumpBuilder` expects + let regions = ctx + .regions + .iter() + .filter(|r| !r.host_region.is_empty()) + .map(|r| VaRegion { + begin: r.guest_region.start as u64, + end: r.guest_region.end as u64, + offset: r.host_region.start as u64, + protection: VaProtection { + is_private: false, + read: r.flags.contains(MemoryRegionFlags::READ), + write: r.flags.contains(MemoryRegionFlags::WRITE), + execute: r.flags.contains(MemoryRegionFlags::EXECUTE), + }, + mapped_file_name: None, + }) + .collect(); - // write the raw memory dump for each memory region - for region in hv.get_memory_regions() { - if region.host_region.start == 0 || region.host_region.is_empty() { - continue; + // The xsave state is checked as it can be empty + let mut components = vec![]; + if !ctx.xsave.is_empty() { + components.push(ArchComponentState { + name: "XSAVE", + note_type: NT_X86_XSTATE, + note_name: b"LINUX", + data: ctx.xsave.clone(), + }); } - // SAFETY: we got this memory region from the hypervisor so should never be invalid - let region_slice = unsafe { - std::slice::from_raw_parts( - region.host_region.start as *const u8, - region.host_region.len(), - ) + + // Create the thread view + // The thread view contains the information about the thread + // NOTE: Some of these fields are not used in the current implementation + let thread = ThreadView { + flags: 0, // Kernel flags for the process + tid: 1, + uid: 0, // User ID + gid: 0, // Group ID + comm: "\0".to_string(), + ppid: 0, // Parent PID + pgrp: 0, // Process group ID + nice: 0, // Nice value + state: 0, // Process state + utime: 0, // User time + stime: 0, // System time + cutime: 0, // Children User time + cstime: 0, // Children User time + cursig: 0, // Current signal + session: 0, // Session ID of the process + sighold: 0, // Blocked signal + sigpend: 0, // Pending signal + cmd_line: "\0".to_string(), + + arch_state: Box::new(ArchState { + gpr_state: ctx.regs.to_vec(), + components, + }), }; - temp_file.write_all(region_slice)?; + + // Create the auxv vector + // The first entry is AT_ENTRY, which is the entry point of the program + // The entry point is the address where the program starts executing + // This helps the debugger to know that the entry is changed by an offset + // so the symbols can be loaded correctly. + // The second entry is AT_NULL, which marks the end of the vector + let auxv = vec![ + Elf64_Auxv { + a_type: AT_ENTRY, + a_val: ctx.entry, + }, + Elf64_Auxv { + a_type: AT_NULL, + a_val: 0, + }, + ]; + + Self { + regions, + threads: vec![thread], + aux_vector: auxv, + } + } +} + +impl ProcessInfoSource for GuestView { + fn pid(&self) -> i32 { + 1 + } + fn threads(&self) -> &[elfcore::ThreadView] { + &self.threads } - temp_file.flush()?; + fn page_size(&self) -> usize { + 0x1000 + } + fn aux_vector(&self) -> Option<&[elfcore::Elf64_Auxv]> { + Some(&self.aux_vector) + } + fn va_regions(&self) -> &[elfcore::VaRegion] { + &self.regions + } + fn mapped_files(&self) -> Option<&[elfcore::MappedFile]> { + None + } +} - // persist the tempfile to disk - let persist_path = temp_file.path().with_extension("dmp"); +/// Structure that reads the guest memory +/// This structure serves as a custom memory reader for `elfcore`'s +/// [`CoreDumpBuilder`] +struct GuestMemReader { + regions: Vec, +} + +impl GuestMemReader { + fn new(ctx: &CrashDumpContext) -> Self { + Self { + regions: ctx.regions.to_vec(), + } + } +} + +impl ReadProcessMemory for GuestMemReader { + fn read_process_memory( + &mut self, + base: usize, + buf: &mut [u8], + ) -> std::result::Result { + for r in self.regions.iter() { + // Check if the base address is within the guest region + if base >= r.guest_region.start && base < r.guest_region.end { + let offset = base - r.guest_region.start; + let region_slice = unsafe { + std::slice::from_raw_parts( + r.host_region.start as *const u8, + r.host_region.len(), + ) + }; + + // Calculate how much we can copy + let copy_size = min(buf.len(), region_slice.len() - offset); + if copy_size == 0 { + return std::result::Result::Ok(0); + } + + // Only copy the amount that fits in both buffers + buf[..copy_size].copy_from_slice(®ion_slice[offset..offset + copy_size]); + + // Return the number of bytes copied + return std::result::Result::Ok(copy_size); + } + } + + // If we reach here, we didn't find a matching region + std::result::Result::Ok(0) + } +} + +/// Create core dump file from the hypervisor information +/// +/// This function generates an ELF core dump file capturing the hypervisor's state, +/// which can be used for debugging when crashes occur. The file is created in the +/// system's temporary directory with extension '.elf' and the path is printed to stdout and logs. +/// +/// # Arguments +/// * `hv`: Reference to the hypervisor implementation +/// +/// # Returns +/// * `Result<()>`: Success or error +pub(crate) fn crashdump_to_tempfile(hv: &dyn Hypervisor) -> Result<()> { + log::info!("Creating core dump file..."); + + // Create a temporary file with a recognizable prefix + let temp_file = NamedTempFile::with_prefix("hl_core_") + .map_err(|e| new_error!("Failed to create temporary file: {:?}", e))?; + + // Get crash context from hypervisor + let ctx = hv + .crashdump_context() + .map_err(|e| new_error!("Failed to get crashdump context: {:?}", e))?; + + // Set up data sources for the core dump + let guest_view = GuestView::new(&ctx); + let memory_reader = GuestMemReader::new(&ctx); + + // Create and write core dump + let core_builder = CoreDumpBuilder::from_source( + Box::new(guest_view) as Box, + Box::new(memory_reader) as Box, + ); + + core_builder + .write(&temp_file) + .map_err(|e| new_error!("Failed to write core dump: {:?}", e))?; + + let persist_path = temp_file.path().with_extension("elf"); temp_file .persist(&persist_path) - .map_err(|e| new_error!("Failed to persist crashdump file: {:?}", e))?; + .map_err(|e| new_error!("Failed to persist core dump file: {:?}", e))?; + + let path_string = persist_path.to_string_lossy().to_string(); - println!("Memory dumped to file: {:?}", persist_path); - log::error!("Memory dumped to file: {:?}", persist_path); + println!("Core dump created successfully: {}", path_string); + log::error!("Core dump file: {}", path_string); Ok(()) } diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index 8419cb073..f017b39b6 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -49,6 +49,8 @@ use mshv_bindings::{ use mshv_ioctls::{Mshv, VcpuFd, VmFd}; use tracing::{instrument, Span}; +#[cfg(crashdump)] +use super::crashdump; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, MshvDebug}; @@ -658,8 +660,48 @@ impl Hypervisor for HypervLinuxDriver { } #[cfg(crashdump)] - fn get_memory_regions(&self) -> &[MemoryRegion] { - &self.mem_regions + fn crashdump_context(&self) -> Result { + let mut regs = [0; 27]; + + let vcpu_regs = self.vcpu_fd.get_regs()?; + let sregs = self.vcpu_fd.get_sregs()?; + let xsave = self.vcpu_fd.get_xsave()?; + + // Set up the registers for the crash dump + regs[0] = vcpu_regs.r15; // r15 + regs[1] = vcpu_regs.r14; // r14 + regs[2] = vcpu_regs.r13; // r13 + regs[3] = vcpu_regs.r12; // r12 + regs[4] = vcpu_regs.rbp; // rbp + regs[5] = vcpu_regs.rbx; // rbx + regs[6] = vcpu_regs.r11; // r11 + regs[7] = vcpu_regs.r10; // r10 + regs[8] = vcpu_regs.r9; // r9 + regs[9] = vcpu_regs.r8; // r8 + regs[10] = vcpu_regs.rax; // rax + regs[11] = vcpu_regs.rcx; // rcx + regs[12] = vcpu_regs.rdx; // rdx + regs[13] = vcpu_regs.rsi; // rsi + regs[14] = vcpu_regs.rdi; // rdi + regs[15] = 0; // orig rax + regs[16] = vcpu_regs.rip; // rip + regs[17] = sregs.cs.selector as u64; // cs + regs[18] = vcpu_regs.rflags; // eflags + regs[19] = vcpu_regs.rsp; // rsp + regs[20] = sregs.ss.selector as u64; // ss + regs[21] = sregs.fs.base; // fs_base + regs[22] = sregs.gs.base; // gs_base + regs[23] = sregs.ds.selector as u64; // ds + regs[24] = sregs.es.selector as u64; // es + regs[25] = sregs.fs.selector as u64; // fs + regs[26] = sregs.gs.selector as u64; // gs + + Ok(crashdump::CrashDumpContext::new( + &self.mem_regions, + regs, + xsave.buffer.to_vec(), + self.entrypoint, + )) } #[cfg(gdb)] diff --git a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs index 47fbaaa9d..d6d2f63c6 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs @@ -28,6 +28,8 @@ use windows::Win32::System::Hypervisor::{ WHV_RUN_VP_EXIT_REASON, WHV_X64_SEGMENT_REGISTER, WHV_X64_SEGMENT_REGISTER_0, }; +#[cfg(crashdump)] +use super::crashdump; use super::fpu::{FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] use super::handlers::DbgMemAccessHandlerWrapper; @@ -487,8 +489,48 @@ impl Hypervisor for HypervWindowsDriver { } #[cfg(crashdump)] - fn get_memory_regions(&self) -> &[MemoryRegion] { - &self.mem_regions + fn crashdump_context(&self) -> Result { + let mut regs = [0; 27]; + + let vcpu_regs = self.processor.get_regs()?; + let sregs = self.processor.get_sregs()?; + let xsave = self.processor.get_xsave()?; + + // Set the registers in the order expected by the crashdump context + regs[0] = vcpu_regs.r15; // r15 + regs[1] = vcpu_regs.r14; // r14 + regs[2] = vcpu_regs.r13; // r13 + regs[3] = vcpu_regs.r12; // r12 + regs[4] = vcpu_regs.rbp; // rbp + regs[5] = vcpu_regs.rbx; // rbx + regs[6] = vcpu_regs.r11; // r11 + regs[7] = vcpu_regs.r10; // r10 + regs[8] = vcpu_regs.r9; // r9 + regs[9] = vcpu_regs.r8; // r8 + regs[10] = vcpu_regs.rax; // rax + regs[11] = vcpu_regs.rcx; // rcx + regs[12] = vcpu_regs.rdx; // rdx + regs[13] = vcpu_regs.rsi; // rsi + regs[14] = vcpu_regs.rdi; // rdi + regs[15] = 0; // orig rax + regs[16] = vcpu_regs.rip; // rip + regs[17] = unsafe { sregs.cs.Segment.Selector } as u64; // cs + regs[18] = vcpu_regs.rflags; // eflags + regs[19] = vcpu_regs.rsp; // rsp + regs[20] = unsafe { sregs.ss.Segment.Selector } as u64; // ss + regs[21] = unsafe { sregs.fs.Segment.Base }; // fs_base + regs[22] = unsafe { sregs.gs.Segment.Base }; // gs_base + regs[23] = unsafe { sregs.ds.Segment.Selector } as u64; // ds + regs[24] = unsafe { sregs.es.Segment.Selector } as u64; // es + regs[25] = unsafe { sregs.fs.Segment.Selector } as u64; // fs + regs[26] = unsafe { sregs.gs.Segment.Selector } as u64; // gs + + Ok(crashdump::CrashDumpContext::new( + &self.mem_regions, + regs, + xsave, + self.entrypoint, + )) } } diff --git a/src/hyperlight_host/src/hypervisor/inprocess.rs b/src/hyperlight_host/src/hypervisor/inprocess.rs index 0a87c30d2..ad9cbe5d7 100644 --- a/src/hyperlight_host/src/hypervisor/inprocess.rs +++ b/src/hyperlight_host/src/hypervisor/inprocess.rs @@ -19,11 +19,11 @@ use std::os::raw::c_void; use log::LevelFilter; +#[cfg(crashdump)] +use super::crashdump; #[cfg(gdb)] use super::handlers::DbgMemAccessHandlerWrapper; use super::{HyperlightExit, Hypervisor}; -#[cfg(crashdump)] -use crate::mem::memory_region::MemoryRegion; use crate::sandbox::leaked_outb::LeakedOutBWrapper; use crate::Result; @@ -134,7 +134,7 @@ impl<'a> Hypervisor for InprocessDriver<'a> { } #[cfg(crashdump)] - fn get_memory_regions(&self) -> &[MemoryRegion] { - unimplemented!("get_memory_regions is not supported since we are in in-process mode") + fn crashdump_context(&self) -> Result { + unimplemented!("crashdump_context is not supported since we are in in-process mode"); } } diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 967555ffc..a899d285b 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -25,6 +25,8 @@ use kvm_ioctls::{Kvm, VcpuExit, VcpuFd, VmFd}; use log::LevelFilter; use tracing::{instrument, Span}; +#[cfg(crashdump)] +use super::crashdump; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason}; @@ -582,8 +584,55 @@ impl Hypervisor for KVMDriver { } #[cfg(crashdump)] - fn get_memory_regions(&self) -> &[MemoryRegion] { - &self.mem_regions + fn crashdump_context(&self) -> Result { + let mut regs = [0; 27]; + + let vcpu_regs = self.vcpu_fd.get_regs()?; + let sregs = self.vcpu_fd.get_sregs()?; + let xsave = self.vcpu_fd.get_xsave()?; + + // Set the registers in the order expected by the crashdump context + regs[0] = vcpu_regs.r15; // r15 + regs[1] = vcpu_regs.r14; // r14 + regs[2] = vcpu_regs.r13; // r13 + regs[3] = vcpu_regs.r12; // r12 + regs[4] = vcpu_regs.rbp; // rbp + regs[5] = vcpu_regs.rbx; // rbx + regs[6] = vcpu_regs.r11; // r11 + regs[7] = vcpu_regs.r10; // r10 + regs[8] = vcpu_regs.r9; // r9 + regs[9] = vcpu_regs.r8; // r8 + regs[10] = vcpu_regs.rax; // rax + regs[11] = vcpu_regs.rcx; // rcx + regs[12] = vcpu_regs.rdx; // rdx + regs[13] = vcpu_regs.rsi; // rsi + regs[14] = vcpu_regs.rdi; // rdi + regs[15] = 0; // orig rax + regs[16] = vcpu_regs.rip; // rip + regs[17] = sregs.cs.selector as u64; // cs + regs[18] = vcpu_regs.rflags; // eflags + regs[19] = vcpu_regs.rsp; // rsp + regs[20] = sregs.ss.selector as u64; // ss + regs[21] = sregs.fs.base; // fs_base + regs[22] = sregs.gs.base; // gs_base + regs[23] = sregs.ds.selector as u64; // ds + regs[24] = sregs.es.selector as u64; // es + regs[25] = sregs.fs.selector as u64; // fs + regs[26] = sregs.gs.selector as u64; // gs + + // The [`CrashDumpContext`] accepts xsave as a vector of u8, so we need to convert the + // xsave region to a vector of u8 + Ok(crashdump::CrashDumpContext::new( + &self.mem_regions, + regs, + xsave.region.into_iter().fold(vec![], |mut acc, item| { + let bytes = item.to_le_bytes(); + acc.append(&mut bytes.to_vec()); + + acc + }), + self.entrypoint, + )) } #[cfg(gdb)] diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index bf85d3956..096aa5eb4 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -235,7 +235,7 @@ pub(crate) trait Hypervisor: Debug + Sync + Send { fn get_partition_handle(&self) -> windows::Win32::System::Hypervisor::WHV_PARTITION_HANDLE; #[cfg(crashdump)] - fn get_memory_regions(&self) -> &[MemoryRegion]; + fn crashdump_context(&self) -> Result; #[cfg(gdb)] /// handles the cases when the vCPU stops due to a Debug event diff --git a/src/hyperlight_host/src/hypervisor/windows_hypervisor_platform.rs b/src/hyperlight_host/src/hypervisor/windows_hypervisor_platform.rs index 47e200d38..6b02684c3 100644 --- a/src/hyperlight_host/src/hypervisor/windows_hypervisor_platform.rs +++ b/src/hyperlight_host/src/hypervisor/windows_hypervisor_platform.rs @@ -26,6 +26,8 @@ use windows_result::HRESULT; use super::wrappers::HandleWrapper; use crate::hypervisor::wrappers::{WHvFPURegisters, WHvGeneralRegisters, WHvSpecialRegisters}; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; +#[cfg(crashdump)] +use crate::HyperlightError; use crate::{new_error, Result}; // We need to pass in a primitive array of register names/values @@ -416,6 +418,59 @@ impl VMProcessor { } } + #[cfg(crashdump)] + #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] + pub(super) fn get_xsave(&self) -> Result> { + // Get the required buffer size by calling with NULL buffer + let mut buffer_size_needed: u32 = 0; + + unsafe { + // First call with NULL buffer to get required size + // If the buffer is not large enough, the return value is WHV_E_INSUFFICIENT_BUFFER. + // In this case, BytesWritten receives the required buffer size. + let result = WHvGetVirtualProcessorXsaveState( + self.get_partition_hdl(), + 0, + std::ptr::null_mut(), + 0, + &mut buffer_size_needed, + ); + + // If it failed for reasons other than insufficient buffer, return error + if let Err(e) = result { + if e.code() != windows::Win32::Foundation::WHV_E_INSUFFICIENT_BUFFER { + return Err(HyperlightError::WindowsAPIError(e)); + } + } + } + + // Create a buffer with the appropriate size + let mut xsave_buffer = vec![0; buffer_size_needed as usize]; + + // Get the Xsave state + let mut written_bytes = 0; + unsafe { + WHvGetVirtualProcessorXsaveState( + self.get_partition_hdl(), + 0, + xsave_buffer.as_mut_ptr() as *mut std::ffi::c_void, + buffer_size_needed, + &mut written_bytes, + ) + }?; + + // Check if the number of written bytes matches the expected size + if written_bytes != buffer_size_needed { + return Err(new_error!( + "Failed to get Xsave state: expected {} bytes, got {}", + buffer_size_needed, + written_bytes + )); + } + + Ok(xsave_buffer) + } + pub(super) fn set_fpu(&mut self, regs: &WHvFPURegisters) -> Result<()> { const LEN: usize = 26; From 51ce291e149ad6fa7a3f5e52c396e08c130b7c70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Mon, 28 Apr 2025 16:50:19 +0300 Subject: [PATCH 2/2] crashdump: add SandboxMetadata field that store relevant data about the sandbox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - only store the binary path for now Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/crashdump.rs | 21 +++++++++++-- .../src/hypervisor/hyperv_linux.rs | 20 +++++++++++-- .../src/hypervisor/hyperv_windows.rs | 19 ++++++++++-- .../src/hypervisor/hypervisor_handler.rs | 12 ++++++++ src/hyperlight_host/src/hypervisor/kvm.rs | 18 +++++++++-- src/hyperlight_host/src/hypervisor/mod.rs | 4 +++ .../src/sandbox/uninitialized.rs | 30 +++++++++++++++---- .../src/sandbox/uninitialized_evolve.rs | 7 +++++ 8 files changed, 118 insertions(+), 13 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/crashdump.rs b/src/hyperlight_host/src/hypervisor/crashdump.rs index 72673c67a..29c6149ce 100644 --- a/src/hyperlight_host/src/hypervisor/crashdump.rs +++ b/src/hyperlight_host/src/hypervisor/crashdump.rs @@ -22,6 +22,8 @@ pub(crate) struct CrashDumpContext<'a> { regs: [u64; 27], xsave: Vec, entry: u64, + binary: Option, + filename: Option, } impl<'a> CrashDumpContext<'a> { @@ -30,12 +32,16 @@ impl<'a> CrashDumpContext<'a> { regs: [u64; 27], xsave: Vec, entry: u64, + binary: Option, + filename: Option, ) -> Self { Self { regions, regs, xsave, entry, + binary, + filename, } } } @@ -69,6 +75,17 @@ impl GuestView { }) .collect(); + // The filename and command line are set to null-terminated strings + let filename = ctx + .filename + .clone() + .map_or_else(|| "\0".to_string(), |s| format!("{}\0", s)); + + let cmd = ctx + .binary + .clone() + .map_or_else(|| "\0".to_string(), |s| format!("{}\0", s)); + // The xsave state is checked as it can be empty let mut components = vec![]; if !ctx.xsave.is_empty() { @@ -88,7 +105,7 @@ impl GuestView { tid: 1, uid: 0, // User ID gid: 0, // Group ID - comm: "\0".to_string(), + comm: filename, ppid: 0, // Parent PID pgrp: 0, // Process group ID nice: 0, // Nice value @@ -101,7 +118,7 @@ impl GuestView { session: 0, // Session ID of the process sighold: 0, // Blocked signal sigpend: 0, // Pending signal - cmd_line: "\0".to_string(), + cmd_line: cmd, arch_state: Box::new(ArchState { gpr_state: ctx.regs.to_vec(), diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index f017b39b6..e869ced53 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -48,9 +48,9 @@ use mshv_bindings::{ }; use mshv_ioctls::{Mshv, VcpuFd, VmFd}; use tracing::{instrument, Span}; - #[cfg(crashdump)] -use super::crashdump; +use {super::crashdump, crate::sandbox::uninitialized::SandboxMetadata, std::path::Path}; + use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, MshvDebug}; @@ -296,6 +296,8 @@ pub(super) struct HypervLinuxDriver { debug: Option, #[cfg(gdb)] gdb_conn: Option>, + #[cfg(crashdump)] + metadata: SandboxMetadata, } impl HypervLinuxDriver { @@ -314,6 +316,7 @@ impl HypervLinuxDriver { rsp_ptr: GuestPtr, pml4_ptr: GuestPtr, #[cfg(gdb)] gdb_conn: Option>, + #[cfg(crashdump)] metadata: SandboxMetadata, ) -> Result { let mshv = Mshv::new()?; let pr = Default::default(); @@ -393,6 +396,8 @@ impl HypervLinuxDriver { debug, #[cfg(gdb)] gdb_conn, + #[cfg(crashdump)] + metadata, }) } @@ -696,11 +701,20 @@ impl Hypervisor for HypervLinuxDriver { regs[25] = sregs.fs.selector as u64; // fs regs[26] = sregs.gs.selector as u64; // gs + // Get the filename from the binary path + let filename = self.metadata.binary_path.clone().and_then(|path| { + Path::new(&path) + .file_name() + .and_then(|name| name.to_os_string().into_string().ok()) + }); + Ok(crashdump::CrashDumpContext::new( &self.mem_regions, regs, xsave.buffer.to_vec(), self.entrypoint, + self.metadata.binary_path.clone(), + filename, )) } @@ -821,6 +835,8 @@ mod tests { pml4_ptr, #[cfg(gdb)] None, + #[cfg(crashdump)] + SandboxMetadata { binary_path: None }, ) .unwrap(); } diff --git a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs index d6d2f63c6..86a09c117 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs @@ -27,9 +27,9 @@ use windows::Win32::System::Hypervisor::{ WHV_MEMORY_ACCESS_TYPE, WHV_PARTITION_HANDLE, WHV_REGISTER_VALUE, WHV_RUN_VP_EXIT_CONTEXT, WHV_RUN_VP_EXIT_REASON, WHV_X64_SEGMENT_REGISTER, WHV_X64_SEGMENT_REGISTER_0, }; - #[cfg(crashdump)] -use super::crashdump; +use {super::crashdump, crate::sandbox::uninitialized::SandboxMetadata, std::path::Path}; + use super::fpu::{FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] use super::handlers::DbgMemAccessHandlerWrapper; @@ -58,6 +58,8 @@ pub(crate) struct HypervWindowsDriver { entrypoint: u64, orig_rsp: GuestPtr, mem_regions: Vec, + #[cfg(crashdump)] + metadata: SandboxMetadata, } /* This does not automatically impl Send/Sync because the host * address of the shared memory region is a raw pointer, which are @@ -68,6 +70,7 @@ unsafe impl Send for HypervWindowsDriver {} unsafe impl Sync for HypervWindowsDriver {} impl HypervWindowsDriver { + #[allow(clippy::too_many_arguments)] #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] pub(crate) fn new( mem_regions: Vec, @@ -77,6 +80,7 @@ impl HypervWindowsDriver { entrypoint: u64, rsp: u64, mmap_file_handle: HandleWrapper, + #[cfg(crashdump)] metadata: SandboxMetadata, ) -> Result { // create and setup hypervisor partition let mut partition = VMPartition::new(1)?; @@ -104,6 +108,8 @@ impl HypervWindowsDriver { entrypoint, orig_rsp: GuestPtr::try_from(RawPtr::from(rsp))?, mem_regions, + #[cfg(crashdump)] + metadata, }) } @@ -525,11 +531,20 @@ impl Hypervisor for HypervWindowsDriver { regs[25] = unsafe { sregs.fs.Segment.Selector } as u64; // fs regs[26] = unsafe { sregs.gs.Segment.Selector } as u64; // gs + // Get the filename from the metadata + let filename = self.metadata.binary_path.clone().and_then(|path| { + Path::new(&path) + .file_name() + .and_then(|name| name.to_os_string().into_string().ok()) + }); + Ok(crashdump::CrashDumpContext::new( &self.mem_regions, regs, xsave, self.entrypoint, + self.metadata.binary_path.clone(), + filename, )) } } diff --git a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs index 591f43a07..a925cc1e4 100644 --- a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs +++ b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs @@ -51,6 +51,8 @@ use crate::mem::shared_mem::{GuestSharedMemory, HostSharedMemory, SharedMemory}; #[cfg(gdb)] use crate::sandbox::config::DebugInfo; use crate::sandbox::hypervisor::{get_available_hypervisor, HypervisorType}; +#[cfg(crashdump)] +use crate::sandbox::uninitialized::SandboxMetadata; #[cfg(target_os = "linux")] use crate::signal_handlers::setup_signal_handlers; use crate::HyperlightError::{ @@ -238,6 +240,7 @@ impl HypervisorHandler { &mut self, sandbox_memory_manager: SandboxMemoryManager, #[cfg(gdb)] debug_info: Option, + #[cfg(crashdump)] metadata: SandboxMetadata, ) -> Result<()> { let configuration = self.configuration.clone(); #[cfg(target_os = "windows")] @@ -304,6 +307,8 @@ impl HypervisorHandler { configuration.outb_handler.clone(), #[cfg(gdb)] &debug_info, + #[cfg(crashdump)] + &metadata, )?); } let hv = hv.as_mut().ok_or_else(|| new_error!("Hypervisor not set"))?; @@ -835,6 +840,7 @@ fn set_up_hypervisor_partition( #[allow(unused_variables)] // parameter only used for in-process mode outb_handler: OutBHandlerWrapper, #[cfg(gdb)] debug_info: &Option, + #[cfg(crashdump)] metadata: &SandboxMetadata, ) -> Result> { let mem_size = u64::try_from(mgr.shared_mem.mem_size())?; let mut regions = mgr.layout.get_memory_regions(&mgr.shared_mem)?; @@ -924,6 +930,8 @@ fn set_up_hypervisor_partition( pml4_ptr, #[cfg(gdb)] gdb_conn, + #[cfg(crashdump)] + metadata.clone(), )?; Ok(Box::new(hv)) } @@ -937,6 +945,8 @@ fn set_up_hypervisor_partition( rsp_ptr.absolute()?, #[cfg(gdb)] gdb_conn, + #[cfg(crashdump)] + metadata.clone(), )?; Ok(Box::new(hv)) } @@ -954,6 +964,8 @@ fn set_up_hypervisor_partition( entrypoint_ptr.absolute()?, rsp_ptr.absolute()?, HandleWrapper::from(mmap_file_handle), + #[cfg(crashdump)] + metadata.clone(), )?; Ok(Box::new(hv)) } diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index a899d285b..0dbf250a4 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -24,9 +24,9 @@ use kvm_ioctls::Cap::UserMemory; use kvm_ioctls::{Kvm, VcpuExit, VcpuFd, VmFd}; use log::LevelFilter; use tracing::{instrument, Span}; - #[cfg(crashdump)] -use super::crashdump; +use {super::crashdump, crate::sandbox::uninitialized::SandboxMetadata, std::path::Path}; + use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason}; @@ -288,6 +288,8 @@ pub(super) struct KVMDriver { debug: Option, #[cfg(gdb)] gdb_conn: Option>, + #[cfg(crashdump)] + metadata: SandboxMetadata, } impl KVMDriver { @@ -301,6 +303,7 @@ impl KVMDriver { entrypoint: u64, rsp: u64, #[cfg(gdb)] gdb_conn: Option>, + #[cfg(crashdump)] metadata: SandboxMetadata, ) -> Result { let kvm = Kvm::new()?; @@ -352,6 +355,8 @@ impl KVMDriver { debug, #[cfg(gdb)] gdb_conn, + #[cfg(crashdump)] + metadata, }; Ok(ret) @@ -620,6 +625,13 @@ impl Hypervisor for KVMDriver { regs[25] = sregs.fs.selector as u64; // fs regs[26] = sregs.gs.selector as u64; // gs + // Get the filename from the metadata + let filename = self.metadata.binary_path.clone().and_then(|path| { + Path::new(&path) + .file_name() + .and_then(|name| name.to_os_string().into_string().ok()) + }); + // The [`CrashDumpContext`] accepts xsave as a vector of u8, so we need to convert the // xsave region to a vector of u8 Ok(crashdump::CrashDumpContext::new( @@ -632,6 +644,8 @@ impl Hypervisor for KVMDriver { acc }), self.entrypoint, + self.metadata.binary_path.clone(), + filename, )) } diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index 096aa5eb4..63dbd022c 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -350,6 +350,8 @@ pub(crate) mod tests { }; use crate::mem::ptr::RawPtr; use crate::sandbox::uninitialized::GuestBinary; + #[cfg(crashdump)] + use crate::sandbox::uninitialized::SandboxMetadata; use crate::sandbox::{SandboxConfiguration, UninitializedSandbox}; use crate::{new_error, Result}; @@ -411,6 +413,8 @@ pub(crate) mod tests { gshm, #[cfg(gdb)] None, + #[cfg(crashdump)] + SandboxMetadata { binary_path: None }, )?; hv_handler.execute_hypervisor_handler_action(HypervisorHandlerAction::Initialise) diff --git a/src/hyperlight_host/src/sandbox/uninitialized.rs b/src/hyperlight_host/src/sandbox/uninitialized.rs index c1527a5c2..ffc13853e 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized.rs @@ -39,6 +39,12 @@ use crate::sandbox_state::sandbox::EvolvableSandbox; use crate::sandbox_state::transition::Noop; use crate::{log_build_details, log_then_return, new_error, MultiUseSandbox, Result}; +#[cfg(crashdump)] +#[derive(Clone, Debug, Default)] +pub(crate) struct SandboxMetadata { + pub(crate) binary_path: Option, +} + /// A preliminary `Sandbox`, not yet ready to execute guest code. /// /// Prior to initializing a full-fledged `Sandbox`, you must create one of @@ -58,6 +64,8 @@ pub struct UninitializedSandbox { pub(crate) max_guest_log_level: Option, #[cfg(gdb)] pub(crate) debug_info: Option, + #[cfg(crashdump)] + pub(crate) metadata: SandboxMetadata, } impl crate::sandbox_state::sandbox::UninitializedSandbox for UninitializedSandbox { @@ -142,15 +150,25 @@ impl UninitializedSandbox { let path = Path::new(&binary_path) .canonicalize() .map_err(|e| new_error!("GuestBinary not found: '{}': {}", binary_path, e))?; - GuestBinary::FilePath( - path.into_os_string() - .into_string() - .map_err(|e| new_error!("Error converting OsString to String: {:?}", e))?, - ) + let path = path + .into_os_string() + .into_string() + .map_err(|e| new_error!("Error converting OsString to String: {:?}", e))?; + + GuestBinary::FilePath(path) } buffer @ GuestBinary::Buffer(_) => buffer, }; + #[cfg(crashdump)] + let metadata = if let GuestBinary::FilePath(ref path) = guest_binary { + SandboxMetadata { + binary_path: Some(path.clone()), + } + } else { + SandboxMetadata::default() + }; + let run_opts = sandbox_run_options.unwrap_or_default(); let run_inprocess = run_opts.in_process(); @@ -200,6 +218,8 @@ impl UninitializedSandbox { max_guest_log_level: None, #[cfg(gdb)] debug_info, + #[cfg(crashdump)] + metadata, }; // TODO: These only here to accommodate some writer functions. diff --git a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs index 6ddba5e29..808a018b3 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs @@ -23,6 +23,8 @@ use tracing::{instrument, Span}; #[cfg(gdb)] use super::mem_access::dbg_mem_access_handler_wrapper; +#[cfg(crashdump)] +use super::uninitialized::SandboxMetadata; use crate::hypervisor::hypervisor_handler::{ HvHandlerConfig, HypervisorHandler, HypervisorHandlerAction, }; @@ -74,6 +76,8 @@ where u_sbox.max_guest_log_level, #[cfg(gdb)] u_sbox.debug_info, + #[cfg(crashdump)] + u_sbox.metadata, )?; { @@ -111,6 +115,7 @@ fn hv_init( max_wait_for_cancellation: Duration, max_guest_log_level: Option, #[cfg(gdb)] debug_info: Option, + #[cfg(crashdump)] metadata: SandboxMetadata, ) -> Result { let outb_hdl = outb_handler_wrapper(hshm.clone(), host_funcs); let mem_access_hdl = mem_access_handler_wrapper(hshm.clone()); @@ -149,6 +154,8 @@ fn hv_init( gshm, #[cfg(gdb)] debug_info, + #[cfg(crashdump)] + metadata, )?; hv_handler