diff --git a/Cargo.lock b/Cargo.lock index 39d259a37..8907f2126 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -485,6 +485,7 @@ dependencies = [ "seccompiler", "serde_json", "signal-hook", + "signal-hook-registry", "test_infra", "thiserror", "tracer", diff --git a/Cargo.toml b/Cargo.toml index 1a7c13860..8e18cbe3e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -103,6 +103,7 @@ rustls = { version = "0.23.40", default-features = false, features = [ ] } sha2 = "0.11.0" signal-hook = "0.4.4" +signal-hook-registry = "1.4.8" thiserror = "2.0.18" uuid = { version = "1.23.2" } wait-timeout = "0.2.1" diff --git a/cloud-hypervisor/Cargo.toml b/cloud-hypervisor/Cargo.toml index e0a94cb80..7c4cc7e0c 100644 --- a/cloud-hypervisor/Cargo.toml +++ b/cloud-hypervisor/Cargo.toml @@ -24,6 +24,7 @@ option_parser = { path = "../option_parser" } seccompiler = { workspace = true } serde_json = { workspace = true } signal-hook = { workspace = true } +signal-hook-registry = { workspace = true } thiserror = { workspace = true } tracer = { path = "../tracer" } vm-migration = { path = "../vm-migration" } diff --git a/cloud-hypervisor/src/main.rs b/cloud-hypervisor/src/main.rs index 17d10842e..9cb9fe9e4 100644 --- a/cloud-hypervisor/src/main.rs +++ b/cloud-hypervisor/src/main.rs @@ -13,7 +13,7 @@ use std::path::Path; #[cfg(feature = "guest_debug")] use std::path::PathBuf; use std::sync::mpsc::channel; -use std::{any, cmp, env, io, num, process}; +use std::{any, cmp, env, io, num, process, str, thread}; use clap::{Arg, ArgAction, ArgGroup, ArgMatches, Command}; use event_monitor::event; @@ -48,6 +48,41 @@ use vmm_sys_util::signal::block_signal; use crate::logger::Logger; +// Linux exposes seccomp's SIGSYS payload via the siginfo_t layout; this struct mirrors the +// fields we need so the handler can read the syscall and arch. +#[repr(C)] +struct SeccompSiginfo { + si_signo: libc::c_int, + si_errno: libc::c_int, + si_code: libc::c_int, + _pad0: libc::c_int, + si_call_addr: *mut libc::c_void, + si_syscall: libc::c_int, + si_arch: libc::c_uint, +} + +fn handle_sigsys(info: &libc::siginfo_t) { + // SAFETY: The handler only reads the provided siginfo pointer, writes a + // diagnostic message, and then delegates to the default SIGSYS handler. + unsafe { + let current_thread = thread::current(); + let thread_name = current_thread.name().unwrap_or(""); + let tid = libc::syscall(libc::SYS_gettid) as i64; + let info = &*(info as *const libc::siginfo_t as *const SeccompSiginfo); + eprintln!( + concat!( + "\n==== Possible seccomp violation ====\n", + "Syscall number: {} (arch: {:#x}, tid: {}, thread: {})\n", + "Try running with `strace -ff` to identify the cause and open an issue: ", + "https://github.com/cloud-hypervisor/cloud-hypervisor/issues/new", + ), + info.si_syscall, info.si_arch, tid, thread_name, + ); + + low_level::emulate_default_handler(SIGSYS).unwrap(); + } +} + #[cfg(feature = "dhat-heap")] #[global_allocator] static ALLOC: dhat::Alloc = dhat::Alloc; @@ -563,20 +598,13 @@ fn start_vmm( }; if seccomp_action == SeccompAction::Trap { - // SAFETY: We only using signal_hook for managing signals and only execute signal + // SAFETY: We only use signal_hook for managing signals and only execute signal // handler safe functions (writing to stderr) and manipulating signals. unsafe { - low_level::register(SIGSYS, || { - eprintln!( - "\n==== Possible seccomp violation ====\n\ - Try running with `strace -ff` to identify the cause and open an issue: \ - https://github.com/cloud-hypervisor/cloud-hypervisor/issues/new" - ); - low_level::emulate_default_handler(SIGSYS).unwrap(); - }) + signal_hook_registry::register_sigaction(SIGSYS, handle_sigsys) + .map_err(|e| error!("Error adding SIGSYS signal handler: {e}")) + .ok(); } - .map_err(|e| error!("Error adding SIGSYS signal handler: {e}")) - .ok(); } // SAFETY: Trivially safe. diff --git a/vmm/src/seccomp_filters.rs b/vmm/src/seccomp_filters.rs index 9df136dd6..eb76c7697 100644 --- a/vmm/src/seccomp_filters.rs +++ b/vmm/src/seccomp_filters.rs @@ -555,6 +555,12 @@ fn create_serial_manager_ioctl_seccomp_rule() -> Result, Backen Ok(or![and![Cond::new(1, ArgLen::Dword, Eq, FIONBIO as _)?]]) } +// Syscalls needed by all threads, because they are used in the seccomp signal +// handler. +fn common_thread_rules() -> Result)>, BackendError> { + Ok(vec![(libc::SYS_gettid, vec![])]) +} + fn create_signal_handler_ioctl_seccomp_rule() -> Result, BackendError> { Ok(or![ and![Cond::new(1, ArgLen::Dword, Eq, TCGETS as _)?], @@ -580,7 +586,6 @@ fn signal_handler_thread_rules() -> Result)>, Backend (libc::SYS_mmap, vec![]), (libc::SYS_munmap, vec![]), (libc::SYS_prctl, vec![]), - (libc::SYS_gettid, vec![]), (libc::SYS_recvfrom, vec![]), (libc::SYS_rt_sigprocmask, vec![]), (libc::SYS_rt_sigreturn, vec![]), @@ -1120,22 +1125,25 @@ fn get_seccomp_rules( thread_type: Thread, hypervisor_type: Option, ) -> Result)>, BackendError> { - match thread_type { - Thread::HttpApi => Ok(http_api_thread_rules()?), + let mut rules = common_thread_rules()?; + let specific_rules = match thread_type { + Thread::HttpApi => http_api_thread_rules()?, #[cfg(feature = "dbus_api")] - Thread::DBusApi => Ok(dbus_api_thread_rules()?), - Thread::EventMonitor => Ok(event_monitor_thread_rules()?), - Thread::SerialManager => Ok(serial_manager_thread_rules()?), - Thread::SignalHandler => Ok(signal_handler_thread_rules()?), - Thread::Vcpu => Ok(vcpu_thread_rules( + Thread::DBusApi => dbus_api_thread_rules()?, + Thread::EventMonitor => event_monitor_thread_rules()?, + Thread::SerialManager => serial_manager_thread_rules()?, + Thread::SignalHandler => signal_handler_thread_rules()?, + Thread::Vcpu => vcpu_thread_rules( hypervisor_type.expect("hypervisor_type is required for Vcpu threads"), - )?), - Thread::Vmm => Ok(vmm_thread_rules( - hypervisor_type.expect("hypervisor_type is required for Vmm threads"), - )?), - Thread::PtyForeground => Ok(pty_foreground_thread_rules()?), - Thread::MigrateSendPostcopy => Ok(migrate_send_postcopy_thread_rules()?), - } + )?, + Thread::Vmm => { + vmm_thread_rules(hypervisor_type.expect("hypervisor_type is required for Vmm threads"))? + } + Thread::PtyForeground => pty_foreground_thread_rules()?, + Thread::MigrateSendPostcopy => migrate_send_postcopy_thread_rules()?, + }; + rules.extend(specific_rules); + Ok(rules) } /// Generate a BPF program based on the seccomp_action value