From 08526a65b5a86794910a83d7b20277dade44c695 Mon Sep 17 00:00:00 2001 From: Sebastian Eydam Date: Fri, 19 Jun 2026 14:31:22 +0200 Subject: [PATCH] main: print seccomp syscall details When seccomp traps a SIGSYS, print the syscall number that caused it, the current thread id and thread name to make violations easier to debug. This change requires that all threads are allowed to execute the `gettid` and the `prctl` syscalls, thus the seccomp filters have also been adjusted. On-behalf-of: SAP sebastian.eydam@sap.com Signed-off-by: Sebastian Eydam --- Cargo.lock | 1 + Cargo.toml | 1 + cloud-hypervisor/Cargo.toml | 1 + cloud-hypervisor/src/main.rs | 52 +++++++++++++++++++++++++++--------- vmm/src/seccomp_filters.rs | 38 +++++++++++++++----------- 5 files changed, 66 insertions(+), 27 deletions(-) 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