From a9cade6fcd0e530e9af0122b18e77d4f097eaf53 Mon Sep 17 00:00:00 2001 From: Etienne Cordonnier Date: Tue, 9 Dec 2025 22:50:42 +0100 Subject: [PATCH 1/2] lib.rs: restore default SIGPIPE signal handler Rust's start-up code sets the SIGPIPE signal handler to ignored. However the vast majority of the utilities should use the default signal handler for SIGPIPE. Instead of restoring the default signaler handler in individual utilities, do it in lib.rs and add some logic in the utilities which can't use the default SIGPIPE signal handler (cat, env, seq, split, tail, tee, tr, tty). Signed-off-by: Etienne Cordonnier --- src/uu/cat/src/cat.rs | 11 ----------- src/uu/env/src/env.rs | 8 -------- src/uu/seq/Cargo.toml | 1 + src/uu/seq/src/error.rs | 5 +++++ src/uu/seq/src/seq.rs | 10 ++++++++++ src/uu/split/Cargo.toml | 2 +- src/uu/split/src/split.rs | 12 ++++++++++++ src/uu/tail/src/tail.rs | 9 --------- src/uu/tee/src/tee.rs | 5 ++++- src/uu/tr/src/tr.rs | 11 ----------- src/uu/tty/Cargo.toml | 2 +- src/uu/tty/src/tty.rs | 10 ++++++++++ src/uucore/src/lib/features/signals.rs | 8 ++++++++ src/uucore/src/lib/lib.rs | 18 +++++++++++++++--- tests/by-util/test_dd.rs | 11 +++++++++-- 15 files changed, 76 insertions(+), 47 deletions(-) diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index 02a85ade02b..5ced5a0196f 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -25,8 +25,6 @@ use std::os::unix::net::UnixStream; use thiserror::Error; use uucore::display::Quotable; use uucore::error::UResult; -#[cfg(not(target_os = "windows"))] -use uucore::libc; use uucore::translate; use uucore::{fast_inc::fast_inc_one, format_usage}; @@ -222,15 +220,6 @@ mod options { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - // When we receive a SIGPIPE signal, we want to terminate the process so - // that we don't print any error messages to stderr. Rust ignores SIGPIPE - // (see https://github.com/rust-lang/rust/issues/62569), so we restore it's - // default action here. - #[cfg(not(target_os = "windows"))] - unsafe { - libc::signal(libc::SIGPIPE, libc::SIG_DFL); - } - let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; let number_mode = if matches.get_flag(options::NUMBER_NONBLANK) { diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index 72f5aa7923e..7576e2472a3 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -18,8 +18,6 @@ use native_int_str::{ Convert, NCvt, NativeIntStr, NativeIntString, NativeStr, from_native_int_representation_owned, }; #[cfg(unix)] -use nix::libc; -#[cfg(unix)] use nix::sys::signal::{SigHandler::SigIgn, Signal, signal}; use std::borrow::Cow; use std::env; @@ -845,12 +843,6 @@ fn ignore_signal(sig: Signal) -> UResult<()> { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - // Rust ignores SIGPIPE (see https://github.com/rust-lang/rust/issues/62569). - // We restore its default action here. - #[cfg(unix)] - unsafe { - libc::signal(libc::SIGPIPE, libc::SIG_DFL); - } EnvAppData::default().run_env(args) } diff --git a/src/uu/seq/Cargo.toml b/src/uu/seq/Cargo.toml index 6f74ce37a9e..534b675e126 100644 --- a/src/uu/seq/Cargo.toml +++ b/src/uu/seq/Cargo.toml @@ -30,6 +30,7 @@ uucore = { workspace = true, features = [ "format", "parser", "quoting-style", + "signals", ] } fluent = { workspace = true } diff --git a/src/uu/seq/src/error.rs b/src/uu/seq/src/error.rs index 2fd92938c10..5193f73568d 100644 --- a/src/uu/seq/src/error.rs +++ b/src/uu/seq/src/error.rs @@ -36,6 +36,11 @@ pub enum SeqError { translate!("seq-error-format-and-equal-width") )] FormatAndEqualWidth, + + /// Failed to set signal handler + #[cfg(unix)] + #[error("failed to set signal handler")] + SignalHandler, } fn parse_error_type(e: &ParseNumberError) -> String { diff --git a/src/uu/seq/src/seq.rs b/src/uu/seq/src/seq.rs index 7b56c26f574..e7fde351a44 100644 --- a/src/uu/seq/src/seq.rs +++ b/src/uu/seq/src/seq.rs @@ -17,6 +17,9 @@ use uucore::format::num_format::FloatVariant; use uucore::format::{Format, num_format}; use uucore::{fast_inc::fast_inc, format_usage}; +#[cfg(unix)] +use uucore::signals::ignore_pipe; + mod error; // public to allow fuzzing @@ -92,6 +95,13 @@ fn select_precision( #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { + #[cfg(unix)] + { + // Ignore SIGPIPE so that broken pipes return EPIPE errors instead of + // killing the process. This allows seq to handle broken pipes gracefully + // (printing an error message but exiting with status 0, matching GNU seq behavior). + ignore_pipe().map_err(|_| SeqError::SignalHandler)?; + } let matches = uucore::clap_localization::handle_clap_result(uu_app(), split_short_args_with_value(args))?; diff --git a/src/uu/split/Cargo.toml b/src/uu/split/Cargo.toml index 2c51bb7807a..b8f67b677e6 100644 --- a/src/uu/split/Cargo.toml +++ b/src/uu/split/Cargo.toml @@ -20,7 +20,7 @@ path = "src/split.rs" [dependencies] clap = { workspace = true } memchr = { workspace = true } -uucore = { workspace = true, features = ["fs", "parser-size"] } +uucore = { workspace = true, features = ["fs", "parser-size", "signals"] } thiserror = { workspace = true } fluent = { workspace = true } diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index 3fe80cd5d93..305f8b39d96 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -29,6 +29,9 @@ use uucore::parser::parse_size::parse_size_u64; use uucore::format_usage; use uucore::uio_error; +#[cfg(unix)] +use uucore::signals::ignore_pipe; + static OPT_BYTES: &str = "bytes"; static OPT_LINE_BYTES: &str = "line-bytes"; static OPT_LINES: &str = "lines"; @@ -1530,6 +1533,15 @@ where #[allow(clippy::cognitive_complexity)] fn split(settings: &Settings) -> UResult<()> { + #[cfg(unix)] + { + // When using --filter, ignore SIGPIPE so that closure of one pipe + // does not terminate the process, as there may still be other streams + // expecting input from us. This matches GNU split behavior. + if settings.filter.is_some() { + ignore_pipe().map_err(|_| USimpleError::new(1, "failed to set signal handler"))?; + } + } let r_box = if settings.input == "-" { Box::new(stdin()) as Box } else { diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index cd10203b3f7..e4a3e3d2801 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -40,15 +40,6 @@ use uucore::{show, show_error}; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - // When we receive a SIGPIPE signal, we want to terminate the process so - // that we don't print any error messages to stderr. Rust ignores SIGPIPE - // (see https://github.com/rust-lang/rust/issues/62569), so we restore it's - // default action here. - #[cfg(not(target_os = "windows"))] - unsafe { - libc::signal(libc::SIGPIPE, libc::SIG_DFL); - } - let settings = parse_args(args)?; settings.check_warnings(); diff --git a/src/uu/tee/src/tee.rs b/src/uu/tee/src/tee.rs index 17afea9384f..8458dca7e46 100644 --- a/src/uu/tee/src/tee.rs +++ b/src/uu/tee/src/tee.rs @@ -19,7 +19,7 @@ use uucore::{format_usage, show_error}; // spell-checker:ignore nopipe #[cfg(unix)] -use uucore::signals::{enable_pipe_errors, ignore_interrupts}; +use uucore::signals::{enable_pipe_errors, ignore_interrupts, ignore_pipe}; mod options { pub const APPEND: &str = "append"; @@ -164,6 +164,9 @@ fn tee(options: &Options) -> Result<()> { } if options.output_error.is_none() { enable_pipe_errors().map_err(|_| Error::from(ErrorKind::Other))?; + } else { + // When --output-error is set, ignore SIGPIPE to handle broken pipes gracefully + ignore_pipe().map_err(|_| Error::from(ErrorKind::Other))?; } } let mut writers: Vec = options diff --git a/src/uu/tr/src/tr.rs b/src/uu/tr/src/tr.rs index d9349baa5bb..3a0ee625371 100644 --- a/src/uu/tr/src/tr.rs +++ b/src/uu/tr/src/tr.rs @@ -18,8 +18,6 @@ use std::io::{stdin, stdout}; use uucore::display::Quotable; use uucore::error::{UResult, USimpleError, UUsageError}; use uucore::fs::is_stdin_directory; -#[cfg(not(target_os = "windows"))] -use uucore::libc; use uucore::translate; use uucore::{format_usage, os_str_as_bytes, show}; @@ -33,15 +31,6 @@ mod options { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - // When we receive a SIGPIPE signal, we want to terminate the process so - // that we don't print any error messages to stderr. Rust ignores SIGPIPE - // (see https://github.com/rust-lang/rust/issues/62569), so we restore it's - // default action here. - #[cfg(not(target_os = "windows"))] - unsafe { - libc::signal(libc::SIGPIPE, libc::SIG_DFL); - } - let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?; let delete_flag = matches.get_flag(options::DELETE); diff --git a/src/uu/tty/Cargo.toml b/src/uu/tty/Cargo.toml index 407e8b0d14b..ba66f035dcb 100644 --- a/src/uu/tty/Cargo.toml +++ b/src/uu/tty/Cargo.toml @@ -20,7 +20,7 @@ path = "src/tty.rs" [dependencies] clap = { workspace = true } nix = { workspace = true, features = ["term"] } -uucore = { workspace = true, features = ["fs"] } +uucore = { workspace = true, features = ["fs", "signals"] } fluent = { workspace = true } [[bin]] diff --git a/src/uu/tty/src/tty.rs b/src/uu/tty/src/tty.rs index 984f34c60a4..774f8dc128e 100644 --- a/src/uu/tty/src/tty.rs +++ b/src/uu/tty/src/tty.rs @@ -12,12 +12,22 @@ use uucore::format_usage; use uucore::translate; +#[cfg(unix)] +use uucore::signals::ignore_pipe; + mod options { pub const SILENT: &str = "silent"; } #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { + #[cfg(unix)] + { + // Ignore SIGPIPE so that broken pipes return EPIPE errors instead of + // killing the process. This allows tty to handle write failures gracefully + // and exit with code 3 when stdout fails, matching GNU tty behavior. + ignore_pipe().ok(); + } let matches = uucore::clap_localization::handle_clap_result_with_exit_code(uu_app(), args, 2)?; let silent = matches.get_flag(options::SILENT); diff --git a/src/uucore/src/lib/features/signals.rs b/src/uucore/src/lib/features/signals.rs index 0bccb2173f6..125ad40d06e 100644 --- a/src/uucore/src/lib/features/signals.rs +++ b/src/uucore/src/lib/features/signals.rs @@ -426,6 +426,14 @@ pub fn ignore_interrupts() -> Result<(), Errno> { unsafe { signal(SIGINT, SigIgn) }.map(|_| ()) } +/// Ignores the SIGPIPE signal. +#[cfg(unix)] +pub fn ignore_pipe() -> Result<(), Errno> { + // We pass the error as is, the return value would just be Ok(SigIgn), so we can safely ignore it. + // SAFETY: this function is safe as long as we do not use a custom SigHandler -- we use the default one. + unsafe { signal(SIGPIPE, SigIgn) }.map(|_| ()) +} + #[test] fn signal_by_value() { assert_eq!(signal_by_name_or_value("0"), Some(0)); diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index 40632ae983b..899857f29fd 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -131,7 +131,8 @@ pub use crate::features::selinux; use nix::errno::Errno; #[cfg(unix)] use nix::sys::signal::{ - SaFlags, SigAction, SigHandler::SigDfl, SigSet, Signal::SIGBUS, Signal::SIGSEGV, sigaction, + SaFlags, SigAction, SigHandler::SigDfl, SigSet, Signal::SIGBUS, Signal::SIGPIPE, + Signal::SIGSEGV, sigaction, }; use std::borrow::Cow; use std::ffi::{OsStr, OsString}; @@ -143,10 +144,12 @@ use std::str; use std::str::Utf8Chunk; use std::sync::{LazyLock, atomic::Ordering}; -/// 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. +/// Restore standard Unix signal behavior. #[cfg(unix)] pub fn disable_rust_signal_handlers() -> Result<(), Errno> { + // Disable 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. unsafe { sigaction( SIGSEGV, @@ -159,6 +162,15 @@ pub fn disable_rust_signal_handlers() -> Result<(), Errno> { &SigAction::new(SigDfl, SaFlags::empty(), SigSet::all()), ) }?; + // Reset SIGPIPE to default behavior since Rust ignores it by default. + // This ensures Unix utilities behave normally when pipes are broken. + // See https://github.com/rust-lang/rust/issues/62569 for discussion. + unsafe { + sigaction( + SIGPIPE, + &SigAction::new(SigDfl, SaFlags::empty(), SigSet::all()), + ) + }?; Ok(()) } diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 0bce976dc80..5b589aa8b49 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -1251,13 +1251,20 @@ fn test_final_stats_more_than_one_kb() { #[test] fn test_final_stats_three_char_limit() { - let result = new_ucmd!().pipe_in("0".repeat(10_000)).succeeds(); + // Use of=/dev/null to avoid SIGPIPE when stdout pipe closes before dd finishes writing + let result = new_ucmd!() + .arg("of=/dev/null") + .pipe_in("0".repeat(10_000)) + .succeeds(); let s = result.stderr_str(); assert!( s.starts_with("19+1 records in\n19+1 records out\n10000 bytes (10 kB, 9.8 KiB) copied,") ); - let result = new_ucmd!().pipe_in("0".repeat(100_000)).succeeds(); + let result = new_ucmd!() + .arg("of=/dev/null") + .pipe_in("0".repeat(100_000)) + .succeeds(); let s = result.stderr_str(); assert!( s.starts_with("195+1 records in\n195+1 records out\n100000 bytes (100 kB, 98 KiB) copied,") From 70a6245903728a2312e73cf45f06d239ddc92d38 Mon Sep 17 00:00:00 2001 From: Etienne Cordonnier Date: Wed, 10 Dec 2025 14:16:00 +0100 Subject: [PATCH 2/2] revert changes to test_dd --- tests/by-util/test_dd.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 5b589aa8b49..0bce976dc80 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -1251,20 +1251,13 @@ fn test_final_stats_more_than_one_kb() { #[test] fn test_final_stats_three_char_limit() { - // Use of=/dev/null to avoid SIGPIPE when stdout pipe closes before dd finishes writing - let result = new_ucmd!() - .arg("of=/dev/null") - .pipe_in("0".repeat(10_000)) - .succeeds(); + let result = new_ucmd!().pipe_in("0".repeat(10_000)).succeeds(); let s = result.stderr_str(); assert!( s.starts_with("19+1 records in\n19+1 records out\n10000 bytes (10 kB, 9.8 KiB) copied,") ); - let result = new_ucmd!() - .arg("of=/dev/null") - .pipe_in("0".repeat(100_000)) - .succeeds(); + let result = new_ucmd!().pipe_in("0".repeat(100_000)).succeeds(); let s = result.stderr_str(); assert!( s.starts_with("195+1 records in\n195+1 records out\n100000 bytes (100 kB, 98 KiB) copied,")