From b68eb04b8b67b19c1604e1cd75c6f011ce198361 Mon Sep 17 00:00:00 2001 From: Etienne Cordonnier Date: Sat, 19 Oct 2024 22:54:35 +0200 Subject: [PATCH 1/2] uucore: disable default signal-handlers added by Rust Fixes https://github.com/uutils/coreutils/issues/6759 Test procedure (verifies that a single SIGSEGV stops a program): ``` ecordonnier@lj8k2dq3:~/dev/coreutils$ ./target/release/coreutils sleep 100 & [1] 4175464 ecordonnier@lj8k2dq3:~/dev/coreutils$ kill -11 $(pidof coreutils) ecordonnier@lj8k2dq3:~/dev/coreutils$ [1]+ Segmentation fault (core dumped) ./target/release/coreutils sleep 100 ``` --- src/uucore/src/lib/lib.rs | 27 +++++++++++++++++++++++++++ src/uucore_procs/src/lib.rs | 6 +++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index a636fcdab2e..6142e688d7c 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -4,6 +4,8 @@ // file that was distributed with this source code. //! library ~ (core/bundler file) // #![deny(missing_docs)] //TODO: enable this +// +// spell-checker:ignore sigaction SIGBUS SIGSEGV // * feature-gated external crates (re-shared as public internal modules) #[cfg(feature = "libc")] @@ -100,6 +102,12 @@ pub use crate::features::fsxattr; //## core functions +#[cfg(unix)] +use nix::errno::Errno; +#[cfg(unix)] +use nix::sys::signal::{ + sigaction, SaFlags, SigAction, SigHandler::SigDfl, SigSet, Signal::SIGBUS, Signal::SIGSEGV, +}; use std::borrow::Cow; use std::ffi::OsStr; use std::ffi::OsString; @@ -112,6 +120,25 @@ use std::sync::atomic::Ordering; use once_cell::sync::Lazy; +/// Disables the custom signal handlers installed by Rust for stack-overflow handling. With those custom signal handlers processes ignore the first SIGBUS and SIGSEGV signal they receive. +/// See for details. +#[cfg(unix)] +pub fn disable_rust_signal_handlers() -> Result<(), Errno> { + unsafe { + sigaction( + SIGSEGV, + &SigAction::new(SigDfl, SaFlags::empty(), SigSet::all()), + ) + }?; + unsafe { + sigaction( + SIGBUS, + &SigAction::new(SigDfl, SaFlags::empty(), SigSet::all()), + ) + }?; + Ok(()) +} + /// Execute utility code for `util`. /// /// This macro expands to a main function that invokes the `uumain` function in `util` diff --git a/src/uucore_procs/src/lib.rs b/src/uucore_procs/src/lib.rs index 11d89c97eb4..6504171041d 100644 --- a/src/uucore_procs/src/lib.rs +++ b/src/uucore_procs/src/lib.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. // -// spell-checker:ignore backticks uuhelp +// spell-checker:ignore backticks uuhelp SIGSEGV //! A collection of procedural macros for uutils. #![deny(missing_docs)] @@ -25,6 +25,10 @@ pub fn main(_args: TokenStream, stream: TokenStream) -> TokenStream { let new = quote!( pub fn uumain(args: impl uucore::Args) -> i32 { #stream + + // disable rust signal handlers (otherwise processes don't dump core after e.g. one SIGSEGV) + #[cfg(unix)] + uucore::disable_rust_signal_handlers().expect("Disabling rust signal handlers failed"); let result = uumain(args); match result { Ok(()) => uucore::error::get_exit_code(), From 8ab99d4bfea71df3f66af1efa02e5370a9ac25ed Mon Sep 17 00:00:00 2001 From: Etienne Cordonnier Date: Mon, 21 Oct 2024 22:58:54 +0200 Subject: [PATCH 2/2] test(sleep): add test for signal handling --- tests/by-util/test_sleep.rs | 38 ++++++++++++++++++- tests/common/util.rs | 75 ++++++++++++++++++++++++++++++++++++- 2 files changed, 110 insertions(+), 3 deletions(-) diff --git a/tests/by-util/test_sleep.rs b/tests/by-util/test_sleep.rs index 374156e285c..2708b01c169 100644 --- a/tests/by-util/test_sleep.rs +++ b/tests/by-util/test_sleep.rs @@ -4,9 +4,11 @@ // file that was distributed with this source code. use rstest::rstest; -// spell-checker:ignore dont +// spell-checker:ignore dont SIGBUS SIGSEGV sigsegv sigbus use crate::common::util::TestScenario; +#[cfg(unix)] +use nix::sys::signal::Signal::{SIGBUS, SIGSEGV}; use std::time::{Duration, Instant}; #[test] @@ -135,6 +137,40 @@ fn test_sleep_wrong_time() { new_ucmd!().args(&["0.1s", "abc"]).fails(); } +#[test] +#[cfg(unix)] +fn test_sleep_stops_after_sigsegv() { + let mut child = new_ucmd!() + .arg("100") + .timeout(Duration::from_secs(10)) + .run_no_wait(); + + child + .delay(100) + .kill_with_custom_signal(SIGSEGV) + .make_assertion() + .with_current_output() + .signal_is(SIGSEGV as i32) // make sure it was us who terminated the process + .no_output(); +} + +#[test] +#[cfg(unix)] +fn test_sleep_stops_after_sigbus() { + let mut child = new_ucmd!() + .arg("100") + .timeout(Duration::from_secs(10)) + .run_no_wait(); + + child + .delay(100) + .kill_with_custom_signal(SIGBUS) + .make_assertion() + .with_current_output() + .signal_is(SIGBUS as i32) // make sure it was us who terminated the process + .no_output(); +} + #[test] fn test_sleep_when_single_input_exceeds_max_duration_then_no_error() { let mut child = new_ucmd!() diff --git a/tests/common/util.rs b/tests/common/util.rs index 2d1fd91d17a..87c937492f3 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. //spell-checker: ignore (linux) rlimit prlimit coreutil ggroups uchild uncaptured scmd SHLVL canonicalized openpty -//spell-checker: ignore (linux) winsize xpixel ypixel setrlimit FSIZE +//spell-checker: ignore (linux) winsize xpixel ypixel setrlimit FSIZE SIGBUS SIGSEGV sigbus #![allow(dead_code)] #![allow( @@ -17,6 +17,8 @@ use libc::mode_t; #[cfg(unix)] use nix::pty::OpenptyResult; +#[cfg(unix)] +use nix::sys; use pretty_assertions::assert_eq; #[cfg(unix)] use rlimit::setrlimit; @@ -2095,7 +2097,7 @@ impl UChild { self.delay(millis).make_assertion() } - /// Try to kill the child process and wait for it's termination. + /// Try to kill the child process and wait for its termination. /// /// This method blocks until the child process is killed, but returns an error if `self.timeout` /// or the default of 60s was reached. If no such error happened, the process resources are @@ -2155,6 +2157,75 @@ impl UChild { self } + /// Try to kill the child process and wait for its termination. + /// + /// This method blocks until the child process is killed, but returns an error if `self.timeout` + /// or the default of 60s was reached. If no such error happened, the process resources are + /// released, so there is usually no need to call `wait` or alike on unix systems although it's + /// still possible to do so. + /// + /// # Platform specific behavior + /// + /// On unix systems the child process resources will be released like a call to [`Child::wait`] + /// or alike would do. + /// + /// # Error + /// + /// If [`Child::kill`] returned an error or if the child process could not be terminated within + /// `self.timeout` or the default of 60s. + #[cfg(unix)] + pub fn try_kill_with_custom_signal( + &mut self, + signal_name: sys::signal::Signal, + ) -> io::Result<()> { + let start = Instant::now(); + sys::signal::kill( + nix::unistd::Pid::from_raw(self.raw.id().try_into().unwrap()), + signal_name, + ) + .unwrap(); + + let timeout = self.timeout.unwrap_or(Duration::from_secs(60)); + // As a side effect, we're cleaning up the killed child process with the implicit call to + // `Child::try_wait` in `self.is_alive`, which reaps the process id on unix systems. We + // always fail with error on timeout if `self.timeout` is set to zero. + while self.is_alive() || timeout == Duration::ZERO { + if start.elapsed() < timeout { + self.delay(10); + } else { + return Err(io::Error::new( + io::ErrorKind::Other, + format!("kill: Timeout of '{}s' reached", timeout.as_secs_f64()), + )); + } + hint::spin_loop(); + } + + Ok(()) + } + + /// Terminate the child process using custom signal parameter and wait for the termination. + /// + /// Ignores any errors happening during [`Child::kill`] (i.e. child process already exited) but + /// still panics on timeout. + /// + /// # Panics + /// If the child process could not be terminated within `self.timeout` or the default of 60s. + #[cfg(unix)] + pub fn kill_with_custom_signal(&mut self, signal_name: sys::signal::Signal) -> &mut Self { + self.try_kill_with_custom_signal(signal_name) + .or_else(|error| { + // We still throw the error on timeout in the `try_kill` function + if error.kind() == io::ErrorKind::Other { + Err(error) + } else { + Ok(()) + } + }) + .unwrap(); + self + } + /// Wait for the child process to terminate and return a [`CmdResult`]. /// /// See [`UChild::wait_with_output`] for details on timeouts etc. This method can also be run if