From 327fce0a3bb9e3eba60bd1f2a33acebd384c8022 Mon Sep 17 00:00:00 2001 From: m0g3r <87276771+m0g3r@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:34:35 +0200 Subject: [PATCH 1/3] stty: do not write settings back for query-only arguments `stty size` is a query, but any invocation carrying settings arguments ended with an unconditional `tcsetattr`. POSIX requires the kernel to raise SIGTTOU on `tcsetattr` from a background process group, and its default disposition stops the process, so `stty size` run off the foreground hangs with no diagnostic and never reaps. Only call `tcsetattr` when at least one argument actually changes the terminal settings. `Print` arguments do not. Fixes #13722 Co-authored-by: Claude Opus 5 --- src/uu/stty/src/stty.rs | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index 193c863c3d3..34cba9ffbdc 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -159,6 +159,17 @@ enum ArgOptions<'a> { SavedState(Vec), } +impl ArgOptions<'_> { + /// Whether applying this argument changes the terminal settings. + /// + /// `Print` arguments only query the terminal, so a command built entirely out of them must + /// not call `tcsetattr`: doing so raises `SIGTTOU` in a background process group, which + /// stops the process instead of answering the query. + fn modifies_termios(&self) -> bool { + !matches!(self, ArgOptions::Print(_)) + } +} + impl<'a> From> for ArgOptions<'a> { fn from(flag: AllFlags<'a>) -> Self { ArgOptions::Flags(flag) @@ -436,7 +447,10 @@ fn stty(opts: &Options) -> UResult<()> { } } } - tcsetattr(opts.file.as_fd(), set_arg, &termios)?; + // A query-only invocation such as `stty size` must not write the settings back. + if valid_args.iter().any(ArgOptions::modifies_termios) { + tcsetattr(opts.file.as_fd(), set_arg, &termios)?; + } } else { let termios = tcgetattr(opts.file.as_fd()).map_err_context(|| opts.device_name.clone())?; print_settings(&termios, opts)?; @@ -1346,6 +1360,18 @@ mod tests { // Essential unit tests for complex internal parsing and logic functions. + #[test] + fn test_print_settings_do_not_modify_termios() { + // `stty size` and `stty --help`-style queries must not reach `tcsetattr`, otherwise + // they raise SIGTTOU and hang when run from a background process group. + assert!(!ArgOptions::Print(PrintSetting::Size).modifies_termios()); + + // Anything that actually applies a setting still has to be written back. + assert!(ArgOptions::Mapping((S::VEOF, 4)).modifies_termios()); + assert!(ArgOptions::SavedState(vec![0; 3]).modifies_termios()); + assert!(ArgOptions::Special(SpecialSetting::Rows(24)).modifies_termios()); + } + // Control character parsing tests #[test] fn test_string_to_control_char_undef() { From 93d7b2df34e5a4672e09d732d054cdb44cebc9bf Mon Sep 17 00:00:00 2001 From: m0g3r <87276771+m0g3r@users.noreply.github.com> Date: Sun, 16 Aug 2026 15:10:55 +0200 Subject: [PATCH 2/3] tests/stty: cover `stty size` in a background process group Add a regression test for the SIGTTOU stop this change fixes. The test allocates a PTY, gives a helper its own session and controlling terminal, then runs `stty size` from a background process group with SIGTTOU restored to its default disposition and asserts it exits normally rather than being stopped. Verified against the fix: it passes with the patch, and reverting src/uu/stty/src/stty.rs to its pre-fix state makes it fail with "`stty size` was stopped by SIGTTOU in a background process group" (signal 22). Co-Authored-By: Claude Opus 5 --- tests/by-util/test_stty.rs | 91 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/tests/by-util/test_stty.rs b/tests/by-util/test_stty.rs index f8a2095a89d..8ef0fc375d5 100644 --- a/tests/by-util/test_stty.rs +++ b/tests/by-util/test_stty.rs @@ -34,6 +34,97 @@ fn test_basic() { .stdout_contains("speed"); } +#[test] +#[cfg(unix)] +fn test_size_from_background_process_group() { + use std::env; + use std::io; + use std::os::unix::process::CommandExt; + use std::process::{Command, Stdio}; + + const HELPER_ENV: &str = "UUTILS_STTY_BACKGROUND_HELPER"; + const TEST_NAME: &str = "test_stty::test_size_from_background_process_group"; + + if env::var_os(HELPER_ENV).is_some() { + let mut command = Command::new(uutests::util::get_tests_binary()); + command.args(["stty", "size"]); + // SAFETY: setpgid and signal are async-signal-safe and do not access memory shared with + // the parent between fork and exec. + unsafe { + command.pre_exec(|| { + if libc::setpgid(0, 0) == -1 { + return Err(io::Error::last_os_error()); + } + libc::signal(libc::SIGTTOU, libc::SIG_DFL); + Ok(()) + }); + } + + let mut child = command.spawn().expect("failed to start stty"); + let pid = child.id() as libc::pid_t; + let mut status = 0; + // SAFETY: pid belongs to child and status points to a valid integer for waitpid to fill. + assert_eq!( + unsafe { libc::waitpid(pid, &raw mut status, libc::WUNTRACED) }, + pid + ); + + if libc::WIFSTOPPED(status) { + let signal = libc::WSTOPSIG(status); + // SAFETY: pid is also the process-group ID established by setpgid above. + unsafe { + libc::kill(-pid, libc::SIGKILL); + } + child.wait().expect("failed to reap stopped stty"); + assert_ne!( + signal, + libc::SIGTTOU, + "`stty size` was stopped by SIGTTOU in a background process group" + ); + panic!("`stty size` was stopped by signal {signal}"); + } + + // waitpid already reaped a child that exited normally; calling wait keeps Child's + // lifecycle explicit and should therefore report that no child remains. + let _ = child.wait(); + + assert!( + libc::WIFEXITED(status), + "`stty size` ended with {status:#x}" + ); + assert_eq!(libc::WEXITSTATUS(status), 0); + return; + } + + let (_path, _controller, replica) = pty_path(); + let mut helper = Command::new(env::current_exe().unwrap()); + helper + .args([TEST_NAME, "--exact", "--nocapture"]) + .env(HELPER_ENV, "1") + .stdin(Stdio::from(replica)); + // SAFETY: these libc calls are async-signal-safe. They give the helper its own session and + // make the fresh PTY on stdin its controlling terminal before exec. + unsafe { + helper.pre_exec(|| { + if libc::setsid() == -1 + || libc::ioctl(0, libc::TIOCSCTTY as libc::c_ulong, 0) == -1 + || libc::tcsetpgrp(0, libc::getpgrp()) == -1 + { + return Err(io::Error::last_os_error()); + } + Ok(()) + }); + } + + let output = helper.output().expect("failed to start test helper"); + assert!( + output.status.success(), + "background process-group helper failed:\n{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + #[test] #[cfg(unix)] fn test_all_flag() { From d769acd57598e43c677c8f681d9856369834647f Mon Sep 17 00:00:00 2001 From: m0g3r <87276771+m0g3r@users.noreply.github.com> Date: Tue, 18 Aug 2026 17:56:23 +0200 Subject: [PATCH 3/3] tests/stty: cast TIOCSCTTY to the target's ioctl request type `libc::ioctl`'s request parameter is `libc::Ioctl`, which is `c_ulong` on glibc and Darwin but `c_int` on musl and Android. Casting TIOCSCTTY to `c_ulong` unconditionally broke the test build on those targets: error[E0308]: mismatched types --> tests/by-util/test_stty.rs:110:35 | libc::ioctl(0, libc::TIOCSCTTY as libc::c_ulong, 0) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `i32`, found `u64` Let inference pick the request type from the signature instead. Checked with `cargo check` against x86_64-unknown-linux-musl, x86_64-unknown-linux-gnu and aarch64-apple-darwin: the old cast fails on musl with exactly the error above, the new one compiles on all three. `cargo test --test tests --features feat_os_unix test_stty` passes 52 tests, including test_size_from_background_process_group. Co-Authored-By: Claude Opus 5 --- tests/by-util/test_stty.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/by-util/test_stty.rs b/tests/by-util/test_stty.rs index 8ef0fc375d5..6eeec4a8f70 100644 --- a/tests/by-util/test_stty.rs +++ b/tests/by-util/test_stty.rs @@ -107,7 +107,7 @@ fn test_size_from_background_process_group() { unsafe { helper.pre_exec(|| { if libc::setsid() == -1 - || libc::ioctl(0, libc::TIOCSCTTY as libc::c_ulong, 0) == -1 + || libc::ioctl(0, libc::TIOCSCTTY as _, 0) == -1 || libc::tcsetpgrp(0, libc::getpgrp()) == -1 { return Err(io::Error::last_os_error());