From 468cd6dc9b270c335923adcf2401265a8f7e3ea9 Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Mon, 8 Sep 2025 20:35:04 +0200 Subject: [PATCH 1/3] put sudo parent process in background if stdout is a pipe --- src/exec/use_pty/parent.rs | 5 +++++ src/exec/use_pty/pipe/mod.rs | 39 ++++++++++++++++++++++++++---------- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/exec/use_pty/parent.rs b/src/exec/use_pty/parent.rs index b1e0948ac..0df9060bd 100644 --- a/src/exec/use_pty/parent.rs +++ b/src/exec/use_pty/parent.rs @@ -120,6 +120,7 @@ pub(in crate::exec) fn exec_pty( if !io::stdout().is_terminal() { dev_info!("stdout is not a terminal, command will inherit it"); pipeline = true; + foreground = false; command.stdout(Stdio::inherit()); } @@ -158,6 +159,10 @@ pub(in crate::exec) fn exec_pty( } }; + if !foreground { + tty_pipe.disable_input(&mut registry); + } + // SAFETY: There should be no other threads at this point. let ForkResult::Parent(monitor_pid) = (unsafe { fork() }).map_err(|err| { dev_error!("cannot fork monitor process: {err}"); diff --git a/src/exec/use_pty/pipe/mod.rs b/src/exec/use_pty/pipe/mod.rs index 88409122e..13b0f4c22 100644 --- a/src/exec/use_pty/pipe/mod.rs +++ b/src/exec/use_pty/pipe/mod.rs @@ -16,6 +16,7 @@ pub(super) struct Pipe { right: R, buffer_lr: Buffer, buffer_rl: Buffer, + background: bool, } impl Pipe { @@ -40,6 +41,7 @@ impl Pipe { ), left, right, + background: false, } } @@ -66,9 +68,17 @@ impl Pipe { self.buffer_rl.write_handle.ignore(registry); } + /// Stop the poll events of the left end of this pipe. + pub(super) fn disable_input(&mut self, registry: &mut EventRegistry) { + self.buffer_lr.read_handle.ignore(registry); + self.background = true; + } + /// Resume the poll events of this pipe pub(super) fn resume_events(&mut self, registry: &mut EventRegistry) { - self.buffer_lr.read_handle.resume(registry); + if !self.background { + self.buffer_lr.read_handle.resume(registry); + } self.buffer_lr.write_handle.resume(registry); self.buffer_rl.read_handle.resume(registry); self.buffer_rl.write_handle.resume(registry); @@ -82,7 +92,12 @@ impl Pipe { ) -> io::Result<()> { match poll_event { PollEvent::Readable => self.buffer_lr.read(&mut self.left, registry), - PollEvent::Writable => self.buffer_rl.write(&mut self.left, registry), + PollEvent::Writable => { + if self.buffer_rl.write(&mut self.left, registry)? { + self.buffer_rl.read_handle.resume(registry); + } + Ok(()) + } } } @@ -94,7 +109,13 @@ impl Pipe { ) -> io::Result<()> { match poll_event { PollEvent::Readable => self.buffer_rl.read(&mut self.right, registry), - PollEvent::Writable => self.buffer_lr.write(&mut self.right, registry), + PollEvent::Writable => { + if self.buffer_lr.write(&mut self.right, registry)? && !self.background { + self.buffer_lr.read_handle.resume(registry); + } + + Ok(()) + } } } @@ -164,22 +185,18 @@ impl Buffer { &mut self, write: &mut W, registry: &mut EventRegistry, - ) -> io::Result<()> { + ) -> io::Result { // If the buffer is empty, there is nothing to be written. if self.internal.is_empty() { self.write_handle.ignore(registry); - return Ok(()); + return Ok(false); } // Remove bytes from the buffer and write them. let removed_len = self.internal.remove(write)?; - // If we removed something, the buffer is not full anymore and we can resume reading. - if removed_len > 0 { - self.read_handle.resume(registry); - } - - Ok(()) + // Return whether we actually freed up some buffer space + Ok(removed_len > 0) } /// Flush this buffer, ensuring that all the contents of its internal buffer are written. From 00daa96aa3807afc18dd69b4307b841c88e703ed Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Thu, 11 Sep 2025 16:52:08 +0200 Subject: [PATCH 2/3] add is_pipe function to Terminal --- src/cutils/mod.rs | 30 +++++++++++++++++++----------- src/system/term/mod.rs | 7 ++++++- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/cutils/mod.rs b/src/cutils/mod.rs index bc608d8ee..2373813b4 100644 --- a/src/cutils/mod.rs +++ b/src/cutils/mod.rs @@ -69,10 +69,7 @@ pub unsafe fn os_string_from_ptr(ptr: *const libc::c_char) -> OsString { } } -/// Rust's standard library IsTerminal just directly calls isatty, which -/// we don't want since this performs IOCTL calls on them and file descriptors are under -/// the control of the user; so this checks if they are a character device first. -pub fn safe_isatty(fildes: BorrowedFd) -> bool { +fn fstat_mode_set(fildes: &BorrowedFd, mask: libc::mode_t) -> bool { // The Rust standard library doesn't have FileTypeExt on Std{in,out,err}, so we // can't just use FileTypeExt::is_char_device and have to resort to libc::fstat. let mut maybe_stat = std::mem::MaybeUninit::::uninit(); @@ -83,19 +80,30 @@ pub fn safe_isatty(fildes: BorrowedFd) -> bool { let mode = unsafe { maybe_stat.assume_init() }.st_mode; // To complicate matters further, the S_ISCHR macro isn't in libc as well. - let is_char_device = (mode & libc::S_IFMT) == libc::S_IFCHR; + (mode & libc::S_IFMT) == mask + } else { + false + } +} +/// Rust's standard library IsTerminal just directly calls isatty, which +/// we don't want since this performs IOCTL calls on them and file descriptors are under +/// the control of the user; so this checks if they are a character device first. +pub fn safe_isatty(fildes: BorrowedFd) -> bool { + let is_char_device = fstat_mode_set(&fildes, libc::S_IFCHR); - if is_char_device { - // SAFETY: isatty will return 0 or 1 - unsafe { libc::isatty(fildes.as_raw_fd()) != 0 } - } else { - false - } + if is_char_device { + // SAFETY: isatty will return 0 or 1 + unsafe { libc::isatty(fildes.as_raw_fd()) != 0 } } else { false } } +/// Check whether the file descriptor is a pipe +pub fn is_fifo(fildes: BorrowedFd) -> bool { + fstat_mode_set(&fildes, libc::S_IFIFO) +} + #[allow(clippy::undocumented_unsafe_blocks)] #[cfg(test)] mod test { diff --git a/src/system/term/mod.rs b/src/system/term/mod.rs index 57f376037..8815ff80f 100644 --- a/src/system/term/mod.rs +++ b/src/system/term/mod.rs @@ -11,7 +11,7 @@ use std::{ use libc::{ioctl, winsize, TIOCSWINSZ}; -use crate::cutils::{cerr, os_string_from_ptr, safe_isatty}; +use crate::cutils::{cerr, is_fifo, os_string_from_ptr, safe_isatty}; use super::interface::ProcessId; @@ -157,6 +157,7 @@ pub(crate) trait Terminal: sealed::Sealed { fn make_controlling_terminal(&self) -> io::Result<()>; fn ttyname(&self) -> io::Result; fn is_terminal(&self) -> bool; + fn is_pipe(&self) -> bool; fn tcgetsid(&self) -> io::Result; } @@ -200,6 +201,10 @@ impl Terminal for F { safe_isatty(self.as_fd()) } + fn is_pipe(&self) -> bool { + is_fifo(self.as_fd()) + } + fn tcgetsid(&self) -> io::Result { // SAFETY: tcgetsid cannot cause UB let id = cerr(unsafe { libc::tcgetsid(self.as_fd().as_raw_fd()) })?; From 962e46966e91db71388bd4e3ce4ceb17dba8f0ec Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Thu, 11 Sep 2025 16:52:24 +0200 Subject: [PATCH 3/3] only activate background mode if stdout is a pipe --- src/exec/use_pty/parent.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/exec/use_pty/parent.rs b/src/exec/use_pty/parent.rs index 0df9060bd..9f29ac903 100644 --- a/src/exec/use_pty/parent.rs +++ b/src/exec/use_pty/parent.rs @@ -120,7 +120,6 @@ pub(in crate::exec) fn exec_pty( if !io::stdout().is_terminal() { dev_info!("stdout is not a terminal, command will inherit it"); pipeline = true; - foreground = false; command.stdout(Stdio::inherit()); } @@ -129,6 +128,12 @@ pub(in crate::exec) fn exec_pty( command.stderr(Stdio::inherit()); } + // If there is another process later in the pipeline, don't interfere + // with its access to the Tty + if io::stdout().is_pipe() { + foreground = false; + } + // Copy terminal settings from `/dev/tty` to the pty. if let Err(err) = user_tty.copy_to(&pty.follower) { dev_error!("cannot copy terminal settings to pty: {err}");