From 90a3f91f5f09db71944ef3b2ca6ff5c1064928b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 15:16:57 +0200 Subject: [PATCH 01/20] add gdb cargo feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- src/hyperlight_host/Cargo.toml | 2 ++ src/hyperlight_host/build.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/src/hyperlight_host/Cargo.toml b/src/hyperlight_host/Cargo.toml index 032911dd..b0e37c94 100644 --- a/src/hyperlight_host/Cargo.toml +++ b/src/hyperlight_host/Cargo.toml @@ -128,6 +128,8 @@ kvm = ["dep:kvm-bindings", "dep:kvm-ioctls"] mshv2 = ["dep:mshv-bindings2", "dep:mshv-ioctls2"] mshv3 = ["dep:mshv-bindings3", "dep:mshv-ioctls3"] inprocess = [] +# This enables compilation of gdb stub for easy debug in the guest +gdb = [] [[bench]] name = "benchmarks" diff --git a/src/hyperlight_host/build.rs b/src/hyperlight_host/build.rs index 7600f647..d7a34a3f 100644 --- a/src/hyperlight_host/build.rs +++ b/src/hyperlight_host/build.rs @@ -89,6 +89,7 @@ fn main() -> Result<()> { // Essentially the kvm and mshv features are ignored on windows as long as you use #[cfg(kvm)] and not #[cfg(feature = "kvm")]. // You should never use #[cfg(feature = "kvm")] or #[cfg(feature = "mshv")] in the codebase. cfg_aliases::cfg_aliases! { + gdb: { all(feature = "gdb", debug_assertions, target_os = "linux") }, kvm: { all(feature = "kvm", target_os = "linux") }, mshv: { all(any(feature = "mshv2", feature = "mshv3"), target_os = "linux") }, // inprocess feature is aliased with debug_assertions to make it only available in debug-builds. From 1913aa5a90fab00ed9da70aac81f51152b6ed445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 15:33:47 +0200 Subject: [PATCH 02/20] disable timeouts for messaging between host and hypervisor handler when gdb feature is on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/hypervisor_handler.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs index c78624c0..e440c700 100644 --- a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs +++ b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs @@ -597,11 +597,15 @@ impl HypervisorHandler { /// and still have to receive after sorting that out without sending /// an extra message. pub(crate) fn try_receive_handler_msg(&self) -> Result<()> { - match self + #[cfg(gdb)] + let response = self.communication_channels.from_handler_rx.recv(); + #[cfg(not(gdb))] + let response = self .communication_channels .from_handler_rx - .recv_timeout(self.execution_variables.get_timeout()?) - { + .recv_timeout(self.execution_variables.get_timeout()?); + + match response { Ok(msg) => match msg { HandlerMsg::Error(e) => Err(e), HandlerMsg::FinishedHypervisorHandlerAction => Ok(()), From c5aeada92b71e25cf37413febd06a3593fa0c962 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 16:34:47 +0200 Subject: [PATCH 03/20] add guest debug port to sandbox configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/hypervisor_handler.rs | 6 ++++ src/hyperlight_host/src/hypervisor/mod.rs | 6 +++- src/hyperlight_host/src/sandbox/config.rs | 36 +++++++++++++++++++ .../src/sandbox/uninitialized.rs | 16 +++++++++ .../src/sandbox/uninitialized_evolve.rs | 11 +++++- 5 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs index e440c700..3f56a11a 100644 --- a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs +++ b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs @@ -49,6 +49,8 @@ use crate::mem::shared_mem::{GuestSharedMemory, HostSharedMemory, SharedMemory}; use crate::sandbox::hypervisor::{get_available_hypervisor, HypervisorType}; #[cfg(feature = "function_call_metrics")] use crate::sandbox::metrics::SandboxMetric::GuestFunctionCallDurationMicroseconds; +#[cfg(gdb)] +use crate::sandbox::uninitialized::DebugInfo; #[cfg(target_os = "linux")] use crate::signal_handlers::setup_signal_handlers; use crate::HyperlightError::{ @@ -232,6 +234,7 @@ impl HypervisorHandler { pub(crate) fn start_hypervisor_handler( &mut self, sandbox_memory_manager: SandboxMemoryManager, + #[cfg(gdb)] debug_info: Option, ) -> Result<()> { let configuration = self.configuration.clone(); #[cfg(target_os = "windows")] @@ -292,6 +295,8 @@ impl HypervisorHandler { hv = Some(set_up_hypervisor_partition( execution_variables.shm.try_lock().unwrap().deref_mut().as_mut().unwrap(), configuration.outb_handler.clone(), + #[cfg(gdb)] + &debug_info, )?); } let hv = hv.as_mut().unwrap(); @@ -827,6 +832,7 @@ fn set_up_hypervisor_partition( mgr: &mut SandboxMemoryManager, #[allow(unused_variables)] // parameter only used for in-process mode outb_handler: OutBHandlerWrapper, + #[cfg(gdb)] _debug_info: &Option, ) -> Result> { let mem_size = u64::try_from(mgr.shared_mem.mem_size())?; let mut regions = mgr.layout.get_memory_regions(&mgr.shared_mem)?; diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index c34d7eb1..62ec8ca8 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -336,7 +336,11 @@ pub(crate) mod tests { // whether we can configure the shared memory region, load a binary // into it, and run the CPU to completion (e.g., a HLT interrupt) - hv_handler.start_hypervisor_handler(gshm)?; + hv_handler.start_hypervisor_handler( + gshm, + #[cfg(gdb)] + None, + )?; hv_handler.execute_hypervisor_handler_action(HypervisorHandlerAction::Initialise) } diff --git a/src/hyperlight_host/src/sandbox/config.rs b/src/hyperlight_host/src/sandbox/config.rs index c03528e3..0dbf1e04 100644 --- a/src/hyperlight_host/src/sandbox/config.rs +++ b/src/hyperlight_host/src/sandbox/config.rs @@ -25,6 +25,9 @@ use crate::mem::exe::ExeInfo; #[derive(Copy, Clone, Debug, Eq, PartialEq)] #[repr(C)] pub struct SandboxConfiguration { + #[cfg(gdb)] + /// Guest gdb debug port + guest_debug_port: Option, /// The maximum size of the guest error buffer. guest_error_buffer_size: usize, /// The size of the memory buffer that is made available for Guest Function @@ -136,6 +139,9 @@ impl SandboxConfiguration { pub const MIN_KERNEL_STACK_SIZE: usize = 0x1000; /// The default value for kernel stack size pub const DEFAULT_KERNEL_STACK_SIZE: usize = Self::MIN_KERNEL_STACK_SIZE; + #[cfg(gdb)] + /// The minimum value for debug port + pub const MIN_GUEST_DEBUG_PORT: u16 = 9000; #[allow(clippy::too_many_arguments)] /// Create a new configuration for a sandbox with the given sizes. @@ -153,6 +159,7 @@ impl SandboxConfiguration { max_initialization_time: Option, max_wait_for_cancellation: Option, guest_panic_context_buffer_size: usize, + #[cfg(gdb)] guest_debug_port: Option, ) -> Self { Self { input_data_size: max(input_data_size, Self::MIN_INPUT_SIZE), @@ -220,6 +227,8 @@ impl SandboxConfiguration { guest_panic_context_buffer_size, Self::MIN_GUEST_PANIC_CONTEXT_BUFFER_SIZE, ), + #[cfg(gdb)] + guest_debug_port, } } @@ -346,6 +355,13 @@ impl SandboxConfiguration { ); } + #[cfg(gdb)] + #[instrument(skip_all, parent = Span::current(), level= "Trace")] + /// Sets the configuration for the guest debug + pub fn set_guest_debug_port(&mut self, port: u16) { + self.guest_debug_port = Some(max(port, Self::MIN_GUEST_DEBUG_PORT)); + } + #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn get_guest_error_buffer_size(&self) -> usize { self.guest_error_buffer_size @@ -390,6 +406,12 @@ impl SandboxConfiguration { self.max_initialization_time } + #[cfg(gdb)] + #[instrument(skip_all, parent = Span::current(), level= "Trace")] + pub(crate) fn get_guest_debug_port(&self) -> Option { + self.guest_debug_port + } + #[instrument(skip_all, parent = Span::current(), level= "Trace")] fn stack_size_override_opt(&self) -> Option { (self.stack_size_override > 0).then_some(self.stack_size_override) @@ -438,6 +460,8 @@ impl Default for SandboxConfiguration { None, None, Self::DEFAULT_GUEST_PANIC_CONTEXT_BUFFER_SIZE, + #[cfg(gdb)] + None, ) } } @@ -480,6 +504,8 @@ mod tests { MAX_WAIT_FOR_CANCELLATION_OVERRIDE as u64, )), GUEST_PANIC_CONTEXT_BUFFER_SIZE_OVERRIDE, + #[cfg(gdb)] + None, ); let exe_infos = vec![ simple_guest_exe_info().unwrap(), @@ -543,6 +569,8 @@ mod tests { SandboxConfiguration::MIN_MAX_WAIT_FOR_CANCELLATION as u64 - 1, )), SandboxConfiguration::MIN_GUEST_PANIC_CONTEXT_BUFFER_SIZE - 1, + #[cfg(gdb)] + None, ); assert_eq!(SandboxConfiguration::MIN_INPUT_SIZE, cfg.input_data_size); assert_eq!(SandboxConfiguration::MIN_OUTPUT_SIZE, cfg.output_data_size); @@ -711,6 +739,14 @@ mod tests { cfg.set_heap_size(size); prop_assert_eq!(size, cfg.heap_size_override); } + + #[test] + #[cfg(gdb)] + fn guest_debug_port(port in 9000..=u16::MAX) { + let mut cfg = SandboxConfiguration::default(); + cfg.set_guest_debug_port(port); + prop_assert_eq!(port, *cfg.get_guest_debug_port().as_ref().unwrap()); + } } } } diff --git a/src/hyperlight_host/src/sandbox/uninitialized.rs b/src/hyperlight_host/src/sandbox/uninitialized.rs index 4ca951d5..721b1395 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized.rs @@ -36,6 +36,14 @@ use crate::sandbox_state::sandbox::EvolvableSandbox; use crate::sandbox_state::transition::Noop; use crate::{log_build_details, log_then_return, new_error, MultiUseSandbox, Result}; +#[allow(dead_code)] +#[cfg(gdb)] +/// Used for passing debug configuration to a sandbox +pub struct DebugInfo { + /// Guest debug port + pub(crate) port: u16, +} + /// A preliminary `Sandbox`, not yet ready to execute guest code. /// /// Prior to initializing a full-fledged `Sandbox`, you must create one of @@ -52,6 +60,8 @@ pub struct UninitializedSandbox { pub(crate) max_initialization_time: Duration, pub(crate) max_execution_time: Duration, pub(crate) max_wait_for_cancellation: Duration, + #[cfg(gdb)] + pub(crate) debug_info: Option, } impl crate::sandbox_state::sandbox::UninitializedSandbox for UninitializedSandbox { @@ -161,6 +171,10 @@ impl UninitializedSandbox { } let sandbox_cfg = cfg.unwrap_or_default(); + #[cfg(gdb)] + let debug_info = sandbox_cfg + .get_guest_debug_port() + .map(|port| DebugInfo { port }); let mut mem_mgr_wrapper = { let mut mgr = UninitializedSandbox::load_guest_binary( sandbox_cfg, @@ -188,6 +202,8 @@ impl UninitializedSandbox { max_wait_for_cancellation: Duration::from_millis( sandbox_cfg.get_max_wait_for_cancellation() as u64, ), + #[cfg(gdb)] + debug_info, }; // 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 ccbed836..559ddeae 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs @@ -20,6 +20,8 @@ use std::sync::{Arc, Mutex}; use rand::Rng; use tracing::{instrument, Span}; +#[cfg(gdb)] +use super::uninitialized::DebugInfo; use crate::hypervisor::hypervisor_handler::{ HvHandlerConfig, HypervisorHandler, HypervisorHandlerAction, }; @@ -66,6 +68,8 @@ where u_sbox.max_initialization_time, u_sbox.max_execution_time, u_sbox.max_wait_for_cancellation, + #[cfg(gdb)] + u_sbox.debug_info, )?; { @@ -98,6 +102,7 @@ fn hv_init( max_init_time: Duration, max_exec_time: Duration, max_wait_for_cancellation: Duration, + #[cfg(gdb)] debug_info: Option, ) -> Result { let outb_hdl = outb_handler_wrapper(hshm.clone(), host_funcs); let mem_access_hdl = mem_access_handler_wrapper(hshm.clone()); @@ -126,7 +131,11 @@ fn hv_init( let mut hv_handler = HypervisorHandler::new(hv_handler_config); - hv_handler.start_hypervisor_handler(gshm)?; + hv_handler.start_hypervisor_handler( + gshm, + #[cfg(gdb)] + debug_info, + )?; hv_handler .execute_hypervisor_handler_action(HypervisorHandlerAction::Initialise) From 579b3249ca620a3f2059a40ea1128fd046133110 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 17:16:57 +0200 Subject: [PATCH 04/20] add gdb debug tread creation method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- Cargo.lock | 32 +++++++++++++ src/hyperlight_host/Cargo.toml | 2 + src/hyperlight_host/src/hypervisor/gdb/mod.rs | 46 +++++++++++++++++++ .../src/hypervisor/hypervisor_handler.rs | 4 +- src/hyperlight_host/src/hypervisor/kvm.rs | 10 ++++ src/hyperlight_host/src/hypervisor/mod.rs | 4 ++ .../src/sandbox/uninitialized.rs | 1 - 7 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 src/hyperlight_host/src/hypervisor/gdb/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 7e7b5c15..7c47a773 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -805,6 +805,30 @@ dependencies = [ "slab", ] +[[package]] +name = "gdbstub" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "31c683a9f13de31432e6097131d5f385898c7f0635c0f392b9d0fa165063c8ac" +dependencies = [ + "bitflags 2.8.0", + "cfg-if", + "log", + "managed", + "num-traits", + "paste", +] + +[[package]] +name = "gdbstub_arch" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "328a9e9425db13770d0d11de6332a608854266e44c53d12776be7b4aa427e3de" +dependencies = [ + "gdbstub", + "num-traits", +] + [[package]] name = "generic-array" version = "0.14.7" @@ -1106,6 +1130,8 @@ dependencies = [ "env_logger", "envy", "flatbuffers", + "gdbstub", + "gdbstub_arch", "goblin", "hyperlight-common", "hyperlight-testing", @@ -1568,6 +1594,12 @@ dependencies = [ "libc", ] +[[package]] +name = "managed" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ca88d725a0a943b096803bd34e73a4437208b6077654cc4ecb2947a5f91618d" + [[package]] name = "matchers" version = "0.1.0" diff --git a/src/hyperlight_host/Cargo.toml b/src/hyperlight_host/Cargo.toml index b0e37c94..8a3c6c19 100644 --- a/src/hyperlight_host/Cargo.toml +++ b/src/hyperlight_host/Cargo.toml @@ -71,6 +71,8 @@ sha256 = "1.4.0" windows-version = "0.1" [target.'cfg(unix)'.dependencies] +gdbstub = "0.7.3" +gdbstub_arch = "0.3.1" seccompiler = { version = "0.4.0", optional = true } kvm-bindings = { version = "0.11", features = ["fam-wrappers"], optional = true } kvm-ioctls = { version = "0.20", optional = true } diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs new file mode 100644 index 00000000..c6f73642 --- /dev/null +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -0,0 +1,46 @@ +use std::io::{self, ErrorKind}; +use std::net::TcpListener; +use std::thread; +use thiserror::Error; +#[derive(Debug, Error)] +pub enum GdbTargetError { + #[error("Error encountered while binding to address and port")] + CannotBind, + #[error("Error encountered while listening for connections")] + ListenerError, + #[error("Unexpected error encountered")] + UnexpectedError, +} + +impl From for GdbTargetError { + fn from(err: io::Error) -> Self { + match err.kind() { + ErrorKind::AddrInUse => Self::CannotBind, + ErrorKind::AddrNotAvailable => Self::CannotBind, + ErrorKind::ConnectionReset + | ErrorKind::ConnectionAborted + | ErrorKind::ConnectionRefused => Self::ListenerError, + _ => Self::UnexpectedError, + } + } +} +/// Creates a thread that handles gdb protocol +pub fn create_gdb_thread( + port: u16, +) -> Result<(), GdbTargetError> { + let socket = format!("localhost:{}", port); + + log::info!("Listening on {:?}", socket); + let listener = TcpListener::bind(socket)?; + + log::info!("Starting GDB thread"); + let _handle = thread::Builder::new() + .name("GDB handler".to_string()) + .spawn(move || -> Result<(), GdbTargetError> { + log::info!("Waiting for GDB connection ... "); + let (_, _) = listener.accept()?; + Ok(()) + }); + + Ok(()) +} diff --git a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs index 3f56a11a..737eada6 100644 --- a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs +++ b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs @@ -832,7 +832,7 @@ fn set_up_hypervisor_partition( mgr: &mut SandboxMemoryManager, #[allow(unused_variables)] // parameter only used for in-process mode outb_handler: OutBHandlerWrapper, - #[cfg(gdb)] _debug_info: &Option, + #[cfg(gdb)] debug_info: &Option, ) -> Result> { let mem_size = u64::try_from(mgr.shared_mem.mem_size())?; let mut regions = mgr.layout.get_memory_regions(&mgr.shared_mem)?; @@ -911,6 +911,8 @@ fn set_up_hypervisor_partition( pml4_ptr.absolute()?, entrypoint_ptr.absolute()?, rsp_ptr.absolute()?, + #[cfg(gdb)] + debug_info, )?; Ok(Box::new(hv)) } diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index e21c6a35..af58e11d 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -23,6 +23,8 @@ use kvm_ioctls::{Kvm, VcpuExit, VcpuFd, VmFd}; use tracing::{instrument, Span}; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; +#[cfg(gdb)] +use super::gdb::create_gdb_thread; use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; use super::{ HyperlightExit, Hypervisor, VirtualCPU, CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, @@ -31,6 +33,8 @@ use super::{ use crate::hypervisor::hypervisor_handler::HypervisorHandler; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; use crate::mem::ptr::{GuestPtr, RawPtr}; +#[cfg(gdb)] +use crate::sandbox::uninitialized::DebugInfo; use crate::{log_then_return, new_error, Result}; /// Return `true` if the KVM API is available, version 12, and has UserMemory capability, or `false` otherwise @@ -75,6 +79,7 @@ impl KVMDriver { pml4_addr: u64, entrypoint: u64, rsp: u64, + #[cfg(gdb)] debug_info: &Option, ) -> Result { let kvm = Kvm::new()?; @@ -101,6 +106,11 @@ impl KVMDriver { let mut vcpu_fd = vm_fd.create_vcpu(0)?; Self::setup_initial_sregs(&mut vcpu_fd, pml4_addr)?; + #[cfg(gdb)] + if let Some(DebugInfo { port }) = debug_info { + create_gdb_thread(*port).map_err(|_| new_error!("Cannot create GDB thread"))?; + } + let rsp_gp = GuestPtr::try_from(RawPtr::from(rsp))?; Ok(Self { _kvm: kvm, diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index 62ec8ca8..f7364689 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -34,6 +34,10 @@ pub mod hyperv_linux; pub(crate) mod hyperv_windows; pub(crate) mod hypervisor_handler; +/// GDB debugging support +#[cfg(gdb)] +mod gdb; + /// Driver for running in process instead of using hypervisor #[cfg(inprocess)] pub mod inprocess; diff --git a/src/hyperlight_host/src/sandbox/uninitialized.rs b/src/hyperlight_host/src/sandbox/uninitialized.rs index 721b1395..66f33fa9 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized.rs @@ -36,7 +36,6 @@ use crate::sandbox_state::sandbox::EvolvableSandbox; use crate::sandbox_state::transition::Noop; use crate::{log_build_details, log_then_return, new_error, MultiUseSandbox, Result}; -#[allow(dead_code)] #[cfg(gdb)] /// Used for passing debug configuration to a sandbox pub struct DebugInfo { From 535dd755b992c441c118e9990ed4c93927a2c57d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 19:44:47 +0200 Subject: [PATCH 05/20] add debug communication channel type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - this type will be used by the gdb and the hypervisor handler to send requests and receive responses Signed-off-by: Doru Blânzeanu --- src/hyperlight_host/src/hypervisor/gdb/mod.rs | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index c6f73642..1e8d35e2 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -1,6 +1,9 @@ +#![allow(dead_code)] use std::io::{self, ErrorKind}; use std::net::TcpListener; use std::thread; + +use crossbeam_channel::{Receiver, Sender, TryRecvError}; use thiserror::Error; #[derive(Debug, Error)] pub enum GdbTargetError { @@ -8,6 +11,10 @@ pub enum GdbTargetError { CannotBind, #[error("Error encountered while listening for connections")] ListenerError, + #[error("Error encountered when waiting to receive message")] + CannotReceiveMsg, + #[error("Error encountered when sending message")] + CannotSendMsg, #[error("Unexpected error encountered")] UnexpectedError, } @@ -24,6 +31,48 @@ impl From for GdbTargetError { } } } +/// Type that takes care of communication between Hypervisor and Gdb +pub struct DebugCommChannel { + /// Transmit channel + tx: Sender, + /// Receive channel + rx: Receiver, +} + +impl DebugCommChannel { + pub fn unbounded() -> (DebugCommChannel, DebugCommChannel) { + let (hyp_tx, gdb_rx): (Sender, Receiver) = crossbeam_channel::unbounded(); + let (gdb_tx, hyp_rx): (Sender, Receiver) = crossbeam_channel::unbounded(); + + let gdb_conn = DebugCommChannel { + tx: gdb_tx, + rx: gdb_rx, + }; + + let hyp_conn = DebugCommChannel { + tx: hyp_tx, + rx: hyp_rx, + }; + + (gdb_conn, hyp_conn) + } + + /// Sends message over the transmit channel and expects a response + pub fn send(&self, msg: T) -> Result<(), GdbTargetError> { + self.tx.send(msg).map_err(|_| GdbTargetError::CannotSendMsg) + } + + /// Waits for a message over the receive channel + pub fn recv(&self) -> Result { + self.rx.recv().map_err(|_| GdbTargetError::CannotReceiveMsg) + } + + /// Checks whether there's a message waiting on the receive channel + pub fn try_recv(&self) -> Result { + self.rx.try_recv() + } +} + /// Creates a thread that handles gdb protocol pub fn create_gdb_thread( port: u16, From 1203ff4cf79e08e8812b64a69984d995fa908dd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 20:17:53 +0200 Subject: [PATCH 06/20] add hyperlight sandbox target to handle gdb commands support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - the target implements the traits to provide callbacks for the gdb commands Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/event_loop.rs | 99 ++++++++++++++++ src/hyperlight_host/src/hypervisor/gdb/mod.rs | 65 ++++++++++- .../src/hypervisor/gdb/x86_64_target.rs | 108 ++++++++++++++++++ src/hyperlight_host/src/hypervisor/mod.rs | 20 ++++ 4 files changed, 287 insertions(+), 5 deletions(-) create mode 100644 src/hyperlight_host/src/hypervisor/gdb/event_loop.rs create mode 100644 src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs diff --git a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs new file mode 100644 index 00000000..716f54e9 --- /dev/null +++ b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs @@ -0,0 +1,99 @@ +use gdbstub::common::Signal; +use gdbstub::conn::ConnectionExt; +use gdbstub::stub::run_blocking::{self, WaitForStopReasonError}; +use gdbstub::stub::{BaseStopReason, DisconnectReason, GdbStub, SingleThreadStopReason}; + +use super::x86_64_target::HyperlightSandboxTarget; +use super::{DebugResponse, VcpuStopReason}; + +pub struct GdbBlockingEventLoop; + +impl run_blocking::BlockingEventLoop for GdbBlockingEventLoop { + type Connection = Box>; + type StopReason = SingleThreadStopReason; + type Target = HyperlightSandboxTarget; + + fn wait_for_stop_reason( + target: &mut Self::Target, + conn: &mut Self::Connection, + ) -> Result< + run_blocking::Event, + run_blocking::WaitForStopReasonError< + ::Error, + ::Error, + >, + > { + loop { + match target.try_recv() { + Ok(DebugResponse::VcpuStopped(stop_reason)) => { + log::debug!("VcpuStopped with reason {:?}", stop_reason); + + // Resume execution if unknown reason for stop + let stop_response = match stop_reason { + VcpuStopReason::DoneStep => BaseStopReason::DoneStep, + VcpuStopReason::SwBp => BaseStopReason::SwBreak(()), + VcpuStopReason::HwBp => BaseStopReason::HwBreak(()), + VcpuStopReason::Unknown => { + target + .resume_vcpu() + .map_err(WaitForStopReasonError::Target)?; + + continue; + } + }; + + return Ok(run_blocking::Event::TargetStopped(stop_response)); + } + Ok(msg) => { + log::error!("Unexpected message received {:?}", msg); + } + Err(crossbeam_channel::TryRecvError::Empty) => (), + Err(crossbeam_channel::TryRecvError::Disconnected) => { + return Ok(run_blocking::Event::TargetStopped(BaseStopReason::Exited( + 0, + ))); + } + } + + if conn.peek().map(|b| b.is_some()).unwrap_or(false) { + let byte = conn + .read() + .map_err(run_blocking::WaitForStopReasonError::Connection)?; + + return Ok(run_blocking::Event::IncomingData(byte)); + } + } + } + + fn on_interrupt( + _target: &mut Self::Target, + ) -> Result, ::Error> { + Ok(Some(SingleThreadStopReason::SignalWithThread { + tid: (), + signal: Signal::SIGINT, + })) + } +} + +pub fn event_loop_thread( + debugger: GdbStub>>, + target: &mut HyperlightSandboxTarget, +) { + match debugger.run_blocking::(target) { + Ok(disconnect_reason) => match disconnect_reason { + DisconnectReason::Disconnect => { + log::info!("Gdb client disconnected"); + } + DisconnectReason::TargetExited(_) => { + log::info!("Guest finalized execution and disconnected"); + } + DisconnectReason::TargetTerminated(sig) => { + log::info!("Gdb target terminated with signal {}", sig) + } + DisconnectReason::Kill => log::info!("Gdb sent a kill command"), + }, + Err(e) => { + log::error!("fatal error encountered: {e:?}"); + } + } +} diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 1e8d35e2..e5819a78 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -1,10 +1,18 @@ -#![allow(dead_code)] +mod event_loop; +pub mod x86_64_target; + use std::io::{self, ErrorKind}; use std::net::TcpListener; use std::thread; use crossbeam_channel::{Receiver, Sender, TryRecvError}; +use event_loop::event_loop_thread; +use gdbstub::conn::ConnectionExt; +use gdbstub::stub::GdbStub; +use gdbstub::target::TargetError; use thiserror::Error; +use x86_64_target::HyperlightSandboxTarget; + #[derive(Debug, Error)] pub enum GdbTargetError { #[error("Error encountered while binding to address and port")] @@ -15,6 +23,8 @@ pub enum GdbTargetError { CannotReceiveMsg, #[error("Error encountered when sending message")] CannotSendMsg, + #[error("Encountered an unexpected message over communication channel")] + UnexpectedMessage, #[error("Unexpected error encountered")] UnexpectedError, } @@ -31,7 +41,39 @@ impl From for GdbTargetError { } } } -/// Type that takes care of communication between Hypervisor and Gdb + +impl From for TargetError { + fn from(value: GdbTargetError) -> TargetError { + TargetError::Io(std::io::Error::other(value)) + } +} + +#[derive(Debug)] +/// Defines the possible reasons for which a vCPU ce be stopped when debugging +pub enum VcpuStopReason { + DoneStep, + HwBp, + SwBp, + Unknown, +} + +#[allow(dead_code)] +#[derive(Debug)] +/// Enumerates the possible actions that a debugger can ask from a Hypervisor +pub enum DebugMsg { + Continue, +} + +#[allow(dead_code)] +#[derive(Debug)] +/// Enumerates the possible responses that a hypervisor can provide to a debugger +pub enum DebugResponse { + Continue, + VcpuStopped(VcpuStopReason), +} + +/// Debug communication channel that is used for sending a request type and +/// receive a different response type pub struct DebugCommChannel { /// Transmit channel tx: Sender, @@ -76,7 +118,8 @@ impl DebugCommChannel { /// Creates a thread that handles gdb protocol pub fn create_gdb_thread( port: u16, -) -> Result<(), GdbTargetError> { +) -> Result, GdbTargetError> { + let (gdb_conn, hyp_conn) = DebugCommChannel::unbounded(); let socket = format!("localhost:{}", port); log::info!("Listening on {:?}", socket); @@ -87,9 +130,21 @@ pub fn create_gdb_thread( .name("GDB handler".to_string()) .spawn(move || -> Result<(), GdbTargetError> { log::info!("Waiting for GDB connection ... "); - let (_, _) = listener.accept()?; + let (conn, _) = listener.accept()?; + + let conn: Box> = Box::new(conn); + let debugger = GdbStub::new(conn); + + let mut target = HyperlightSandboxTarget::new(hyp_conn); + + // Waits for vCPU to stop at entrypoint breakpoint + let res = target.recv()?; + if let DebugResponse::VcpuStopped(_) = res { + event_loop_thread(debugger, &mut target); + } + Ok(()) }); - Ok(()) + Ok(gdb_conn) } diff --git a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs new file mode 100644 index 00000000..9c3eaaba --- /dev/null +++ b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs @@ -0,0 +1,108 @@ +use crossbeam_channel::TryRecvError; +use gdbstub::arch::Arch; +use gdbstub::target::ext::base::singlethread::SingleThreadBase; +use gdbstub::target::ext::base::BaseOps; +use gdbstub::target::{Target, TargetResult}; +use gdbstub_arch::x86::X86_64_SSE as GdbTargetArch; + +use super::{DebugMsg, DebugResponse, DebugCommChannel, GdbTargetError}; + +/// Gdbstub target used by the gdbstub crate to provide GDB protocol implementation +pub struct HyperlightSandboxTarget { + /// Hypervisor communication channels + hyp_conn: DebugCommChannel, +} + +impl HyperlightSandboxTarget { + pub fn new(hyp_conn: DebugCommChannel) -> Self { + HyperlightSandboxTarget { hyp_conn } + } + + /// Sends a command over the communication channel and waits for response + fn send_command(&self, cmd: DebugMsg) -> Result { + self.send(cmd)?; + + // Wait for response + self.recv() + } + + /// Sends a command over the communication channel + fn send(&self, ev: DebugMsg) -> Result<(), GdbTargetError> { + self.hyp_conn.send(ev) + } + + /// Waits for a response over the communication channel + pub fn recv(&self) -> Result { + self.hyp_conn.recv() + } + + /// Non-Blocking check for a response over the communication channel + pub fn try_recv(&self) -> Result { + self.hyp_conn.try_recv() + } + + /// Sends an event to the Hypervisor that tells it to resume vCPU execution + /// Note: The method waits for a confirmation message + pub fn resume_vcpu(&mut self) -> Result<(), GdbTargetError> { + log::info!("Resume vCPU execution"); + + match self.send_command(DebugMsg::Continue)? { + DebugResponse::Continue => Ok(()), + msg => { + log::error!("Unexpected message received: {:?}", msg); + Err(GdbTargetError::UnexpectedMessage) + } + } + } + +} + +impl Target for HyperlightSandboxTarget { + type Arch = GdbTargetArch; + type Error = GdbTargetError; + + #[inline(always)] + fn base_ops(&mut self) -> BaseOps { + BaseOps::SingleThread(self) + } +} + +impl SingleThreadBase for HyperlightSandboxTarget { + fn read_addrs( + &mut self, + gva: ::Usize, + data: &mut [u8], + ) -> TargetResult { + log::debug!("Read addr: {:X} len: {:X}", gva, data.len()); + + unimplemented!() + } + + fn write_addrs( + &mut self, + gva: ::Usize, + data: &[u8], + ) -> TargetResult<(), Self> { + log::debug!("Write addr: {:X} len: {:X}", gva, data.len()); + + unimplemented!() + } + + fn read_registers( + &mut self, + _regs: &mut ::Registers, + ) -> TargetResult<(), Self> { + log::debug!("Read regs"); + + unimplemented!() + } + + fn write_registers( + &mut self, + _regs: &::Registers, + ) -> TargetResult<(), Self> { + log::debug!("Write regs"); + + unimplemented!() + } +} diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index f7364689..6ca78664 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -65,6 +65,9 @@ pub(crate) mod crashdump; use std::fmt::Debug; use std::sync::{Arc, Mutex}; +#[cfg(gdb)] +use gdb::VcpuStopReason; + use self::handlers::{ MemAccessHandlerCaller, MemAccessHandlerWrapper, OutBHandlerCaller, OutBHandlerWrapper, }; @@ -89,6 +92,9 @@ pub(crate) const EFER_NX: u64 = 1 << 11; /// These are the generic exit reasons that we can handle from a Hypervisor the Hypervisors run method is responsible for mapping from /// the hypervisor specific exit reasons to these generic ones pub enum HyperlightExit { + #[cfg(gdb)] + /// The vCPU has exited due to a debug event + Debug(VcpuStopReason), /// The vCPU has halted Halt(), /// The vCPU has issued a write to the given port with the given value @@ -193,6 +199,15 @@ pub(crate) trait Hypervisor: Debug + Sync + Send { #[cfg(crashdump)] fn get_memory_regions(&self) -> &[MemoryRegion]; + + #[cfg(gdb)] + /// handles the cases when the vCPU stops due to a Debug event + fn handle_debug( + &mut self, + _stop_reason: VcpuStopReason, + ) -> Result<()> { + unimplemented!() + } } /// A virtual CPU that can be run until an exit occurs @@ -209,6 +224,11 @@ impl VirtualCPU { ) -> Result<()> { loop { match hv.run() { + #[cfg(gdb)] + Ok(HyperlightExit::Debug(stop_reason)) => { + hv.handle_debug(stop_reason)?; + } + Ok(HyperlightExit::Halt()) => { break; } From ba5797127898398d6c9b7e9d6cbcf2a72e3fb121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 21:14:06 +0200 Subject: [PATCH 07/20] handle debug vcpu exit on kvm side MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - adds a specific handler for the vcpu exit debug that waits for debug messages and processes them Signed-off-by: Doru Blânzeanu --- src/hyperlight_host/src/hypervisor/kvm.rs | 115 ++++++++++++++++++++-- 1 file changed, 109 insertions(+), 6 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index af58e11d..87863661 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -24,7 +24,7 @@ use tracing::{instrument, Span}; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] -use super::gdb::create_gdb_thread; +use super::gdb::{create_gdb_thread, DebugMsg, DebugResponse, DebugCommChannel, VcpuStopReason}; use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; use super::{ HyperlightExit, Hypervisor, VirtualCPU, CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, @@ -59,6 +59,60 @@ pub(crate) fn is_hypervisor_present() -> bool { } } +#[cfg(gdb)] +mod debug { + use super::KVMDriver; + use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason}; + use crate::{new_error, Result}; + + + impl KVMDriver { + /// Get the reason the vCPU has stopped + pub fn get_stop_reason(&self) -> Result { + Ok(VcpuStopReason::Unknown) + } + + pub fn process_dbg_request( + &mut self, + req: DebugMsg, + ) -> Result { + match req { + DebugMsg::Continue => { + Ok(DebugResponse::Continue) + } + } + } + + pub fn recv_dbg_msg(&mut self) -> Result { + let gdb_conn = self + .gdb_conn + .as_mut() + .ok_or_else(|| new_error!("Debug is not enabled"))?; + + gdb_conn.recv().map_err(|e| { + new_error!( + "Got an error while waiting to receive a + message: {:?}", + e + ) + }) + } + + pub fn send_dbg_msg(&mut self, cmd: DebugResponse) -> Result<()> { + log::debug!("Sending {:?}", cmd); + + let gdb_conn = self + .gdb_conn + .as_mut() + .ok_or_else(|| new_error!("Debug is not enabled"))?; + + gdb_conn + .send(cmd) + .map_err(|e| new_error!("Got an error while sending a response message {:?}", e)) + } + } +} + /// A Hypervisor driver for KVM on Linux pub(super) struct KVMDriver { _kvm: Kvm, @@ -67,6 +121,9 @@ pub(super) struct KVMDriver { entrypoint: u64, orig_rsp: GuestPtr, mem_regions: Vec, + + #[cfg(gdb)] + gdb_conn: Option>, } impl KVMDriver { @@ -107,19 +164,26 @@ impl KVMDriver { Self::setup_initial_sregs(&mut vcpu_fd, pml4_addr)?; #[cfg(gdb)] - if let Some(DebugInfo { port }) = debug_info { - create_gdb_thread(*port).map_err(|_| new_error!("Cannot create GDB thread"))?; - } + let gdb_conn = if let Some(DebugInfo { port }) = debug_info { + Some(create_gdb_thread(*port).map_err(|_| new_error!("Cannot create GDB thread"))?) + } else { + None + }; let rsp_gp = GuestPtr::try_from(RawPtr::from(rsp))?; - Ok(Self { + + let ret = Self { _kvm: kvm, _vm_fd: vm_fd, vcpu_fd, entrypoint, orig_rsp: rsp_gp, mem_regions, - }) + #[cfg(gdb)] + gdb_conn, + }; + + Ok(ret) } #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] @@ -311,6 +375,12 @@ impl Hypervisor for KVMDriver { None => HyperlightExit::Mmio(addr), } } + #[cfg(gdb)] + Ok(VcpuExit::Debug(_)) => { + let reason = self.get_stop_reason()?; + + HyperlightExit::Debug(reason) + } Err(e) => match e.errno() { // we send a signal to the thread to cancel execution this results in EINTR being returned by KVM so we return Cancelled libc::EINTR => HyperlightExit::Cancelled(), @@ -337,7 +407,40 @@ impl Hypervisor for KVMDriver { fn get_memory_regions(&self) -> &[MemoryRegion] { &self.mem_regions } + + #[cfg(gdb)] + fn handle_debug( + &mut self, + stop_reason: VcpuStopReason, + ) -> Result<()> { + self.send_dbg_msg(DebugResponse::VcpuStopped(stop_reason)) + .map_err(|e| new_error!("Couldn't signal vCPU stopped event to GDB thread: {:?}", e))?; + + loop { + log::debug!("Debug wait for event to resume vCPU"); + // Wait for a message from gdb + let req = self.recv_dbg_msg()?; + + let response = self.process_dbg_request(req)?; + + // If the command was either step or continue, we need to run the vcpu + let cont = matches!( + response, + DebugResponse::Continue + ); + + self.send_dbg_msg(response) + .map_err(|e| new_error!("Couldn't send reponse to gdb: {:?}", e))?; + + if cont { + break; + } + } + + Ok(()) + } } + #[cfg(test)] mod tests { use std::sync::{Arc, Mutex}; From a4172063a0e1a39dd64e1a8637526a35254cb54a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 21:28:38 +0200 Subject: [PATCH 08/20] add kvm debug configuration that handles vCPU interraction 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/kvm.rs | 170 +++++++++++++++++++++- 1 file changed, 167 insertions(+), 3 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 87863661..1e3114f8 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -61,14 +61,166 @@ pub(crate) fn is_hypervisor_present() -> bool { #[cfg(gdb)] mod debug { + use kvm_bindings::{ + kvm_guest_debug, KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_SINGLESTEP, + KVM_GUESTDBG_USE_HW_BP, KVM_GUESTDBG_USE_SW_BP, + }; + use kvm_ioctls::VcpuFd; + use super::KVMDriver; use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason}; use crate::{new_error, Result}; + /// KVM Debug struct + /// This struct is used to abstract the internal details of the kvm + /// guest debugging settings + pub struct KvmDebug { + /// vCPU stepping state + single_step: bool, + + /// Array of addresses for HW breakpoints + hw_breakpoints: Vec, + + /// Sent to KVM for enabling guest debug + pub dbg_cfg: kvm_guest_debug, + } + + impl KvmDebug { + const MAX_NO_OF_HW_BP: usize = 4; + + pub fn new() -> Self { + let dbg = kvm_guest_debug { + control: KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP, + ..Default::default() + }; + + Self { + single_step: false, + hw_breakpoints: vec![], + dbg_cfg: dbg, + } + } + + /// Method to set the kvm debugreg fields for breakpoints + /// The maximum number of supported breakpoints is `Self::MAX_NO_OF_HW_BP` + pub fn set_breakpoints(&mut self, vcpu_fd: &VcpuFd, step: bool) -> Result<()> { + let addrs: &[u64] = &self.hw_breakpoints; + + if addrs.len() >= Self::MAX_NO_OF_HW_BP { + return Err(new_error!( + "The breakpoints array is larger than expected: {} >= {}", + addrs.len(), + Self::MAX_NO_OF_HW_BP + )); + } + + self.dbg_cfg.arch.debugreg = [0; 8]; + for (k, addr) in addrs.iter().enumerate() { + self.dbg_cfg.arch.debugreg[k] = *addr; + self.dbg_cfg.arch.debugreg[7] |= 1 << (k * 2); + } + + if !addrs.is_empty() { + self.dbg_cfg.control |= KVM_GUESTDBG_USE_HW_BP; + } else { + self.dbg_cfg.control &= !KVM_GUESTDBG_USE_HW_BP; + } + + if step { + self.dbg_cfg.control |= KVM_GUESTDBG_SINGLESTEP; + } else { + self.dbg_cfg.control &= !KVM_GUESTDBG_SINGLESTEP; + } + + log::debug!("Setting bp: {:?} cfg: {:?}", addrs, self.dbg_cfg); + vcpu_fd + .set_guest_debug(&self.dbg_cfg) + .map_err(|e| new_error!("Could not set guest debug: {:?}", e))?; + + self.single_step = step; + + Ok(()) + } + } impl KVMDriver { + + /// Returns the instruction pointer from the stopped vCPU + pub fn get_instruction_pointer(&self) -> Result { + let regs = self + .vcpu_fd + .get_regs() + .map_err(|e| new_error!("Could not retrieve registers from vCPU: {:?}", e))?; + + Ok(regs.rip) + } + + /// Sets or clears stepping for vCPU + pub fn set_single_step(&mut self, enable: bool) -> Result<()> { + let debug = self + .debug + .as_mut() + .ok_or_else(|| new_error!("Debug is not enabled"))?; + + debug.set_breakpoints(&self.vcpu_fd, enable) + } + + /// Gdb expects the target to be stopped when connected. + /// This method provides a way to set a breakpoint at the entrypoint + /// it does not keep this breakpoint set after the vcpu already stopped at the address + pub fn set_entrypoint_bp(&self) -> Result<()> { + if self.debug.is_some() { + log::debug!("Setting entrypoint bp {:X}", self.entrypoint); + let mut entrypoint_debug = KvmDebug::new(); + entrypoint_debug.hw_breakpoints.push(self.entrypoint); + entrypoint_debug.set_breakpoints(&self.vcpu_fd, false) + } else { + Ok(()) + } + } + + /// Translates the guest address to physical address + fn translate_gva(&self, gva: u64) -> Result { + let tr = self.vcpu_fd.translate_gva(gva).map_err(|e| { + new_error!( + "Could not translate guest virtual address {:X}: {:?}", + gva, + e + ) + })?; + + if tr.valid == 0 { + Err(new_error!( + "Could not translate guest virtual address {:X}", + gva + )) + } else { + Ok(tr.physical_address) + } + } + /// Get the reason the vCPU has stopped pub fn get_stop_reason(&self) -> Result { + let debug = self + .debug + .as_ref() + .ok_or_else(|| new_error!("Debug is not enabled"))?; + + if debug.single_step { + return Ok(VcpuStopReason::DoneStep); + } + + let ip = self.get_instruction_pointer()?; + let gpa = self.translate_gva(ip)?; + + if debug.hw_breakpoints.contains(&gpa) { + return Ok(VcpuStopReason::HwBp); + } + + if ip == self.entrypoint { + return Ok(VcpuStopReason::HwBp); + } + Ok(VcpuStopReason::Unknown) } @@ -78,6 +230,7 @@ mod debug { ) -> Result { match req { DebugMsg::Continue => { + self.set_single_step(false)?; Ok(DebugResponse::Continue) } } @@ -122,6 +275,8 @@ pub(super) struct KVMDriver { orig_rsp: GuestPtr, mem_regions: Vec, + #[cfg(gdb)] + debug: Option, #[cfg(gdb)] gdb_conn: Option>, } @@ -164,10 +319,13 @@ impl KVMDriver { Self::setup_initial_sregs(&mut vcpu_fd, pml4_addr)?; #[cfg(gdb)] - let gdb_conn = if let Some(DebugInfo { port }) = debug_info { - Some(create_gdb_thread(*port).map_err(|_| new_error!("Cannot create GDB thread"))?) + let (debug, gdb_conn) = if let Some(DebugInfo { port }) = debug_info { + ( + Some(debug::KvmDebug::new()), + Some(create_gdb_thread(*port).map_err(|_| new_error!("Cannot create GDB thread"))?), + ) } else { - None + (None, None) }; let rsp_gp = GuestPtr::try_from(RawPtr::from(rsp))?; @@ -179,10 +337,16 @@ impl KVMDriver { entrypoint, orig_rsp: rsp_gp, mem_regions, + + #[cfg(gdb)] + debug, #[cfg(gdb)] gdb_conn, }; + #[cfg(gdb)] + ret.set_entrypoint_bp()?; + Ok(ret) } From bfe70b09f4eccbe1a3d823aa84a724c51f8bf091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 21:48:27 +0200 Subject: [PATCH 09/20] add read/write registers support 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 | 29 +++++++- .../src/hypervisor/gdb/x86_64_target.rs | 68 +++++++++++++++-- src/hyperlight_host/src/hypervisor/kvm.rs | 73 ++++++++++++++++++- 3 files changed, 160 insertions(+), 10 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index e5819a78..044e4978 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -48,6 +48,29 @@ impl From for TargetError { } } +#[derive(Debug, Default)] +/// Struct that contains the x86_64 core registers +pub struct X86_64Regs { + pub rax: u64, + pub rbx: u64, + pub rcx: u64, + pub rdx: u64, + pub rsi: u64, + pub rdi: u64, + pub rbp: u64, + pub rsp: u64, + pub r8: u64, + pub r9: u64, + pub r10: u64, + pub r11: u64, + pub r12: u64, + pub r13: u64, + pub r14: u64, + pub r15: u64, + pub rip: u64, + pub rflags: u64, +} + #[derive(Debug)] /// Defines the possible reasons for which a vCPU ce be stopped when debugging pub enum VcpuStopReason { @@ -57,19 +80,21 @@ pub enum VcpuStopReason { Unknown, } -#[allow(dead_code)] #[derive(Debug)] /// Enumerates the possible actions that a debugger can ask from a Hypervisor pub enum DebugMsg { Continue, + ReadRegisters, + WriteRegisters(X86_64Regs), } -#[allow(dead_code)] #[derive(Debug)] /// Enumerates the possible responses that a hypervisor can provide to a debugger pub enum DebugResponse { Continue, + ReadRegisters(X86_64Regs), VcpuStopped(VcpuStopReason), + WriteRegisters, } /// Debug communication channel that is used for sending a request type and diff --git a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs index 9c3eaaba..2b9fb5db 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs @@ -2,10 +2,10 @@ use crossbeam_channel::TryRecvError; use gdbstub::arch::Arch; use gdbstub::target::ext::base::singlethread::SingleThreadBase; use gdbstub::target::ext::base::BaseOps; -use gdbstub::target::{Target, TargetResult}; +use gdbstub::target::{Target, TargetError, TargetResult}; use gdbstub_arch::x86::X86_64_SSE as GdbTargetArch; -use super::{DebugMsg, DebugResponse, DebugCommChannel, GdbTargetError}; +use super::{DebugMsg, DebugResponse, DebugCommChannel, GdbTargetError, X86_64Regs}; /// Gdbstub target used by the gdbstub crate to provide GDB protocol implementation pub struct HyperlightSandboxTarget { @@ -90,19 +90,75 @@ impl SingleThreadBase for HyperlightSandboxTarget { fn read_registers( &mut self, - _regs: &mut ::Registers, + regs: &mut ::Registers, ) -> TargetResult<(), Self> { log::debug!("Read regs"); - unimplemented!() + match self.send_command(DebugMsg::ReadRegisters)? { + DebugResponse::ReadRegisters(read_regs) => { + regs.regs[0] = read_regs.rax; + regs.regs[1] = read_regs.rbp; + regs.regs[2] = read_regs.rcx; + regs.regs[3] = read_regs.rdx; + regs.regs[4] = read_regs.rsi; + regs.regs[5] = read_regs.rdi; + regs.regs[6] = read_regs.rbp; + regs.regs[7] = read_regs.rsp; + regs.regs[8] = read_regs.r8; + regs.regs[9] = read_regs.r9; + regs.regs[10] = read_regs.r10; + regs.regs[11] = read_regs.r11; + regs.regs[12] = read_regs.r12; + regs.regs[13] = read_regs.r13; + regs.regs[14] = read_regs.r14; + regs.regs[15] = read_regs.r15; + regs.rip = read_regs.rip; + regs.eflags = u32::try_from(read_regs.rflags) + .expect("Couldn't convert rflags from u64 to u32"); + + Ok(()) + } + + msg => { + log::error!("Unexpected message received: {:?}", msg); + Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) + } + } } fn write_registers( &mut self, - _regs: &::Registers, + regs: &::Registers, ) -> TargetResult<(), Self> { log::debug!("Write regs"); - unimplemented!() + let regs = X86_64Regs { + rax: regs.regs[0], + rbx: regs.regs[1], + rcx: regs.regs[2], + rdx: regs.regs[3], + rsi: regs.regs[4], + rdi: regs.regs[5], + rbp: regs.regs[6], + rsp: regs.regs[7], + r8: regs.regs[8], + r9: regs.regs[9], + r10: regs.regs[10], + r11: regs.regs[11], + r12: regs.regs[12], + r13: regs.regs[13], + r14: regs.regs[14], + r15: regs.regs[15], + rip: regs.rip, + rflags: u64::from(regs.eflags), + }; + + match self.send_command(DebugMsg::WriteRegisters(regs))? { + DebugResponse::WriteRegisters => Ok(()), + msg => { + log::error!("Unexpected message received: {:?}", msg); + Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) + } + } } } diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 1e3114f8..39954cba 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -62,13 +62,13 @@ pub(crate) fn is_hypervisor_present() -> bool { #[cfg(gdb)] mod debug { use kvm_bindings::{ - kvm_guest_debug, KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_SINGLESTEP, + kvm_guest_debug, kvm_regs, KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_SINGLESTEP, KVM_GUESTDBG_USE_HW_BP, KVM_GUESTDBG_USE_SW_BP, }; use kvm_ioctls::VcpuFd; use super::KVMDriver; - use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason}; + use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs}; use crate::{new_error, Result}; /// KVM Debug struct @@ -199,6 +199,66 @@ mod debug { } } + pub fn read_regs(&self, regs: &mut X86_64Regs) -> Result<()> { + log::debug!("Read registers"); + let vcpu_regs = self + .vcpu_fd + .get_regs() + .map_err(|e| new_error!("Could not read guest registers: {:?}", e))?; + + regs.rax = vcpu_regs.rax; + regs.rbx = vcpu_regs.rbx; + regs.rcx = vcpu_regs.rcx; + regs.rdx = vcpu_regs.rdx; + regs.rsi = vcpu_regs.rsi; + regs.rdi = vcpu_regs.rdi; + regs.rbp = vcpu_regs.rbp; + regs.rsp = vcpu_regs.rsp; + regs.r8 = vcpu_regs.r8; + regs.r9 = vcpu_regs.r9; + regs.r10 = vcpu_regs.r10; + regs.r11 = vcpu_regs.r11; + regs.r12 = vcpu_regs.r12; + regs.r13 = vcpu_regs.r13; + regs.r14 = vcpu_regs.r14; + regs.r15 = vcpu_regs.r15; + + regs.rip = vcpu_regs.rip; + regs.rflags = vcpu_regs.rflags; + + Ok(()) + } + + pub fn write_regs(&self, regs: &X86_64Regs) -> Result<()> { + log::debug!("Write registers"); + let new_regs = kvm_regs { + rax: regs.rax, + rbx: regs.rbx, + rcx: regs.rcx, + rdx: regs.rdx, + rsi: regs.rsi, + rdi: regs.rdi, + rbp: regs.rbp, + rsp: regs.rsp, + r8: regs.r8, + r9: regs.r9, + r10: regs.r10, + r11: regs.r11, + r12: regs.r12, + r13: regs.r13, + r14: regs.r14, + r15: regs.r15, + + rip: regs.rip, + rflags: regs.rflags, + }; + + self.vcpu_fd + .set_regs(&new_regs) + .map_err(|e| new_error!("Could not write guest registers: {:?}", e)) + } + + /// Get the reason the vCPU has stopped pub fn get_stop_reason(&self) -> Result { let debug = self @@ -233,6 +293,15 @@ mod debug { self.set_single_step(false)?; Ok(DebugResponse::Continue) } + DebugMsg::ReadRegisters => { + let mut regs = X86_64Regs::default(); + self.read_regs(&mut regs).expect("Read Regs error"); + Ok(DebugResponse::ReadRegisters(regs)) + } + DebugMsg::WriteRegisters(regs) => { + self.write_regs(®s).expect("Write Regs error"); + Ok(DebugResponse::WriteRegisters) + } } } From 5c226574e22d8a13af44c433752e0aacf17cf59d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 21:56:46 +0200 Subject: [PATCH 10/20] hw breakpoint and resume support added 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 | 4 ++ .../src/hypervisor/gdb/x86_64_target.rs | 63 ++++++++++++++++++- src/hyperlight_host/src/hypervisor/kvm.rs | 54 ++++++++++++++++ 3 files changed, 120 insertions(+), 1 deletion(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 044e4978..046d33b8 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -83,16 +83,20 @@ pub enum VcpuStopReason { #[derive(Debug)] /// Enumerates the possible actions that a debugger can ask from a Hypervisor pub enum DebugMsg { + AddHwBreakpoint(u64), Continue, ReadRegisters, + RemoveHwBreakpoint(u64), WriteRegisters(X86_64Regs), } #[derive(Debug)] /// Enumerates the possible responses that a hypervisor can provide to a debugger pub enum DebugResponse { + AddHwBreakpoint(bool), Continue, ReadRegisters(X86_64Regs), + RemoveHwBreakpoint(bool), VcpuStopped(VcpuStopReason), WriteRegisters, } diff --git a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs index 2b9fb5db..942651b0 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs @@ -1,7 +1,13 @@ use crossbeam_channel::TryRecvError; use gdbstub::arch::Arch; -use gdbstub::target::ext::base::singlethread::SingleThreadBase; +use gdbstub::common::Signal; +use gdbstub::target::ext::base::singlethread::{ + SingleThreadBase, SingleThreadResume, SingleThreadResumeOps, +}; use gdbstub::target::ext::base::BaseOps; +use gdbstub::target::ext::breakpoints::{ + Breakpoints, BreakpointsOps, HwBreakpoint, HwBreakpointOps, +}; use gdbstub::target::{Target, TargetError, TargetResult}; use gdbstub_arch::x86::X86_64_SSE as GdbTargetArch; @@ -61,6 +67,10 @@ impl Target for HyperlightSandboxTarget { type Arch = GdbTargetArch; type Error = GdbTargetError; + fn support_breakpoints(&mut self) -> Option> { + Some(self) + } + #[inline(always)] fn base_ops(&mut self) -> BaseOps { BaseOps::SingleThread(self) @@ -161,4 +171,55 @@ impl SingleThreadBase for HyperlightSandboxTarget { } } } + + fn support_resume(&mut self) -> Option> { + Some(self) + } +} + +impl Breakpoints for HyperlightSandboxTarget { + fn support_hw_breakpoint(&mut self) -> Option> { + Some(self) + } +} + +impl HwBreakpoint for HyperlightSandboxTarget { + fn add_hw_breakpoint( + &mut self, + addr: ::Usize, + _kind: ::BreakpointKind, + ) -> TargetResult { + log::debug!("Add hw breakpoint at address {:X}", addr); + + match self.send_command(DebugMsg::AddHwBreakpoint(addr))? { + DebugResponse::AddHwBreakpoint(rsp) => Ok(rsp), + msg => { + log::error!("Unexpected message received: {:?}", msg); + Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) + } + } + } + + fn remove_hw_breakpoint( + &mut self, + addr: ::Usize, + _kind: ::BreakpointKind, + ) -> TargetResult { + log::debug!("Remove hw breakpoint at address {:X}", addr); + + match self.send_command(DebugMsg::RemoveHwBreakpoint(addr))? { + DebugResponse::RemoveHwBreakpoint(rsp) => Ok(rsp), + msg => { + log::error!("Unexpected message received: {:?}", msg); + Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) + } + } + } +} + +impl SingleThreadResume for HyperlightSandboxTarget { + fn resume(&mut self, _signal: Option) -> Result<(), Self::Error> { + log::debug!("Resume"); + self.resume_vcpu() + } } diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 39954cba..583ba30f 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -258,6 +258,48 @@ mod debug { .map_err(|e| new_error!("Could not write guest registers: {:?}", e)) } + pub fn add_hw_breakpoint(&mut self, addr: u64) -> Result { + let addr = self.translate_gva(addr)?; + + if let Some(debug) = self.debug.as_mut() { + if debug.hw_breakpoints.contains(&addr) { + Ok(true) + } else if debug.hw_breakpoints.len() >= KvmDebug::MAX_NO_OF_HW_BP { + Ok(false) + } else { + debug.hw_breakpoints.push(addr); + debug.set_breakpoints(&self.vcpu_fd, false)?; + + Ok(true) + } + } else { + Ok(false) + } + } + + pub fn remove_hw_breakpoint(&mut self, addr: u64) -> Result { + let addr = self.translate_gva(addr)?; + + if let Some(debug) = self.debug.as_mut() { + if debug.hw_breakpoints.contains(&addr) { + let index = debug + .hw_breakpoints + .iter() + .position(|a| *a == addr) + .unwrap(); + debug.hw_breakpoints.copy_within(index + 1.., index); + debug.hw_breakpoints.pop(); + debug.set_breakpoints(&self.vcpu_fd, false)?; + + Ok(true) + } else { + Ok(false) + } + } else { + Ok(false) + } + } + /// Get the reason the vCPU has stopped pub fn get_stop_reason(&self) -> Result { @@ -289,6 +331,12 @@ mod debug { req: DebugMsg, ) -> Result { match req { + DebugMsg::AddHwBreakpoint(addr) => { + let res = self + .add_hw_breakpoint(addr) + .expect("Add hw breakpoint error"); + Ok(DebugResponse::AddHwBreakpoint(res)) + } DebugMsg::Continue => { self.set_single_step(false)?; Ok(DebugResponse::Continue) @@ -298,6 +346,12 @@ mod debug { self.read_regs(&mut regs).expect("Read Regs error"); Ok(DebugResponse::ReadRegisters(regs)) } + DebugMsg::RemoveHwBreakpoint(addr) => { + let res = self + .remove_hw_breakpoint(addr) + .expect("Remove hw breakpoint error"); + Ok(DebugResponse::RemoveHwBreakpoint(res)) + } DebugMsg::WriteRegisters(regs) => { self.write_regs(®s).expect("Write Regs error"); Ok(DebugResponse::WriteRegisters) From a1b0a5d66d2473a258598390f167684b2b68a2b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 22:14:36 +0200 Subject: [PATCH 11/20] add stepping support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - also adds handling of gdb client disconnect by resuming execution Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/event_loop.rs | 3 ++ src/hyperlight_host/src/hypervisor/gdb/mod.rs | 4 +++ .../src/hypervisor/gdb/x86_64_target.rs | 36 ++++++++++++++++++- src/hyperlight_host/src/hypervisor/kvm.rs | 18 +++++++++- 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs index 716f54e9..ce1a7092 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs @@ -83,6 +83,9 @@ pub fn event_loop_thread( Ok(disconnect_reason) => match disconnect_reason { DisconnectReason::Disconnect => { log::info!("Gdb client disconnected"); + if let Err(e) = target.disable_debug() { + log::error!("Cannot disable debugging: {:?}", e); + } } DisconnectReason::TargetExited(_) => { log::info!("Guest finalized execution and disconnected"); diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 046d33b8..47d4cc91 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -85,8 +85,10 @@ pub enum VcpuStopReason { pub enum DebugMsg { AddHwBreakpoint(u64), Continue, + DisableDebug, ReadRegisters, RemoveHwBreakpoint(u64), + Step, WriteRegisters(X86_64Regs), } @@ -95,8 +97,10 @@ pub enum DebugMsg { pub enum DebugResponse { AddHwBreakpoint(bool), Continue, + DisableDebug, ReadRegisters(X86_64Regs), RemoveHwBreakpoint(bool), + Step, VcpuStopped(VcpuStopReason), WriteRegisters, } diff --git a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs index 942651b0..1867e1a1 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs @@ -2,7 +2,8 @@ use crossbeam_channel::TryRecvError; use gdbstub::arch::Arch; use gdbstub::common::Signal; use gdbstub::target::ext::base::singlethread::{ - SingleThreadBase, SingleThreadResume, SingleThreadResumeOps, + SingleThreadBase, SingleThreadResume, SingleThreadResumeOps, SingleThreadSingleStep, + SingleThreadSingleStepOps, }; use gdbstub::target::ext::base::BaseOps; use gdbstub::target::ext::breakpoints::{ @@ -61,6 +62,20 @@ impl HyperlightSandboxTarget { } } + /// Sends an event to the Hypervisor that tells it to disable debugging + /// and continue executing until end + /// Note: The method waits for a confirmation message + pub fn disable_debug(&mut self) -> Result<(), GdbTargetError> { + log::info!("Disable debugging and continue until end"); + + match self.send_command(DebugMsg::DisableDebug)? { + DebugResponse::DisableDebug => Ok(()), + msg => { + log::error!("Unexpected message received: {:?}", msg); + Err(GdbTargetError::UnexpectedMessage) + } + } + } } impl Target for HyperlightSandboxTarget { @@ -222,4 +237,23 @@ impl SingleThreadResume for HyperlightSandboxTarget { log::debug!("Resume"); self.resume_vcpu() } + fn support_single_step(&mut self) -> Option> { + Some(self) + } +} + +impl SingleThreadSingleStep for HyperlightSandboxTarget { + fn step(&mut self, signal: Option) -> Result<(), Self::Error> { + assert!(signal.is_none()); + + log::debug!("Step"); + match self.send_command(DebugMsg::Step)? { + DebugResponse::Step => Ok(()), + msg => { + log::error!("Unexpected message received: {:?}", msg); + + Err(GdbTargetError::UnexpectedMessage) + } + } + } } diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 583ba30f..7cd9594a 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -74,6 +74,7 @@ mod debug { /// KVM Debug struct /// This struct is used to abstract the internal details of the kvm /// guest debugging settings + #[derive(Default)] pub struct KvmDebug { /// vCPU stepping state single_step: bool, @@ -144,6 +145,12 @@ mod debug { } impl KVMDriver { + /// Resets the debug information to disable debugging + fn disable_debug(&mut self) -> Result<()> { + self.debug = Some(KvmDebug::default()); + + self.set_single_step(false) + } /// Returns the instruction pointer from the stopped vCPU pub fn get_instruction_pointer(&self) -> Result { @@ -341,6 +348,11 @@ mod debug { self.set_single_step(false)?; Ok(DebugResponse::Continue) } + DebugMsg::DisableDebug => { + self.disable_debug()?; + + Ok(DebugResponse::DisableDebug) + } DebugMsg::ReadRegisters => { let mut regs = X86_64Regs::default(); self.read_regs(&mut regs).expect("Read Regs error"); @@ -352,6 +364,10 @@ mod debug { .expect("Remove hw breakpoint error"); Ok(DebugResponse::RemoveHwBreakpoint(res)) } + DebugMsg::Step => { + self.set_single_step(true)?; + Ok(DebugResponse::Step) + } DebugMsg::WriteRegisters(regs) => { self.write_regs(®s).expect("Write Regs error"); Ok(DebugResponse::WriteRegisters) @@ -713,7 +729,7 @@ impl Hypervisor for KVMDriver { // If the command was either step or continue, we need to run the vcpu let cont = matches!( response, - DebugResponse::Continue + DebugResponse::Step | DebugResponse::Continue | DebugResponse::DisableDebug ); self.send_dbg_msg(response) From d9a364b3394105d3da462d9ca568bca00c404970 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 22:31:59 +0200 Subject: [PATCH 12/20] add a way to read/write host shared mem when debugging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/handlers.rs | 93 +++++++++++++++++++ .../src/hypervisor/hyperv_linux.rs | 8 ++ .../src/hypervisor/hyperv_windows.rs | 7 ++ .../src/hypervisor/hypervisor_handler.rs | 10 ++ .../src/hypervisor/inprocess.rs | 4 + src/hyperlight_host/src/hypervisor/kvm.rs | 44 ++++++++- src/hyperlight_host/src/hypervisor/mod.rs | 13 ++- src/hyperlight_host/src/mem/layout.rs | 2 +- src/hyperlight_host/src/mem/shared_mem.rs | 2 +- src/hyperlight_host/src/sandbox/mem_access.rs | 57 +++++++++++- .../src/sandbox/uninitialized.rs | 2 + .../src/sandbox/uninitialized_evolve.rs | 7 ++ 12 files changed, 242 insertions(+), 7 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/handlers.rs b/src/hyperlight_host/src/hypervisor/handlers.rs index b17fabfc..3ee74f0e 100644 --- a/src/hyperlight_host/src/hypervisor/handlers.rs +++ b/src/hyperlight_host/src/hypervisor/handlers.rs @@ -102,3 +102,96 @@ impl MemAccessHandlerCaller for MemAccessHandler { func() } } + +/// The trait representing custom logic to handle the case when +/// a Hypervisor's virtual CPU (vCPU) informs Hyperlight a debug memory access +/// has been requested. +pub trait DbgMemAccessHandlerCaller: Send { + /// Function that gets called when a read is requested. + fn read(&mut self, addr: usize, data: &mut [u8]) -> Result<()>; + + /// Function that gets called when a write is requested. + fn write(&mut self, addr: usize, data: &[u8]) -> Result<()>; + + /// Function that gets called for a request to get guest code offset. + fn get_code_offset(&mut self) -> Result; +} + +/// A convenient type representing a common way `DbgMemAccessHandler` implementations +/// are passed as parameters to functions +/// +/// Note: This needs to be wrapped in a Mutex to be able to grab a mutable +/// reference to the underlying data +pub type DbgMemAccessHandlerWrapper = Arc>; + +pub(crate) type DbgReadMemAccessHandlerFunction = + Box Result<()> + Send>; +pub(crate) type DbgWriteMemAccessHandlerFunction = + Box Result<()> + Send>; +pub(crate) type DbgGetCodeAddrHandlerFunction = Box Result + Send>; + +/// A `DbgMemAccessHandler` implementation using +/// (`DbgReadMemAccessHandlerFunction`, `DbgWriteMemAccessHandlerFunction `, `DbgGetCodeAddrHandlerFunction `). +/// +/// Note: This handler must live for as long as its Sandbox or for +/// static in the case of its C API usage. +pub(crate) struct DbgMemAccessHandler ( + Arc>, + Arc>, + Arc>, +); + +impl + From<( + DbgReadMemAccessHandlerFunction, + DbgWriteMemAccessHandlerFunction, + DbgGetCodeAddrHandlerFunction, + )> for DbgMemAccessHandler +{ + #[instrument(skip_all, parent = Span::current(), level= "Trace")] + fn from( + (f1, f2, f3): ( + DbgReadMemAccessHandlerFunction, + DbgWriteMemAccessHandlerFunction, + DbgGetCodeAddrHandlerFunction, + ), + ) -> Self { + Self( + Arc::new(Mutex::new(f1)), + Arc::new(Mutex::new(f2)), + Arc::new(Mutex::new(f3)), + ) + } +} + +impl DbgMemAccessHandlerCaller for DbgMemAccessHandler { + #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] + fn read(&mut self, addr: usize, data: &mut [u8]) -> Result<()> { + let mut read_func = self + .0 + .try_lock() + .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?; + + read_func(addr, data) + } + + #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] + fn write(&mut self, addr: usize, data: &[u8]) -> Result<()> { + let mut write_func = self + .1 + .try_lock() + .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?; + + write_func(addr, data) + } + + #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] + fn get_code_offset(&mut self) -> Result { + let mut get_code_offset_func = self + .2 + .try_lock() + .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?; + + get_code_offset_func() + } +} diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index 33798d88..882b1c12 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -44,6 +44,8 @@ use mshv_ioctls::{Mshv, VcpuFd, VmFd}; use tracing::{instrument, Span}; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; +#[cfg(gdb)] +use super::handlers::DbgMemAccessHandlerWrapper; use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; use super::{ Hypervisor, VirtualCPU, CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, CR4_OSFXSR, @@ -203,6 +205,7 @@ impl Hypervisor for HypervLinuxDriver { outb_hdl: OutBHandlerWrapper, mem_access_hdl: MemAccessHandlerWrapper, hv_handler: Option, + #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { let regs = StandardRegisters { rip: self.entrypoint, @@ -224,6 +227,8 @@ impl Hypervisor for HypervLinuxDriver { hv_handler, outb_hdl, mem_access_hdl, + #[cfg(gdb)] + dbg_mem_access_fn, )?; // reset RSP to what it was before initialise @@ -242,6 +247,7 @@ impl Hypervisor for HypervLinuxDriver { outb_handle_fn: OutBHandlerWrapper, mem_access_fn: MemAccessHandlerWrapper, hv_handler: Option, + #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { // Reset general purpose registers except RSP, then set RIP let rsp_before = self.vcpu_fd.get_regs()?.rsp; @@ -268,6 +274,8 @@ impl Hypervisor for HypervLinuxDriver { hv_handler, outb_handle_fn, mem_access_fn, + #[cfg(gdb)] + dbg_mem_access_fn, )?; // reset RSP to what it was before function call diff --git a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs index 067d0cb1..947fe089 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::{ }; use super::fpu::{FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; +#[cfg(gdb)] +use super::handlers::DbgMemAccessHandlerWrapper; use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; use super::surrogate_process::SurrogateProcess; use super::surrogate_process_manager::*; @@ -305,6 +307,7 @@ impl Hypervisor for HypervWindowsDriver { outb_hdl: OutBHandlerWrapper, mem_access_hdl: MemAccessHandlerWrapper, hv_handler: Option, + #[cfg(gdb)] dbg_mem_access_hdl: DbgMemAccessHandlerWrapper, ) -> Result<()> { let regs = WHvGeneralRegisters { rip: self.entrypoint, @@ -326,6 +329,8 @@ impl Hypervisor for HypervWindowsDriver { hv_handler, outb_hdl, mem_access_hdl, + #[cfg(gdb)] + dbg_mem_access_hdl, )?; // reset RSP to what it was before initialise @@ -344,6 +349,7 @@ impl Hypervisor for HypervWindowsDriver { outb_hdl: OutBHandlerWrapper, mem_access_hdl: MemAccessHandlerWrapper, hv_handler: Option, + #[cfg(gdb)] dbg_mem_access_hdl: DbgMemAccessHandlerWrapper, ) -> Result<()> { // Reset general purpose registers except RSP, then set RIP let rsp_before = self.processor.get_regs()?.rsp; @@ -368,6 +374,7 @@ impl Hypervisor for HypervWindowsDriver { hv_handler, outb_hdl, mem_access_hdl, + dbg_mem_access_hdl, )?; // reset RSP to what it was before function call diff --git a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs index 737eada6..4bf3af98 100644 --- a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs +++ b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs @@ -37,6 +37,8 @@ use windows::Win32::System::Hypervisor::{WHvCancelRunVirtualProcessor, WHV_PARTI #[cfg(feature = "function_call_metrics")] use crate::histogram_vec_observe; +#[cfg(gdb)] +use crate::hypervisor::handlers::DbgMemAccessHandlerWrapper; use crate::hypervisor::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; #[cfg(target_os = "windows")] use crate::hypervisor::wrappers::HandleWrapper; @@ -187,6 +189,8 @@ pub(crate) struct HvHandlerConfig { pub(crate) outb_handler: OutBHandlerWrapper, pub(crate) mem_access_handler: MemAccessHandlerWrapper, pub(crate) max_wait_for_cancellation: Duration, + #[cfg(gdb)] + pub(crate) dbg_mem_access_handler: DbgMemAccessHandlerWrapper, } impl HypervisorHandler { @@ -351,6 +355,8 @@ impl HypervisorHandler { configuration.outb_handler.clone(), configuration.mem_access_handler.clone(), Some(hv_handler_clone.clone()), + #[cfg(gdb)] + configuration.dbg_mem_access_handler.clone(), ); drop(mem_lock_guard); drop(evar_lock_guard); @@ -436,6 +442,8 @@ impl HypervisorHandler { configuration.outb_handler.clone(), configuration.mem_access_handler.clone(), Some(hv_handler_clone.clone()), + #[cfg(gdb)] + configuration.dbg_mem_access_handler.clone(), ); histogram_vec_observe!( &GuestFunctionCallDurationMicroseconds, @@ -451,6 +459,8 @@ impl HypervisorHandler { configuration.outb_handler.clone(), configuration.mem_access_handler.clone(), Some(hv_handler_clone.clone()), + #[cfg(gdb)] + configuration.dbg_mem_access_handler.clone(), ) }; drop(mem_lock_guard); diff --git a/src/hyperlight_host/src/hypervisor/inprocess.rs b/src/hyperlight_host/src/hypervisor/inprocess.rs index f7076a5b..1fec3df5 100644 --- a/src/hyperlight_host/src/hypervisor/inprocess.rs +++ b/src/hyperlight_host/src/hypervisor/inprocess.rs @@ -17,6 +17,8 @@ limitations under the License. use std::fmt::Debug; use std::os::raw::c_void; +#[cfg(gdb)] +use super::handlers::DbgMemAccessHandlerWrapper; use super::{HyperlightExit, Hypervisor}; #[cfg(crashdump)] use crate::mem::memory_region::MemoryRegion; @@ -73,6 +75,7 @@ impl<'a> Hypervisor for InprocessDriver<'a> { _outb_handle_fn: super::handlers::OutBHandlerWrapper, _mem_access_fn: super::handlers::MemAccessHandlerWrapper, _hv_handler: Option, + #[cfg(gdb)] _dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> crate::Result<()> { let entrypoint_fn: extern "win64" fn(u64, u64, u64, u64) = unsafe { std::mem::transmute(self.args.entrypoint_raw as *const c_void) }; @@ -93,6 +96,7 @@ impl<'a> Hypervisor for InprocessDriver<'a> { _outb_handle_fn: super::handlers::OutBHandlerWrapper, _mem_access_fn: super::handlers::MemAccessHandlerWrapper, _hv_handler: Option, + #[cfg(gdb)] _dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> crate::Result<()> { let ptr: u64 = dispatch_func_addr.into(); let dispatch_func: extern "win64" fn() = diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 7cd9594a..4a6bd455 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -16,6 +16,8 @@ limitations under the License. use std::convert::TryFrom; use std::fmt::Debug; +#[cfg(gdb)] +use std::sync::{Arc, Mutex}; use kvm_bindings::{kvm_fpu, kvm_regs, kvm_userspace_memory_region, KVM_MEM_READONLY}; use kvm_ioctls::Cap::UserMemory; @@ -24,7 +26,9 @@ use tracing::{instrument, Span}; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] -use super::gdb::{create_gdb_thread, DebugMsg, DebugResponse, DebugCommChannel, VcpuStopReason}; +use super::gdb::{create_gdb_thread, DebugCommChannel, DebugMsg, DebugResponse, VcpuStopReason}; +#[cfg(gdb)] +use super::handlers::DbgMemAccessHandlerWrapper; use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; use super::{ HyperlightExit, Hypervisor, VirtualCPU, CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, @@ -61,6 +65,7 @@ pub(crate) fn is_hypervisor_present() -> bool { #[cfg(gdb)] mod debug { + use std::sync::{Arc, Mutex}; use kvm_bindings::{ kvm_guest_debug, kvm_regs, KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_SINGLESTEP, KVM_GUESTDBG_USE_HW_BP, KVM_GUESTDBG_USE_SW_BP, @@ -69,6 +74,7 @@ mod debug { use super::KVMDriver; use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs}; + use crate::hypervisor::handlers::DbgMemAccessHandlerCaller; use crate::{new_error, Result}; /// KVM Debug struct @@ -336,6 +342,7 @@ mod debug { pub fn process_dbg_request( &mut self, req: DebugMsg, + _dbg_mem_access_fn: Arc>, ) -> Result { match req { DebugMsg::AddHwBreakpoint(addr) => { @@ -541,6 +548,7 @@ impl Hypervisor for KVMDriver { outb_hdl: OutBHandlerWrapper, mem_access_hdl: MemAccessHandlerWrapper, hv_handler: Option, + #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { let regs = kvm_regs { rip: self.entrypoint, @@ -561,6 +569,8 @@ impl Hypervisor for KVMDriver { hv_handler, outb_hdl, mem_access_hdl, + #[cfg(gdb)] + dbg_mem_access_fn, )?; // reset RSP to what it was before initialise @@ -578,6 +588,7 @@ impl Hypervisor for KVMDriver { outb_handle_fn: OutBHandlerWrapper, mem_access_fn: MemAccessHandlerWrapper, hv_handler: Option, + #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { // Reset general purpose registers except RSP, then set RIP let rsp_before = self.vcpu_fd.get_regs()?.rsp; @@ -603,6 +614,8 @@ impl Hypervisor for KVMDriver { hv_handler, outb_handle_fn, mem_access_fn, + #[cfg(gdb)] + dbg_mem_access_fn, )?; // reset RSP to what it was before function call @@ -714,6 +727,7 @@ impl Hypervisor for KVMDriver { #[cfg(gdb)] fn handle_debug( &mut self, + dbg_mem_access_fn: Arc>, stop_reason: VcpuStopReason, ) -> Result<()> { self.send_dbg_msg(DebugResponse::VcpuStopped(stop_reason)) @@ -724,7 +738,7 @@ impl Hypervisor for KVMDriver { // Wait for a message from gdb let req = self.recv_dbg_msg()?; - let response = self.process_dbg_request(req)?; + let response = self.process_dbg_request(req, dbg_mem_access_fn.clone())?; // If the command was either step or continue, we need to run the vcpu let cont = matches!( @@ -748,6 +762,8 @@ impl Hypervisor for KVMDriver { mod tests { use std::sync::{Arc, Mutex}; + #[cfg(gdb)] + use crate::hypervisor::handlers::DbgMemAccessHandler; use crate::hypervisor::handlers::{MemAccessHandler, OutBHandler}; use crate::hypervisor::tests::test_initialise; use crate::Result; @@ -767,6 +783,28 @@ mod tests { let func: Box Result<()> + Send> = Box::new(|| -> Result<()> { Ok(()) }); Arc::new(Mutex::new(MemAccessHandler::from(func))) }; - test_initialise(outb_handler, mem_access_handler).unwrap(); + #[cfg(gdb)] + #[allow(clippy::type_complexity)] + let dbg_mem_access_handler = { + let read: Box Result<()> + Send> = + Box::new(|_: usize, _: &mut [u8]| -> Result<()> { Ok(()) }); + let write: Box Result<()> + Send> = + Box::new(|_: usize, _: &[u8]| -> Result<()> { Ok(()) }); + let get_code_offset: Box Result + Send> = + Box::new(|| -> Result { Ok(0) }); + Arc::new(Mutex::new(DbgMemAccessHandler::from(( + read, + write, + get_code_offset, + )))) + }; + + test_initialise( + outb_handler, + mem_access_handler, + #[cfg(gdb)] + dbg_mem_access_handler, + ) + .unwrap(); } } diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index 6ca78664..9a24679d 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -68,6 +68,8 @@ use std::sync::{Arc, Mutex}; #[cfg(gdb)] use gdb::VcpuStopReason; +#[cfg(gdb)] +use self::handlers::{DbgMemAccessHandlerCaller, DbgMemAccessHandlerWrapper}; use self::handlers::{ MemAccessHandlerCaller, MemAccessHandlerWrapper, OutBHandlerCaller, OutBHandlerWrapper, }; @@ -128,6 +130,7 @@ pub(crate) trait Hypervisor: Debug + Sync + Send { outb_handle_fn: OutBHandlerWrapper, mem_access_fn: MemAccessHandlerWrapper, hv_handler: Option, + #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()>; /// Dispatch a call from the host to the guest using the given pointer @@ -143,6 +146,7 @@ pub(crate) trait Hypervisor: Debug + Sync + Send { outb_handle_fn: OutBHandlerWrapper, mem_access_fn: MemAccessHandlerWrapper, hv_handler: Option, + #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()>; /// Handle an IO exit from the internally stored vCPU. @@ -204,6 +208,7 @@ pub(crate) trait Hypervisor: Debug + Sync + Send { /// handles the cases when the vCPU stops due to a Debug event fn handle_debug( &mut self, + _dbg_mem_access_fn: Arc>, _stop_reason: VcpuStopReason, ) -> Result<()> { unimplemented!() @@ -221,12 +226,13 @@ impl VirtualCPU { hv_handler: Option, outb_handle_fn: Arc>, mem_access_fn: Arc>, + #[cfg(gdb)] dbg_mem_access_fn: Arc>, ) -> Result<()> { loop { match hv.run() { #[cfg(gdb)] Ok(HyperlightExit::Debug(stop_reason)) => { - hv.handle_debug(stop_reason)?; + hv.handle_debug(dbg_mem_access_fn.clone(), stop_reason)?; } Ok(HyperlightExit::Halt()) => { @@ -301,6 +307,8 @@ pub(crate) mod tests { use hyperlight_testing::dummy_guest_as_string; + #[cfg(gdb)] + use super::handlers::DbgMemAccessHandlerWrapper; use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; use crate::hypervisor::hypervisor_handler::{ HvHandlerConfig, HypervisorHandler, HypervisorHandlerAction, @@ -313,6 +321,7 @@ pub(crate) mod tests { pub(crate) fn test_initialise( outb_hdl: OutBHandlerWrapper, mem_access_hdl: MemAccessHandlerWrapper, + #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { let filename = dummy_guest_as_string().map_err(|e| new_error!("{}", e))?; if !Path::new(&filename).exists() { @@ -330,6 +339,8 @@ pub(crate) mod tests { let hv_handler_config = HvHandlerConfig { outb_handler: outb_hdl, mem_access_handler: mem_access_hdl, + #[cfg(gdb)] + dbg_mem_access_handler: dbg_mem_access_fn, seed: 1234567890, page_size: 4096, peb_addr: RawPtr::from(0x230000), diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index 251032a2..3a13879c 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -656,7 +656,7 @@ impl SandboxMemoryLayout { /// Get the guest address of the code section in the sandbox #[instrument(skip_all, parent = Span::current(), level= "Trace")] - pub(super) fn get_guest_code_address(&self) -> usize { + pub(crate) fn get_guest_code_address(&self) -> usize { Self::BASE_ADDRESS + self.guest_code_offset } diff --git a/src/hyperlight_host/src/mem/shared_mem.rs b/src/hyperlight_host/src/mem/shared_mem.rs index 2bc4be31..8531ff72 100644 --- a/src/hyperlight_host/src/mem/shared_mem.rs +++ b/src/hyperlight_host/src/mem/shared_mem.rs @@ -826,7 +826,7 @@ impl HostSharedMemory { Ok(()) } - /// /Copy the contents of the sandbox at the specified offset into + /// Copy the contents of the sandbox at the specified offset into /// the slice pub fn copy_from_slice(&self, slice: &[u8], offset: usize) -> Result<()> { bounds_check!(offset, slice.len(), self.mem_size()); diff --git a/src/hyperlight_host/src/sandbox/mem_access.rs b/src/hyperlight_host/src/sandbox/mem_access.rs index 6a1e6b70..69e6eb61 100644 --- a/src/hyperlight_host/src/sandbox/mem_access.rs +++ b/src/hyperlight_host/src/sandbox/mem_access.rs @@ -21,7 +21,9 @@ use tracing::{instrument, Span}; use super::mem_mgr::MemMgrWrapper; use crate::error::HyperlightError::StackOverflow; use crate::hypervisor::handlers::{ - MemAccessHandler, MemAccessHandlerFunction, MemAccessHandlerWrapper, + DbgGetCodeAddrHandlerFunction, DbgMemAccessHandler, DbgMemAccessHandlerWrapper, + DbgReadMemAccessHandlerFunction, DbgWriteMemAccessHandlerFunction, MemAccessHandler, + MemAccessHandlerFunction, MemAccessHandlerWrapper, }; use crate::mem::shared_mem::HostSharedMemory; use crate::{log_then_return, Result}; @@ -44,3 +46,56 @@ pub(crate) fn mem_access_handler_wrapper( let mem_access_hdl = MemAccessHandler::from(mem_access_func); Arc::new(Mutex::new(mem_access_hdl)) } + +#[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] +pub(super) fn handle_dbg_read_mem_access_impl( + wrapper: &mut MemMgrWrapper, + addr: usize, + data: &mut [u8], +) -> Result<()> { + wrapper + .unwrap_mgr_mut() + .get_shared_mem_mut() + .copy_to_slice(data, addr) +} + +#[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] +pub(super) fn handle_dbg_write_mem_access_impl( + wrapper: &mut MemMgrWrapper, + addr: usize, + data: &[u8], +) -> Result<()> { + wrapper + .unwrap_mgr_mut() + .get_shared_mem_mut() + .copy_from_slice(data, addr) +} + +#[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] +pub(super) fn handle_dbg_get_code_addr_impl( + wrapper: &mut MemMgrWrapper, +) -> Result { + Ok(wrapper.unwrap_mgr().layout.get_guest_code_address()) +} + +#[instrument(skip_all, parent = Span::current(), level= "Trace")] +pub(crate) fn dbg_mem_access_handler_wrapper( + mut wrapper: MemMgrWrapper, +) -> DbgMemAccessHandlerWrapper { + let mut wrapper2 = wrapper.clone(); + let mut wrapper3 = wrapper.clone(); + let read_access_func: DbgReadMemAccessHandlerFunction = + Box::new(move |addr: usize, data: &mut [u8]| { + handle_dbg_read_mem_access_impl(&mut wrapper, addr, data) + }); + let write_access_func: DbgWriteMemAccessHandlerFunction = + Box::new(move |addr: usize, data: &[u8]| { + handle_dbg_write_mem_access_impl(&mut wrapper2, addr, data) + }); + let get_code_addr_func: DbgGetCodeAddrHandlerFunction = + Box::new(move || handle_dbg_get_code_addr_impl(&mut wrapper3)); + + let dbg_mem_access_hdl = + DbgMemAccessHandler::from((read_access_func, write_access_func, get_code_addr_func)); + Arc::new(Mutex::new(dbg_mem_access_hdl)) +} diff --git a/src/hyperlight_host/src/sandbox/uninitialized.rs b/src/hyperlight_host/src/sandbox/uninitialized.rs index 66f33fa9..6cac13b5 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized.rs @@ -170,10 +170,12 @@ impl UninitializedSandbox { } let sandbox_cfg = cfg.unwrap_or_default(); + #[cfg(gdb)] let debug_info = sandbox_cfg .get_guest_debug_port() .map(|port| DebugInfo { port }); + let mut mem_mgr_wrapper = { let mut mgr = UninitializedSandbox::load_guest_binary( sandbox_cfg, diff --git a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs index 559ddeae..6e8118c0 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs @@ -20,6 +20,8 @@ use std::sync::{Arc, Mutex}; use rand::Rng; use tracing::{instrument, Span}; +#[cfg(gdb)] +use super::mem_access::dbg_mem_access_handler_wrapper; #[cfg(gdb)] use super::uninitialized::DebugInfo; use crate::hypervisor::hypervisor_handler::{ @@ -106,6 +108,9 @@ fn hv_init( ) -> Result { let outb_hdl = outb_handler_wrapper(hshm.clone(), host_funcs); let mem_access_hdl = mem_access_handler_wrapper(hshm.clone()); + #[cfg(gdb)] + let dbg_mem_access_hdl = dbg_mem_access_handler_wrapper(hshm.clone()); + let seed = { let mut rng = rand::rng(); rng.random::() @@ -118,6 +123,8 @@ fn hv_init( let hv_handler_config = HvHandlerConfig { outb_handler: outb_hdl, mem_access_handler: mem_access_hdl, + #[cfg(gdb)] + dbg_mem_access_handler: dbg_mem_access_hdl, seed, page_size, peb_addr, From e550a84b610b8bf8fb1415bf57663bc6ed52d859 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 22:36:38 +0200 Subject: [PATCH 13/20] add read/write address and text section offset support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/event_loop.rs | 2 +- src/hyperlight_host/src/hypervisor/gdb/mod.rs | 6 ++ .../src/hypervisor/gdb/x86_64_target.rs | 46 +++++++++- src/hyperlight_host/src/hypervisor/kvm.rs | 90 ++++++++++++++++++- 4 files changed, 139 insertions(+), 5 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs index ce1a7092..1a66b589 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs @@ -81,7 +81,7 @@ pub fn event_loop_thread( ) { match debugger.run_blocking::(target) { Ok(disconnect_reason) => match disconnect_reason { - DisconnectReason::Disconnect => { + DisconnectReason::Disconnect => { log::info!("Gdb client disconnected"); if let Err(e) = target.disable_debug() { log::error!("Cannot disable debugging: {:?}", e); diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 47d4cc91..57d0e345 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -86,9 +86,12 @@ pub enum DebugMsg { AddHwBreakpoint(u64), Continue, DisableDebug, + GetCodeSectionOffset, + ReadAddr(u64, usize), ReadRegisters, RemoveHwBreakpoint(u64), Step, + WriteAddr(u64, Vec), WriteRegisters(X86_64Regs), } @@ -98,10 +101,13 @@ pub enum DebugResponse { AddHwBreakpoint(bool), Continue, DisableDebug, + GetCodeSectionOffset(u64), + ReadAddr(Vec), ReadRegisters(X86_64Regs), RemoveHwBreakpoint(bool), Step, VcpuStopped(VcpuStopReason), + WriteAddr, WriteRegisters, } diff --git a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs index 1867e1a1..6f24e4c2 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs @@ -9,10 +9,11 @@ use gdbstub::target::ext::base::BaseOps; use gdbstub::target::ext::breakpoints::{ Breakpoints, BreakpointsOps, HwBreakpoint, HwBreakpointOps, }; +use gdbstub::target::ext::section_offsets::{Offsets, SectionOffsets}; use gdbstub::target::{Target, TargetError, TargetResult}; use gdbstub_arch::x86::X86_64_SSE as GdbTargetArch; -use super::{DebugMsg, DebugResponse, DebugCommChannel, GdbTargetError, X86_64Regs}; +use super::{DebugCommChannel, DebugMsg, DebugResponse, GdbTargetError, X86_64Regs}; /// Gdbstub target used by the gdbstub crate to provide GDB protocol implementation pub struct HyperlightSandboxTarget { @@ -90,6 +91,12 @@ impl Target for HyperlightSandboxTarget { fn base_ops(&mut self) -> BaseOps { BaseOps::SingleThread(self) } + + fn support_section_offsets( + &mut self, + ) -> Option> { + Some(self) + } } impl SingleThreadBase for HyperlightSandboxTarget { @@ -100,7 +107,17 @@ impl SingleThreadBase for HyperlightSandboxTarget { ) -> TargetResult { log::debug!("Read addr: {:X} len: {:X}", gva, data.len()); - unimplemented!() + match self.send_command(DebugMsg::ReadAddr(gva, data.len()))? { + DebugResponse::ReadAddr(v) => { + data.copy_from_slice(&v); + + Ok(v.len()) + } + msg => { + log::error!("Unexpected message received: {:?}", msg); + Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) + } + } } fn write_addrs( @@ -109,8 +126,15 @@ impl SingleThreadBase for HyperlightSandboxTarget { data: &[u8], ) -> TargetResult<(), Self> { log::debug!("Write addr: {:X} len: {:X}", gva, data.len()); + let v = Vec::from(data); - unimplemented!() + match self.send_command(DebugMsg::WriteAddr(gva, v))? { + DebugResponse::WriteAddr => Ok(()), + msg => { + log::error!("Unexpected message received: {:?}", msg); + Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) + } + } } fn read_registers( @@ -191,6 +215,22 @@ impl SingleThreadBase for HyperlightSandboxTarget { Some(self) } } +impl SectionOffsets for HyperlightSandboxTarget { + fn get_section_offsets(&mut self) -> Result::Usize>, Self::Error> { + log::debug!("Get section offsets"); + + match self.send_command(DebugMsg::GetCodeSectionOffset)? { + DebugResponse::GetCodeSectionOffset(text) => Ok(Offsets::Segments { + text_seg: text, + data_seg: None, + }), + msg => { + log::error!("Unexpected message received: {:?}", msg); + Err(GdbTargetError::UnexpectedMessage) + } + } + } +} impl Breakpoints for HyperlightSandboxTarget { fn support_hw_breakpoint(&mut self) -> Option> { diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 4a6bd455..7f6cd201 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -66,6 +66,8 @@ pub(crate) fn is_hypervisor_present() -> bool { #[cfg(gdb)] mod debug { use std::sync::{Arc, Mutex}; + + use hyperlight_common::mem::PAGE_SIZE; use kvm_bindings::{ kvm_guest_debug, kvm_regs, KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_SINGLESTEP, KVM_GUESTDBG_USE_HW_BP, KVM_GUESTDBG_USE_SW_BP, @@ -75,6 +77,7 @@ mod debug { use super::KVMDriver; use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs}; use crate::hypervisor::handlers::DbgMemAccessHandlerCaller; + use crate::mem::layout::SandboxMemoryLayout; use crate::{new_error, Result}; /// KVM Debug struct @@ -212,6 +215,70 @@ mod debug { } } + pub fn read_addrs( + &mut self, + mut gva: u64, + mut data: &mut [u8], + dbg_mem_access_fn: Arc>, + ) -> Result<()> { + let data_len = data.len(); + log::debug!("Read addr: {:X} len: {:X}", gva, data_len); + + while !data.is_empty() { + let gpa = self.translate_gva(gva)?; + + let read_len = std::cmp::min( + data.len(), + (PAGE_SIZE - (gpa & (PAGE_SIZE - 1))).try_into().unwrap(), + ); + let offset = gpa as usize - SandboxMemoryLayout::BASE_ADDRESS; + + dbg_mem_access_fn + .clone() + .try_lock() + .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? + .read(offset, &mut data[..read_len])?; + + data = &mut data[read_len..]; + gva += read_len as u64; + } + + log::debug!("data before after {:?}", data); + + Ok(()) + } + + pub fn write_addrs( + &mut self, + mut gva: u64, + mut data: &[u8], + dbg_mem_access_fn: Arc>, + ) -> Result<()> { + let data_len = data.len(); + log::debug!("Write addr: {:X} len: {:X}", gva, data_len); + + while !data.is_empty() { + let gpa = self.translate_gva(gva)?; + + let write_len = std::cmp::min( + data.len(), + (PAGE_SIZE - (gpa & (PAGE_SIZE - 1))).try_into().unwrap(), + ); + let offset = gpa as usize - SandboxMemoryLayout::BASE_ADDRESS; + + dbg_mem_access_fn + .clone() + .try_lock() + .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? + .write(offset, data)?; + + data = &data[write_len..]; + gva += write_len as u64; + } + + Ok(()) + } + pub fn read_regs(&self, regs: &mut X86_64Regs) -> Result<()> { log::debug!("Read registers"); let vcpu_regs = self @@ -342,7 +409,7 @@ mod debug { pub fn process_dbg_request( &mut self, req: DebugMsg, - _dbg_mem_access_fn: Arc>, + dbg_mem_access_fn: Arc>, ) -> Result { match req { DebugMsg::AddHwBreakpoint(addr) => { @@ -360,6 +427,22 @@ mod debug { Ok(DebugResponse::DisableDebug) } + DebugMsg::GetCodeSectionOffset => { + let offset = dbg_mem_access_fn + .clone() + .try_lock() + .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? + .get_code_offset()?; + + Ok(DebugResponse::GetCodeSectionOffset(offset as u64)) + } + DebugMsg::ReadAddr(addr, len) => { + let mut data = vec![0u8; len]; + + self.read_addrs(addr, &mut data, dbg_mem_access_fn.clone())?; + + Ok(DebugResponse::ReadAddr(data)) + } DebugMsg::ReadRegisters => { let mut regs = X86_64Regs::default(); self.read_regs(&mut regs).expect("Read Regs error"); @@ -375,6 +458,11 @@ mod debug { self.set_single_step(true)?; Ok(DebugResponse::Step) } + DebugMsg::WriteAddr(addr, data) => { + self.write_addrs(addr, &data, dbg_mem_access_fn.clone())?; + + Ok(DebugResponse::WriteAddr) + } DebugMsg::WriteRegisters(regs) => { self.write_regs(®s).expect("Write Regs error"); Ok(DebugResponse::WriteRegisters) From b175f416536e87fbd48383e0f4e20f2156f34f75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 22:37:45 +0200 Subject: [PATCH 14/20] handlers fixup 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/handlers.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/handlers.rs b/src/hyperlight_host/src/hypervisor/handlers.rs index 3ee74f0e..cc6c7e91 100644 --- a/src/hyperlight_host/src/hypervisor/handlers.rs +++ b/src/hyperlight_host/src/hypervisor/handlers.rs @@ -121,7 +121,7 @@ pub trait DbgMemAccessHandlerCaller: Send { /// are passed as parameters to functions /// /// Note: This needs to be wrapped in a Mutex to be able to grab a mutable -/// reference to the underlying data +/// reference to the underlying data pub type DbgMemAccessHandlerWrapper = Arc>; pub(crate) type DbgReadMemAccessHandlerFunction = @@ -130,12 +130,12 @@ pub(crate) type DbgWriteMemAccessHandlerFunction = Box Result<()> + Send>; pub(crate) type DbgGetCodeAddrHandlerFunction = Box Result + Send>; -/// A `DbgMemAccessHandler` implementation using +/// A `DbgMemAccessHandler` implementation using /// (`DbgReadMemAccessHandlerFunction`, `DbgWriteMemAccessHandlerFunction `, `DbgGetCodeAddrHandlerFunction `). /// /// Note: This handler must live for as long as its Sandbox or for /// static in the case of its C API usage. -pub(crate) struct DbgMemAccessHandler ( +pub(crate) struct DbgMemAccessHandler( Arc>, Arc>, Arc>, From a4bb6dda22946de71c3dc699a744b087a912f168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 22:39:38 +0200 Subject: [PATCH 15/20] add sw breakpoint support 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 | 6 +- .../src/hypervisor/gdb/x86_64_target.rs | 45 +++++++++- src/hyperlight_host/src/hypervisor/kvm.rs | 90 +++++++++++++++++++ 3 files changed, 139 insertions(+), 2 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 57d0e345..82d6b45c 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -84,27 +84,31 @@ pub enum VcpuStopReason { /// Enumerates the possible actions that a debugger can ask from a Hypervisor pub enum DebugMsg { AddHwBreakpoint(u64), + AddSwBreakpoint(u64), Continue, DisableDebug, GetCodeSectionOffset, ReadAddr(u64, usize), ReadRegisters, RemoveHwBreakpoint(u64), + RemoveSwBreakpoint(u64), Step, WriteAddr(u64, Vec), WriteRegisters(X86_64Regs), } #[derive(Debug)] -/// Enumerates the possible responses that a hypervisor can provide to a debugger +/// Enumerates the possible responses that a hypervisor can provide to a debugger pub enum DebugResponse { AddHwBreakpoint(bool), + AddSwBreakpoint(bool), Continue, DisableDebug, GetCodeSectionOffset(u64), ReadAddr(Vec), ReadRegisters(X86_64Regs), RemoveHwBreakpoint(bool), + RemoveSwBreakpoint(bool), Step, VcpuStopped(VcpuStopReason), WriteAddr, diff --git a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs index 6f24e4c2..ddcc6901 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs @@ -7,7 +7,7 @@ use gdbstub::target::ext::base::singlethread::{ }; use gdbstub::target::ext::base::BaseOps; use gdbstub::target::ext::breakpoints::{ - Breakpoints, BreakpointsOps, HwBreakpoint, HwBreakpointOps, + Breakpoints, BreakpointsOps, HwBreakpoint, HwBreakpointOps, SwBreakpoint, SwBreakpointOps, }; use gdbstub::target::ext::section_offsets::{Offsets, SectionOffsets}; use gdbstub::target::{Target, TargetError, TargetResult}; @@ -83,6 +83,11 @@ impl Target for HyperlightSandboxTarget { type Arch = GdbTargetArch; type Error = GdbTargetError; + #[inline(always)] + fn guard_rail_implicit_sw_breakpoints(&self) -> bool { + true + } + fn support_breakpoints(&mut self) -> Option> { Some(self) } @@ -215,6 +220,7 @@ impl SingleThreadBase for HyperlightSandboxTarget { Some(self) } } + impl SectionOffsets for HyperlightSandboxTarget { fn get_section_offsets(&mut self) -> Result::Usize>, Self::Error> { log::debug!("Get section offsets"); @@ -236,6 +242,9 @@ impl Breakpoints for HyperlightSandboxTarget { fn support_hw_breakpoint(&mut self) -> Option> { Some(self) } + fn support_sw_breakpoint(&mut self) -> Option> { + Some(self) + } } impl HwBreakpoint for HyperlightSandboxTarget { @@ -272,6 +281,40 @@ impl HwBreakpoint for HyperlightSandboxTarget { } } +impl SwBreakpoint for HyperlightSandboxTarget { + fn add_sw_breakpoint( + &mut self, + addr: ::Usize, + _kind: ::BreakpointKind, + ) -> TargetResult { + log::debug!("Add sw breakpoint at address {:X}", addr); + + match self.send_command(DebugMsg::AddSwBreakpoint(addr))? { + DebugResponse::AddSwBreakpoint(rsp) => Ok(rsp), + msg => { + log::error!("Unexpected message received: {:?}", msg); + Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) + } + } + } + + fn remove_sw_breakpoint( + &mut self, + addr: ::Usize, + _kind: ::BreakpointKind, + ) -> TargetResult { + log::debug!("Remove sw breakpoint at address {:X}", addr); + + match self.send_command(DebugMsg::RemoveSwBreakpoint(addr))? { + DebugResponse::RemoveSwBreakpoint(rsp) => Ok(rsp), + msg => { + log::error!("Unexpected message received: {:?}", msg); + Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) + } + } + } +} + impl SingleThreadResume for HyperlightSandboxTarget { fn resume(&mut self, _signal: Option) -> Result<(), Self::Error> { log::debug!("Resume"); diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 7f6cd201..4ba503fd 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -65,6 +65,7 @@ pub(crate) fn is_hypervisor_present() -> bool { #[cfg(gdb)] mod debug { + use std::collections::HashMap; use std::sync::{Arc, Mutex}; use hyperlight_common::mem::PAGE_SIZE; @@ -80,6 +81,13 @@ mod debug { use crate::mem::layout::SandboxMemoryLayout; use crate::{new_error, Result}; + /// Software Breakpoint size in memory + pub const SW_BP_SIZE: usize = 1; + /// Software Breakpoinnt opcode + const SW_BP_OP: u8 = 0xCC; + /// Software Breakpoint written to memory + pub const SW_BP: [u8; SW_BP_SIZE] = [SW_BP_OP]; + /// KVM Debug struct /// This struct is used to abstract the internal details of the kvm /// guest debugging settings @@ -90,6 +98,8 @@ mod debug { /// Array of addresses for HW breakpoints hw_breakpoints: Vec, + /// Saves the bytes modified to enable sw breakpoints + sw_breakpoints: HashMap, /// Sent to KVM for enabling guest debug pub dbg_cfg: kvm_guest_debug, @@ -107,6 +117,7 @@ mod debug { Self { single_step: false, hw_breakpoints: vec![], + sw_breakpoints: HashMap::new(), dbg_cfg: dbg, } } @@ -380,6 +391,69 @@ mod debug { } } + pub fn add_sw_breakpoint( + &mut self, + addr: u64, + dbg_mem_access_fn: Arc>, + ) -> Result { + let addr = { + let debug = self + .debug + .as_ref() + .ok_or_else(|| new_error!("Debug is not enabled"))?; + let addr = self.translate_gva(addr)?; + if debug.sw_breakpoints.contains_key(&addr) { + return Ok(true); + } + + addr + }; + + let mut save_data = [0; SW_BP_SIZE]; + self.read_addrs(addr, &mut save_data[..], dbg_mem_access_fn.clone())?; + self.write_addrs(addr, &SW_BP, dbg_mem_access_fn.clone())?; + + { + let debug = self + .debug + .as_mut() + .ok_or_else(|| new_error!("Debug is not enabled"))?; + debug.sw_breakpoints.insert(addr, save_data); + } + + Ok(true) + } + + pub fn remove_sw_breakpoint( + &mut self, + addr: u64, + dbg_mem_access_fn: Arc>, + ) -> Result { + let (ret, data) = { + let addr = self.translate_gva(addr)?; + let debug = self + .debug + .as_mut() + .ok_or_else(|| new_error!("Debug is not enabled"))?; + + if debug.sw_breakpoints.contains_key(&addr) { + let save_data = debug + .sw_breakpoints + .remove(&addr) + .expect("Expected the hashmap to contain the address"); + + (true, Some(save_data)) + } else { + (false, None) + } + }; + + if ret { + self.write_addrs(addr, &data.unwrap(), dbg_mem_access_fn.clone())?; + } + + Ok(ret) + } /// Get the reason the vCPU has stopped pub fn get_stop_reason(&self) -> Result { @@ -394,6 +468,9 @@ mod debug { let ip = self.get_instruction_pointer()?; let gpa = self.translate_gva(ip)?; + if debug.sw_breakpoints.contains_key(&gpa) { + return Ok(VcpuStopReason::SwBp); + } if debug.hw_breakpoints.contains(&gpa) { return Ok(VcpuStopReason::HwBp); @@ -418,6 +495,13 @@ mod debug { .expect("Add hw breakpoint error"); Ok(DebugResponse::AddHwBreakpoint(res)) } + DebugMsg::AddSwBreakpoint(addr) => { + let res = self + .add_sw_breakpoint(addr, dbg_mem_access_fn.clone()) + .expect("Add sw breakpoint error"); + + Ok(DebugResponse::AddSwBreakpoint(res)) + } DebugMsg::Continue => { self.set_single_step(false)?; Ok(DebugResponse::Continue) @@ -454,6 +538,12 @@ mod debug { .expect("Remove hw breakpoint error"); Ok(DebugResponse::RemoveHwBreakpoint(res)) } + DebugMsg::RemoveSwBreakpoint(addr) => { + let res = self + .remove_sw_breakpoint(addr, dbg_mem_access_fn.clone()) + .expect("Remove sw breakpoint error"); + Ok(DebugResponse::RemoveSwBreakpoint(res)) + } DebugMsg::Step => { self.set_single_step(true)?; Ok(DebugResponse::Step) From 5476cddd51390343120555ee5454c47b2527497b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 22:42:57 +0200 Subject: [PATCH 16/20] add ci checks for the gdb feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .github/workflows/dep_rust.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/dep_rust.yml b/.github/workflows/dep_rust.yml index b725210d..1810d462 100644 --- a/.github/workflows/dep_rust.yml +++ b/.github/workflows/dep_rust.yml @@ -87,6 +87,7 @@ jobs: # make sure certain cargo features compile cargo check -p hyperlight-host --features crashdump cargo check -p hyperlight-host --features print_debug + cargo check -p hyperlight-host --features gdb # without any driver (shouldn't compile) just test-rust-feature-compilation-fail ${{ matrix.config }} From d0db26e11d20454105d42d47be0bed6e06baac6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 22:50:41 +0200 Subject: [PATCH 17/20] add documentation about the gdb debugging feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- docs/README.md | 1 + docs/how-to-debug-a-hyperlight-guest.md | 41 +++++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 docs/how-to-debug-a-hyperlight-guest.md diff --git a/docs/README.md b/docs/README.md index 1b1572dd..fff51c66 100644 --- a/docs/README.md +++ b/docs/README.md @@ -34,6 +34,7 @@ This project is composed internally of several internal components, depicted in * [Security guidance for developers](./security-guidance-for-developers.md) * [Paging Development Notes](./paging-development-notes.md) +* [How to debug a Hyperlight guest](./how-to-debug-a-hyperlight-guest.md) * [How to use Flatbuffers in Hyperlight](./how-to-use-flatbuffers.md) * [How to make a Hyperlight release](./how-to-make-releases.md) * [Getting Hyperlight Metrics, Logs, and Traces](./hyperlight-metrics-logs-and-traces.md) diff --git a/docs/how-to-debug-a-hyperlight-guest.md b/docs/how-to-debug-a-hyperlight-guest.md new file mode 100644 index 00000000..19d5914e --- /dev/null +++ b/docs/how-to-debug-a-hyperlight-guest.md @@ -0,0 +1,41 @@ +# How to debug a Hyperlight guest + +Hyperlight supports gdb debugging of a guest running inside a Hyperlight sandbox. +When Hyperlight is compiled with the `gdb` feature enabled, a Hyperlight sandbox can be configured +to start listening for a gdb connection. + +## Example +The snipped of a rust host application below configures the Hyperlight Sandbox to +listen on port `9050` for a gdb client to connect. + +```rust + let mut cfg = SandboxConfiguration::default(); + cfg.set_guest_debug_port(9050); + + // Create an uninitialized sandbox with a guest binary + let mut uninitialized_sandbox = UninitializedSandbox::new( + hyperlight_host::GuestBinary::FilePath( + hyperlight_testing::simple_guest_as_string().unwrap(), + ), + Some(cfg), // configuration + None, // default run options + None, // default host print function + )?; +``` + +The execution of the guest will wait for gdb to attach. + +One can use a simple gdb config to provide the symbols and desired configuration: + +For the above snippet, the below contents of the `.gdbinit` file can be used to +provide configuration to gdb startup. +```gdb +file path/to/symbols.elf +target remote :9050 +set disassembly-flavor intel +set disassemble-next-line on +enable pretty-printer +layout src +``` + +One can find more information about the `.gdbinit` file at [gdbinit(5)](https://www.man7.org/linux/man-pages/man5/gdbinit.5.html). From af0a52a5a6aa73c359f58b966303ff5ce1a4e364 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Tue, 4 Feb 2025 23:10:51 +0200 Subject: [PATCH 18/20] fix issues related to clippy, typos and feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - improve documentation - improve gdb errors by adding a specific error variant for when the rflags conversion from u64 to u32 fails - fix clippy issue on windows Signed-off-by: Doru Blânzeanu --- docs/how-to-debug-a-hyperlight-guest.md | 82 +++++++++++++++++-- src/hyperlight_host/Cargo.toml | 6 +- .../src/hypervisor/gdb/event_loop.rs | 16 ++++ src/hyperlight_host/src/hypervisor/gdb/mod.rs | 26 +++++- .../src/hypervisor/gdb/x86_64_target.rs | 18 +++- .../src/hypervisor/hyperv_windows.rs | 1 + src/hyperlight_host/src/hypervisor/kvm.rs | 2 +- 7 files changed, 136 insertions(+), 15 deletions(-) diff --git a/docs/how-to-debug-a-hyperlight-guest.md b/docs/how-to-debug-a-hyperlight-guest.md index 19d5914e..f77f9ffe 100644 --- a/docs/how-to-debug-a-hyperlight-guest.md +++ b/docs/how-to-debug-a-hyperlight-guest.md @@ -1,11 +1,54 @@ -# How to debug a Hyperlight guest +# How to debug a Hyperlight guest using gdb Hyperlight supports gdb debugging of a guest running inside a Hyperlight sandbox. When Hyperlight is compiled with the `gdb` feature enabled, a Hyperlight sandbox can be configured to start listening for a gdb connection. +## Supported features + +The Hyperlight `gdb` feature enables: + +1. KVM guest debugging: + - an entry point breakpoint is automatically set for the guest to stop + - add and remove HW breakpoints (maximum 4 set breakpoints at a time) + - add and remove SW breakpoints + - read and write registers + - read and write addresses + - step/continue + - get code offset from target + +## Expected behavior + +Below is a list describing some cases of expected behavior from a gdb debug +session of a guest binary running inside a Hyperlight sandbox. + +- when the `gdb` feature is enabled and a SandboxConfiguration is provided a + debug port, the created sandbox will wait for a gdb client to connect on the + configured port +- when the gdb client attaches, the guest vCPU is expected to be stopped at the + entrypoint +- if a gdb client disconnects unexpectedly, the debug session will be closed and + the guest will continue executing disregarding any prior breakpoints + +## How it works + +The gdb feature is designed to work like a Request - Response protocol between +a thread that accepts commands from a gdb cliend and the hypervisor handler over +a communication channel. + +All the functionality is implemented on the hypervisor side so it has access to +the shared memory and the vCPU. + +The gdb thread uses the `gdbstub` crate to handle the communication with the gdb client. +When the gdb client requests one of the supported features mentioned above, a request +is sent over the communication channel to the hypervisor handler for the sandbox +to resolve. + ## Example -The snipped of a rust host application below configures the Hyperlight Sandbox to + +### Sandbox configuration + +The snippet of a rust host application below configures the Hyperlight Sandbox to listen on port `9050` for a gdb client to connect. ```rust @@ -25,17 +68,44 @@ listen on port `9050` for a gdb client to connect. The execution of the guest will wait for gdb to attach. -One can use a simple gdb config to provide the symbols and desired configuration: +### Gdb configuration + +One can use a simple gdb config to provide the symbols and desired configuration. + +The below contents of the `.gdbinit` file can be used to provide a basic configuration +to gdb startup. -For the above snippet, the below contents of the `.gdbinit` file can be used to -provide configuration to gdb startup. ```gdb +# Path to symbols file path/to/symbols.elf +# The port on which Hyperlight listens for a connection target remote :9050 set disassembly-flavor intel set disassemble-next-line on enable pretty-printer layout src ``` - One can find more information about the `.gdbinit` file at [gdbinit(5)](https://www.man7.org/linux/man-pages/man5/gdbinit.5.html). + +### End to end example + +Using the [Sandbox configuration](#sandbox-configuration) above to configure the [hello-world](https://github.com/hyperlight-dev/hyperlight/blob/main/src/hyperlight_host/examples/hello-world/main.rs) example +in Hyperlight one can run the below commands to debug the guest binary: + +```bash +# Terminal 1 +$ cargo run --example hello-world --features gdb +``` + +```bash +# Terminal 2 +$ cat .gdbinit +file file src/tests/rust_guests/bin/debug/simpleguest +target remote :9050 +set disassembly-flavor intel +set disassemble-next-line on +enable pretty-printer +layout src + +$ gdb +``` diff --git a/src/hyperlight_host/Cargo.toml b/src/hyperlight_host/Cargo.toml index 8a3c6c19..73e1915c 100644 --- a/src/hyperlight_host/Cargo.toml +++ b/src/hyperlight_host/Cargo.toml @@ -71,8 +71,8 @@ sha256 = "1.4.0" windows-version = "0.1" [target.'cfg(unix)'.dependencies] -gdbstub = "0.7.3" -gdbstub_arch = "0.3.1" +gdbstub = { version = "0.7.3", optional = true } +gdbstub_arch = { version = "0.3.1", optional = true } seccompiler = { version = "0.4.0", optional = true } kvm-bindings = { version = "0.11", features = ["fam-wrappers"], optional = true } kvm-ioctls = { version = "0.20", optional = true } @@ -131,7 +131,7 @@ mshv2 = ["dep:mshv-bindings2", "dep:mshv-ioctls2"] mshv3 = ["dep:mshv-bindings3", "dep:mshv-ioctls3"] inprocess = [] # This enables compilation of gdb stub for easy debug in the guest -gdb = [] +gdb = ["dep:gdbstub", "dep:gdbstub_arch"] [[bench]] name = "benchmarks" diff --git a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs index 1a66b589..fa1f0e69 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs @@ -1,3 +1,19 @@ +/* +Copyright 2024 The Hyperlight Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + use gdbstub::common::Signal; use gdbstub::conn::ConnectionExt; use gdbstub::stub::run_blocking::{self, WaitForStopReasonError}; diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 82d6b45c..48153e48 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -1,3 +1,19 @@ +/* +Copyright 2024 The Hyperlight Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + mod event_loop; pub mod x86_64_target; @@ -23,6 +39,8 @@ pub enum GdbTargetError { CannotReceiveMsg, #[error("Error encountered when sending message")] CannotSendMsg, + #[error("Out of range conversion: {0}")] + OutOfRangeConversion(String), #[error("Encountered an unexpected message over communication channel")] UnexpectedMessage, #[error("Unexpected error encountered")] @@ -48,8 +66,8 @@ impl From for TargetError { } } -#[derive(Debug, Default)] /// Struct that contains the x86_64 core registers +#[derive(Debug, Default)] pub struct X86_64Regs { pub rax: u64, pub rbx: u64, @@ -71,8 +89,8 @@ pub struct X86_64Regs { pub rflags: u64, } -#[derive(Debug)] /// Defines the possible reasons for which a vCPU ce be stopped when debugging +#[derive(Debug)] pub enum VcpuStopReason { DoneStep, HwBp, @@ -80,8 +98,8 @@ pub enum VcpuStopReason { Unknown, } -#[derive(Debug)] /// Enumerates the possible actions that a debugger can ask from a Hypervisor +#[derive(Debug)] pub enum DebugMsg { AddHwBreakpoint(u64), AddSwBreakpoint(u64), @@ -97,8 +115,8 @@ pub enum DebugMsg { WriteRegisters(X86_64Regs), } -#[derive(Debug)] /// Enumerates the possible responses that a hypervisor can provide to a debugger +#[derive(Debug)] pub enum DebugResponse { AddHwBreakpoint(bool), AddSwBreakpoint(bool), diff --git a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs index ddcc6901..c8a75865 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs @@ -1,3 +1,19 @@ +/* +Copyright 2024 The Hyperlight Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + use crossbeam_channel::TryRecvError; use gdbstub::arch::Arch; use gdbstub::common::Signal; @@ -168,7 +184,7 @@ impl SingleThreadBase for HyperlightSandboxTarget { regs.regs[15] = read_regs.r15; regs.rip = read_regs.rip; regs.eflags = u32::try_from(read_regs.rflags) - .expect("Couldn't convert rflags from u64 to u32"); + .map_err(|e| GdbTargetError::OutOfRangeConversion(e.to_string()))?; Ok(()) } diff --git a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs index 947fe089..acde5e8f 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs @@ -374,6 +374,7 @@ impl Hypervisor for HypervWindowsDriver { hv_handler, outb_hdl, mem_access_hdl, + #[cfg(gdb)] dbg_mem_access_hdl, )?; diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 4ba503fd..06fff815 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -925,7 +925,7 @@ impl Hypervisor for KVMDriver { ); self.send_dbg_msg(response) - .map_err(|e| new_error!("Couldn't send reponse to gdb: {:?}", e))?; + .map_err(|e| new_error!("Couldn't send response to gdb: {:?}", e))?; if cont { break; From 6cc618496bcc52e32249da1ccc8632b3ccd99b54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 5 Feb 2025 14:06:42 +0200 Subject: [PATCH 19/20] remove gdb feature guard for sandbox config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - the user experience is not great as the fields and methods related to gdb port are not accessible unless the gdb feature is turned on. - if one runs the build command in the cli and provides the --features gdb the IDE will not know that the feature is enabled and will show errors as it doesn't know about the fields/methods Signed-off-by: Doru Blânzeanu --- src/hyperlight_host/src/sandbox/config.rs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/hyperlight_host/src/sandbox/config.rs b/src/hyperlight_host/src/sandbox/config.rs index 0dbf1e04..2e407e61 100644 --- a/src/hyperlight_host/src/sandbox/config.rs +++ b/src/hyperlight_host/src/sandbox/config.rs @@ -25,7 +25,6 @@ use crate::mem::exe::ExeInfo; #[derive(Copy, Clone, Debug, Eq, PartialEq)] #[repr(C)] pub struct SandboxConfiguration { - #[cfg(gdb)] /// Guest gdb debug port guest_debug_port: Option, /// The maximum size of the guest error buffer. @@ -139,9 +138,8 @@ impl SandboxConfiguration { pub const MIN_KERNEL_STACK_SIZE: usize = 0x1000; /// The default value for kernel stack size pub const DEFAULT_KERNEL_STACK_SIZE: usize = Self::MIN_KERNEL_STACK_SIZE; - #[cfg(gdb)] /// The minimum value for debug port - pub const MIN_GUEST_DEBUG_PORT: u16 = 9000; + pub const MIN_GUEST_DEBUG_PORT: u16 = 1024; #[allow(clippy::too_many_arguments)] /// Create a new configuration for a sandbox with the given sizes. @@ -159,7 +157,7 @@ impl SandboxConfiguration { max_initialization_time: Option, max_wait_for_cancellation: Option, guest_panic_context_buffer_size: usize, - #[cfg(gdb)] guest_debug_port: Option, + guest_debug_port: Option, ) -> Self { Self { input_data_size: max(input_data_size, Self::MIN_INPUT_SIZE), @@ -227,7 +225,6 @@ impl SandboxConfiguration { guest_panic_context_buffer_size, Self::MIN_GUEST_PANIC_CONTEXT_BUFFER_SIZE, ), - #[cfg(gdb)] guest_debug_port, } } @@ -355,7 +352,6 @@ impl SandboxConfiguration { ); } - #[cfg(gdb)] #[instrument(skip_all, parent = Span::current(), level= "Trace")] /// Sets the configuration for the guest debug pub fn set_guest_debug_port(&mut self, port: u16) { @@ -406,7 +402,6 @@ impl SandboxConfiguration { self.max_initialization_time } - #[cfg(gdb)] #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn get_guest_debug_port(&self) -> Option { self.guest_debug_port @@ -460,7 +455,6 @@ impl Default for SandboxConfiguration { None, None, Self::DEFAULT_GUEST_PANIC_CONTEXT_BUFFER_SIZE, - #[cfg(gdb)] None, ) } @@ -504,7 +498,6 @@ mod tests { MAX_WAIT_FOR_CANCELLATION_OVERRIDE as u64, )), GUEST_PANIC_CONTEXT_BUFFER_SIZE_OVERRIDE, - #[cfg(gdb)] None, ); let exe_infos = vec![ @@ -569,7 +562,6 @@ mod tests { SandboxConfiguration::MIN_MAX_WAIT_FOR_CANCELLATION as u64 - 1, )), SandboxConfiguration::MIN_GUEST_PANIC_CONTEXT_BUFFER_SIZE - 1, - #[cfg(gdb)] None, ); assert_eq!(SandboxConfiguration::MIN_INPUT_SIZE, cfg.input_data_size); @@ -741,7 +733,6 @@ mod tests { } #[test] - #[cfg(gdb)] fn guest_debug_port(port in 9000..=u16::MAX) { let mut cfg = SandboxConfiguration::default(); cfg.set_guest_debug_port(port); From 98ac60ea26670268ca6878330c170999eec0fa5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 5 Feb 2025 17:50:15 +0200 Subject: [PATCH 20/20] add a way to signal a non fatal error on the hypervisor side MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - when using visual studio code, sometimes it requests to read from unexpected addresses which can cause an error on the hypervisor side. This fix signals this to the gdb thread which marks it as a non-fatal error Signed-off-by: Doru Blânzeanu --- src/hyperlight_host/src/error.rs | 5 +++ src/hyperlight_host/src/hypervisor/gdb/mod.rs | 1 + .../src/hypervisor/gdb/x86_64_target.rs | 45 ++++++++++++++++++- src/hyperlight_host/src/hypervisor/kvm.rs | 31 +++++++------ 4 files changed, 68 insertions(+), 14 deletions(-) diff --git a/src/hyperlight_host/src/error.rs b/src/hyperlight_host/src/error.rs index e846ee12..76fdb8d0 100644 --- a/src/hyperlight_host/src/error.rs +++ b/src/hyperlight_host/src/error.rs @@ -275,6 +275,11 @@ pub enum HyperlightError { #[error("SystemTimeError {0:?}")] SystemTimeError(#[from] SystemTimeError), + /// Error occurred when translating guest address + #[error("An error occurred when translating guest address: {0:?}")] + #[cfg(gdb)] + TranslateGuestAddress(u64), + /// Error occurred converting a slice to an array #[error("TryFromSliceError {0:?}")] TryFromSliceError(#[from] TryFromSliceError), diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 48153e48..88f68edb 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -122,6 +122,7 @@ pub enum DebugResponse { AddSwBreakpoint(bool), Continue, DisableDebug, + ErrorOccurred, GetCodeSectionOffset(u64), ReadAddr(Vec), ReadRegisters(X86_64Regs), diff --git a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs index c8a75865..8599441b 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs @@ -87,6 +87,10 @@ impl HyperlightSandboxTarget { match self.send_command(DebugMsg::DisableDebug)? { DebugResponse::DisableDebug => Ok(()), + DebugResponse::ErrorOccurred => { + log::error!("Error occurred"); + Err(GdbTargetError::UnexpectedError) + } msg => { log::error!("Unexpected message received: {:?}", msg); Err(GdbTargetError::UnexpectedMessage) @@ -134,6 +138,10 @@ impl SingleThreadBase for HyperlightSandboxTarget { Ok(v.len()) } + DebugResponse::ErrorOccurred => { + log::error!("Error occurred"); + Err(TargetError::NonFatal) + } msg => { log::error!("Unexpected message received: {:?}", msg); Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) @@ -151,6 +159,10 @@ impl SingleThreadBase for HyperlightSandboxTarget { match self.send_command(DebugMsg::WriteAddr(gva, v))? { DebugResponse::WriteAddr => Ok(()), + DebugResponse::ErrorOccurred => { + log::error!("Error occurred"); + Err(TargetError::NonFatal) + } msg => { log::error!("Unexpected message received: {:?}", msg); Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) @@ -188,6 +200,10 @@ impl SingleThreadBase for HyperlightSandboxTarget { Ok(()) } + DebugResponse::ErrorOccurred => { + log::error!("Error occurred"); + Err(TargetError::NonFatal) + } msg => { log::error!("Unexpected message received: {:?}", msg); @@ -225,6 +241,10 @@ impl SingleThreadBase for HyperlightSandboxTarget { match self.send_command(DebugMsg::WriteRegisters(regs))? { DebugResponse::WriteRegisters => Ok(()), + DebugResponse::ErrorOccurred => { + log::error!("Error occurred"); + Err(TargetError::NonFatal) + } msg => { log::error!("Unexpected message received: {:?}", msg); Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) @@ -246,6 +266,10 @@ impl SectionOffsets for HyperlightSandboxTarget { text_seg: text, data_seg: None, }), + DebugResponse::ErrorOccurred => { + log::error!("Error occurred"); + Err(GdbTargetError::UnexpectedError) + } msg => { log::error!("Unexpected message received: {:?}", msg); Err(GdbTargetError::UnexpectedMessage) @@ -273,6 +297,10 @@ impl HwBreakpoint for HyperlightSandboxTarget { match self.send_command(DebugMsg::AddHwBreakpoint(addr))? { DebugResponse::AddHwBreakpoint(rsp) => Ok(rsp), + DebugResponse::ErrorOccurred => { + log::error!("Error occurred"); + Err(TargetError::NonFatal) + } msg => { log::error!("Unexpected message received: {:?}", msg); Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) @@ -289,6 +317,10 @@ impl HwBreakpoint for HyperlightSandboxTarget { match self.send_command(DebugMsg::RemoveHwBreakpoint(addr))? { DebugResponse::RemoveHwBreakpoint(rsp) => Ok(rsp), + DebugResponse::ErrorOccurred => { + log::error!("Error occurred"); + Err(TargetError::NonFatal) + } msg => { log::error!("Unexpected message received: {:?}", msg); Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) @@ -307,6 +339,10 @@ impl SwBreakpoint for HyperlightSandboxTarget { match self.send_command(DebugMsg::AddSwBreakpoint(addr))? { DebugResponse::AddSwBreakpoint(rsp) => Ok(rsp), + DebugResponse::ErrorOccurred => { + log::error!("Error occurred"); + Err(TargetError::NonFatal) + } msg => { log::error!("Unexpected message received: {:?}", msg); Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) @@ -323,6 +359,10 @@ impl SwBreakpoint for HyperlightSandboxTarget { match self.send_command(DebugMsg::RemoveSwBreakpoint(addr))? { DebugResponse::RemoveSwBreakpoint(rsp) => Ok(rsp), + DebugResponse::ErrorOccurred => { + log::error!("Error occurred"); + Err(TargetError::NonFatal) + } msg => { log::error!("Unexpected message received: {:?}", msg); Err(TargetError::Fatal(GdbTargetError::UnexpectedMessage)) @@ -348,9 +388,12 @@ impl SingleThreadSingleStep for HyperlightSandboxTarget { log::debug!("Step"); match self.send_command(DebugMsg::Step)? { DebugResponse::Step => Ok(()), + DebugResponse::ErrorOccurred => { + log::error!("Error occurred"); + Err(GdbTargetError::UnexpectedError) + } msg => { log::error!("Unexpected message received: {:?}", msg); - Err(GdbTargetError::UnexpectedMessage) } } diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 06fff815..0e54e69c 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -39,6 +39,8 @@ use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; use crate::mem::ptr::{GuestPtr, RawPtr}; #[cfg(gdb)] use crate::sandbox::uninitialized::DebugInfo; +#[cfg(gdb)] +use crate::HyperlightError; use crate::{log_then_return, new_error, Result}; /// Return `true` if the KVM API is available, version 12, and has UserMemory capability, or `false` otherwise @@ -79,7 +81,7 @@ mod debug { use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs}; use crate::hypervisor::handlers::DbgMemAccessHandlerCaller; use crate::mem::layout::SandboxMemoryLayout; - use crate::{new_error, Result}; + use crate::{new_error, HyperlightError, Result}; /// Software Breakpoint size in memory pub const SW_BP_SIZE: usize = 1; @@ -208,19 +210,13 @@ mod debug { /// Translates the guest address to physical address fn translate_gva(&self, gva: u64) -> Result { - let tr = self.vcpu_fd.translate_gva(gva).map_err(|e| { - new_error!( - "Could not translate guest virtual address {:X}: {:?}", - gva, - e - ) - })?; + let tr = self + .vcpu_fd + .translate_gva(gva) + .map_err(|_| HyperlightError::TranslateGuestAddress(gva))?; if tr.valid == 0 { - Err(new_error!( - "Could not translate guest virtual address {:X}", - gva - )) + Err(HyperlightError::TranslateGuestAddress(gva)) } else { Ok(tr.physical_address) } @@ -916,7 +912,16 @@ impl Hypervisor for KVMDriver { // Wait for a message from gdb let req = self.recv_dbg_msg()?; - let response = self.process_dbg_request(req, dbg_mem_access_fn.clone())?; + let result = self.process_dbg_request(req, dbg_mem_access_fn.clone()); + + let response = match result { + Ok(response) => response, + // Treat non fatal errors separately so the guest doesn't fail + Err(HyperlightError::TranslateGuestAddress(_)) => DebugResponse::ErrorOccurred, + Err(e) => { + return Err(e); + } + }; // If the command was either step or continue, we need to run the vcpu let cont = matches!(