From 01103579a5f0c3b877b9bc68500547b16ca1f9ec Mon Sep 17 00:00:00 2001 From: Etienne Cordonnier Date: Wed, 26 Nov 2025 22:47:52 +0100 Subject: [PATCH 1/2] stdbuf: use exec instead of forking forking creates a new PID and it not compatible with GNU coreutils implementation. Fixes https://github.com/uutils/coreutils/issues/9066 Signed-off-by: Etienne Cordonnier --- Cargo.lock | 1 + src/uu/stdbuf/Cargo.toml | 3 + src/uu/stdbuf/src/stdbuf.rs | 130 ++++++++++++++++++++---------------- 3 files changed, 76 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 62063777cbb..a97d40bebe4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3875,6 +3875,7 @@ version = "0.4.0" dependencies = [ "clap", "fluent", + "nix", "tempfile", "thiserror 2.0.17", "uu_stdbuf_libstdbuf", diff --git a/src/uu/stdbuf/Cargo.toml b/src/uu/stdbuf/Cargo.toml index ce64792b7ec..dff10538156 100644 --- a/src/uu/stdbuf/Cargo.toml +++ b/src/uu/stdbuf/Cargo.toml @@ -26,6 +26,9 @@ uucore = { workspace = true, features = ["parser"] } thiserror = { workspace = true } fluent = { workspace = true } +[target.'cfg(unix)'.dependencies] +nix = { workspace = true } + # "feat_external_libstdbuf": use an external libstdbuf.so for stdbuf instead of embedding it into # the stdbuf binary. # There are 2 use-cases: diff --git a/src/uu/stdbuf/src/stdbuf.rs b/src/uu/stdbuf/src/stdbuf.rs index fae2942f0b6..e2c1875c730 100644 --- a/src/uu/stdbuf/src/stdbuf.rs +++ b/src/uu/stdbuf/src/stdbuf.rs @@ -6,13 +6,17 @@ // spell-checker:ignore (ToDO) tempdir dyld dylib optgrps libstdbuf use clap::{Arg, ArgAction, ArgMatches, Command}; +use std::env; +#[cfg(unix)] +use std::ffi::CString; use std::ffi::OsString; +#[cfg(unix)] +use std::os::unix::ffi::OsStrExt; 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; @@ -133,13 +137,13 @@ fn check_option(matches: &ArgMatches, name: &str) -> Result { - command.env(buffer_name, m.to_string()); + unsafe { env::set_var(buffer_name, m.to_string()) }; } BufferType::Line => { - command.env(buffer_name, "L"); + unsafe { env::set_var(buffer_name, "L") }; } BufferType::Default => {} } @@ -193,69 +197,79 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let Some(first_command) = command_values.next() else { return Err(UUsageError::new(125, "no command specified".to_string())); }; - let mut command = process::Command::new(first_command); let command_params: Vec<&OsString> = command_values.collect(); let tmp_dir = tempdir() .map_err(|e| UUsageError::new(125, format!("failed to create temp directory: {e}")))?; let (preload_env, libstdbuf) = get_preload_env(&tmp_dir)?; - command.env(preload_env, libstdbuf); - set_command_env(&mut command, "_STDBUF_I", &options.stdin); - set_command_env(&mut command, "_STDBUF_O", &options.stdout); - 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), - )), - }; - } - }; + // Set preload and stdbuf env vars in the current process so execvp will inherit them. + unsafe { env::set_var(&preload_env, &libstdbuf) }; + set_env_var("_STDBUF_I", &options.stdin); + set_env_var("_STDBUF_O", &options.stdout); + set_env_var("_STDBUF_E", &options.stderr); + + // On Unix, replace the current process with the target program (no fork) using execvp. + #[cfg(unix)] + { + use nix::unistd::execvp; - let status = process.wait().map_err_context(String::new)?; - match status.code() { - Some(i) => { - if i == 0 { - Ok(()) - } else { - Err(i.into()) + // Convert program and args into CString + let prog_bytes = first_command.as_os_str().as_bytes(); + let Ok(prog_c) = CString::new(prog_bytes) else { + return Err(USimpleError::new( + 127, + translate!("stdbuf-error-no-such-file"), + )); + }; + + let mut argv: Vec = Vec::new(); + // push arg0 + let Ok(arg0_c) = CString::new(prog_bytes) else { + return Err(USimpleError::new( + 127, + translate!("stdbuf-error-no-such-file"), + )); + }; + argv.push(arg0_c); + + for p in &command_params { + let bytes = p.as_os_str().as_bytes(); + match CString::new(bytes) { + Ok(c) => argv.push(c), + Err(_) => { + return Err(USimpleError::new( + 127, + translate!("stdbuf-error-no-such-file"), + )); + } } } - None => { - #[cfg(unix)] - { - use std::os::unix::process::ExitStatusExt; - let signal_msg = status - .signal() - .map(|s| s.to_string()) - .unwrap_or_else(|| "unknown".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(), - )) - } + + match execvp(&prog_c, &argv) { + Err(nix::errno::Errno::ENOENT) => Err(USimpleError::new( + 127, + translate!("stdbuf-error-no-such-file"), + )), + Err(nix::errno::Errno::EACCES) => Err(USimpleError::new( + 126, + translate!("stdbuf-error-permission-denied"), + )), + Err(_) => Err(USimpleError::new( + 1, + translate!("stdbuf-error-failed-to-execute", "error" => "execvp failed"), + )), + Ok(_) => unreachable!("execvp should not return on success"), } } + + // On non-Unix platforms, stdbuf is not supported (it relies on LD_PRELOAD/DYLD behavior). + #[cfg(not(unix))] + { + return Err(USimpleError::new( + 1, + translate!("stdbuf-error-command-not-supported"), + )); + } } pub fn uu_app() -> Command { From 4c557bf3561685633d06346da84b87ed0ea600fd Mon Sep 17 00:00:00 2001 From: Etienne Cordonnier Date: Wed, 3 Dec 2025 13:08:50 +0100 Subject: [PATCH 2/2] stdbuf: add test verifying that execvp is used Signed-off-by: Etienne Cordonnier --- tests/by-util/test_stdbuf.rs | 71 +++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/tests/by-util/test_stdbuf.rs b/tests/by-util/test_stdbuf.rs index a19900db3e3..42ccb6a717d 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; @@ -247,3 +247,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(); +}