Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uucore: disable default signal-handlers added by Rust #6806

Merged
merged 2 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/uucore/src/lib/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -100,6 +102,12 @@

//## 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;
Expand All @@ -112,6 +120,25 @@

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 <https://github.com/rust-lang/rust/blob/8ac1525e091d3db28e67adcbbd6db1e1deaa37fb/src/libstd/sys/unix/stack_overflow.rs#L71-L92> for details.
#[cfg(unix)]
pub fn disable_rust_signal_handlers() -> Result<(), Errno> {
unsafe {
sigaction(
SIGSEGV,
&SigAction::new(SigDfl, SaFlags::empty(), SigSet::all()),
)
}?;

Check warning on line 132 in src/uucore/src/lib/lib.rs

View check run for this annotation

Codecov / codecov/patch

src/uucore/src/lib/lib.rs#L132

Added line #L132 was not covered by tests
unsafe {
sigaction(
SIGBUS,
&SigAction::new(SigDfl, SaFlags::empty(), SigSet::all()),
)
}?;

Check warning on line 138 in src/uucore/src/lib/lib.rs

View check run for this annotation

Codecov / codecov/patch

src/uucore/src/lib/lib.rs#L138

Added line #L138 was not covered by tests
Ok(())
}

/// Execute utility code for `util`.
///
/// This macro expands to a main function that invokes the `uumain` function in `util`
Expand Down
6 changes: 5 additions & 1 deletion src/uucore_procs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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(),
Expand Down
38 changes: 37 additions & 1 deletion tests/by-util/test_sleep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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!()
Expand Down
75 changes: 73 additions & 2 deletions tests/common/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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;
Expand Down Expand Up @@ -2095,7 +2097,7 @@
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
Expand Down Expand Up @@ -2155,6 +2157,75 @@
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()),

Check warning on line 2198 in tests/common/util.rs

View check run for this annotation

Codecov / codecov/patch

tests/common/util.rs#L2196-L2198

Added lines #L2196 - L2198 were not covered by tests
));
}
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| {

Check warning on line 2217 in tests/common/util.rs

View check run for this annotation

Codecov / codecov/patch

tests/common/util.rs#L2217

Added line #L2217 was not covered by tests
// We still throw the error on timeout in the `try_kill` function
if error.kind() == io::ErrorKind::Other {
Err(error)

Check warning on line 2220 in tests/common/util.rs

View check run for this annotation

Codecov / codecov/patch

tests/common/util.rs#L2220

Added line #L2220 was not covered by tests
} else {
Ok(())

Check warning on line 2222 in tests/common/util.rs

View check run for this annotation

Codecov / codecov/patch

tests/common/util.rs#L2222

Added line #L2222 was not covered by tests
}
})
.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
Expand Down
Loading