From a235f1a8f93ce75c01eba9d6d3f07ef46265fa7f Mon Sep 17 00:00:00 2001 From: Etienne Cordonnier Date: Wed, 26 Nov 2025 22:47:52 +0100 Subject: [PATCH] stdbuf: use exec instead of forking Forking creates a new PID and it not compatible with GNU coreutils implementation. - use exec instead of forking - add test verifying that execvp is used Fixes https://github.com/uutils/coreutils/issues/9066 Signed-off-by: Etienne Cordonnier --- src/uu/stdbuf/src/stdbuf.rs | 68 +++++++++------------------------- tests/by-util/test_stdbuf.rs | 71 +++++++++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 51 deletions(-) diff --git a/src/uu/stdbuf/src/stdbuf.rs b/src/uu/stdbuf/src/stdbuf.rs index f45dd2b97b3..b60165f4d48 100644 --- a/src/uu/stdbuf/src/stdbuf.rs +++ b/src/uu/stdbuf/src/stdbuf.rs @@ -7,12 +7,13 @@ use clap::{Arg, ArgAction, ArgMatches, Command}; use std::ffi::OsString; +use std::os::unix::process::CommandExt; use std::path::PathBuf; use std::process; use tempfile::TempDir; use tempfile::tempdir; use thiserror::Error; -use uucore::error::{FromIo, UResult, USimpleError, UUsageError}; +use uucore::error::{UResult, USimpleError, UUsageError}; use uucore::format_usage; use uucore::parser::parse_size::parse_size_u64; use uucore::translate; @@ -208,55 +209,22 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { set_command_env(&mut command, "_STDBUF_E", &options.stderr); command.args(command_params); - let mut process = match command.spawn() { - Ok(p) => p, - Err(e) => { - return match e.kind() { - std::io::ErrorKind::PermissionDenied => Err(USimpleError::new( - 126, - translate!("stdbuf-error-permission-denied"), - )), - std::io::ErrorKind::NotFound => Err(USimpleError::new( - 127, - translate!("stdbuf-error-no-such-file"), - )), - _ => Err(USimpleError::new( - 1, - translate!("stdbuf-error-failed-to-execute", "error" => e), - )), - }; - } - }; - - let status = process.wait().map_err_context(String::new)?; - match status.code() { - Some(i) => { - if i == 0 { - Ok(()) - } else { - Err(i.into()) - } - } - None => { - #[cfg(unix)] - { - use std::os::unix::process::ExitStatusExt; - let signal_msg = status - .signal() - .map_or_else(|| "unknown".to_string(), |s| s.to_string()); - Err(USimpleError::new( - 1, - translate!("stdbuf-error-killed-by-signal", "signal" => signal_msg), - )) - } - #[cfg(not(unix))] - { - Err(USimpleError::new( - 1, - "process terminated abnormally".to_string(), - )) - } - } + // Replace the current process with the target program (no fork) using exec. + let e = command.exec(); + // exec() only returns if there was an error + match e.kind() { + std::io::ErrorKind::PermissionDenied => Err(USimpleError::new( + 126, + translate!("stdbuf-error-permission-denied"), + )), + std::io::ErrorKind::NotFound => Err(USimpleError::new( + 127, + translate!("stdbuf-error-no-such-file"), + )), + _ => Err(USimpleError::new( + 1, + translate!("stdbuf-error-failed-to-execute", "error" => e), + )), } } diff --git a/tests/by-util/test_stdbuf.rs b/tests/by-util/test_stdbuf.rs index c74ad54ec0d..00e117f47e4 100644 --- a/tests/by-util/test_stdbuf.rs +++ b/tests/by-util/test_stdbuf.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore dyld dylib setvbuf +// spell-checker:ignore cmdline dyld dylib PDEATHSIG setvbuf #[cfg(target_os = "linux")] use uutests::at_and_ucmd; use uutests::new_ucmd; @@ -276,3 +276,72 @@ fn test_stdbuf_non_utf8_paths() { .succeeds() .stdout_is("test content for stdbuf\n"); } + +#[test] +#[cfg(target_os = "linux")] +fn test_stdbuf_no_fork_regression() { + // Regression test for issue #9066: https://github.com/uutils/coreutils/issues/9066 + // The original stdbuf implementation used fork+spawn which broke signal handling + // and PR_SET_PDEATHSIG. This test verifies that stdbuf uses exec() instead. + // With fork: stdbuf process would remain visible in process list + // With exec: stdbuf process is replaced by target command (GNU compatible) + + use std::process::{Command, Stdio}; + use std::thread; + use std::time::Duration; + + let scene = TestScenario::new(util_name!()); + + // Start stdbuf with a long-running command + let mut child = Command::new(&scene.bin_path) + .args(["stdbuf", "-o0", "sleep", "3"]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("Failed to start stdbuf"); + + let child_pid = child.id(); + + // Poll until exec happens or timeout + let cmdline_path = format!("/proc/{child_pid}/cmdline"); + let timeout = Duration::from_secs(2); + let poll_interval = Duration::from_millis(10); + let start_time = std::time::Instant::now(); + + let command_name = loop { + if start_time.elapsed() > timeout { + child.kill().ok(); + panic!("TIMEOUT: Process {child_pid} did not respond within {timeout:?}"); + } + + if let Ok(cmdline) = std::fs::read_to_string(&cmdline_path) { + let cmd_parts: Vec<&str> = cmdline.split('\0').collect(); + let name = cmd_parts.first().map_or("", |v| v); + + // Wait for exec to complete (process name changes from original binary to target) + // Handle both multicall binary (coreutils) and individual utilities (stdbuf) + if !name.contains("coreutils") && !name.contains("stdbuf") && !name.is_empty() { + break name.to_string(); + } + } + + thread::sleep(poll_interval); + }; + + // The loop already waited for exec (no longer original binary), so this should always pass + // But keep the assertion as a safety check and clear documentation + assert!( + !command_name.contains("coreutils") && !command_name.contains("stdbuf"), + "REGRESSION: Process {child_pid} is still original binary (coreutils or stdbuf) - fork() used instead of exec()" + ); + + // Ensure we're running the expected target command + assert!( + command_name.contains("sleep"), + "Expected 'sleep' command at PID {child_pid}, got: {command_name}" + ); + + // Cleanup + child.kill().ok(); + child.wait().ok(); +}