Skip to content

Commit

Permalink
Fix tcgetpgrp and tcsetpgrp on Linux. (#910)
Browse files Browse the repository at this point in the history
On Linux, fix how the pid argument is passed to the `TIOCSPGRP` ioctl used
by `tcsetgrp`.

And, on Linux, it appears `tcgetpgrp` can return a pid of 0 when the fd is
a pseudo-terminal device. This isn't documented behavior, and isn't
compatible with rustix's current signaure for `tcgetpgrp` since it returns
a `Pid` which can't be zero, so handle this situation by having it return
`Errno::OPNOTSUPP` for now.
  • Loading branch information
sunfishcode authored Nov 6, 2023
1 parent cf2ad7f commit bc4fa30
Show file tree
Hide file tree
Showing 7 changed files with 211 additions and 2 deletions.
9 changes: 9 additions & 0 deletions src/backend/libc/termios/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,15 @@ pub(crate) fn tcgetattr(fd: BorrowedFd<'_>) -> io::Result<Termios> {
pub(crate) fn tcgetpgrp(fd: BorrowedFd<'_>) -> io::Result<Pid> {
unsafe {
let pid = ret_pid_t(c::tcgetpgrp(borrowed_fd(fd)))?;

// This doesn't appear to be documented, but on Linux, it appears
// `tcsetpgrp` can succceed and set the pid to 0 if we pass it a
// pseudo-terminal device fd. For now, translate it into `OPNOTSUPP`.
#[cfg(linux_kernel)]
if pid == 0 {
return Err(io::Errno::OPNOTSUPP);
}

Ok(Pid::from_raw_unchecked(pid))
}
}
Expand Down
18 changes: 17 additions & 1 deletion src/backend/linux_raw/termios/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ pub(crate) fn tcgetpgrp(fd: BorrowedFd<'_>) -> io::Result<Pid> {
let mut result = MaybeUninit::<c::pid_t>::uninit();
ret(syscall!(__NR_ioctl, fd, c_uint(TIOCGPGRP), &mut result))?;
let pid = result.assume_init();

// This doesn't appear to be documented, but it appears `tcsetpgrp` can
// succceed and set the pid to 0 if we pass it a pseudo-terminal device
// fd. For now, fail with `OPNOTSUPP`.
if pid == 0 {
return Err(io::Errno::OPNOTSUPP);
}

Ok(Pid::from_raw_unchecked(pid))
}
}
Expand Down Expand Up @@ -177,7 +185,15 @@ pub(crate) fn tcsetwinsize(fd: BorrowedFd<'_>, winsize: Winsize) -> io::Result<(

#[inline]
pub(crate) fn tcsetpgrp(fd: BorrowedFd<'_>, pid: Pid) -> io::Result<()> {
unsafe { ret(syscall!(__NR_ioctl, fd, c_uint(TIOCSPGRP), pid)) }
let raw_pid: c::c_int = pid.as_raw_nonzero().get();
unsafe {
ret(syscall_readonly!(
__NR_ioctl,
fd,
c_uint(TIOCSPGRP),
by_ref(&raw_pid)
))
}
}

/// A wrapper around a conceptual `cfsetspeed` which handles an arbitrary
Expand Down
8 changes: 7 additions & 1 deletion src/termios/tc.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use crate::fd::AsFd;
use crate::pid::Pid;
#[cfg(not(target_os = "espidf"))]
use crate::termios::{Action, OptionalActions, QueueSelector, Termios, Winsize};
use crate::{backend, io};

pub use crate::pid::Pid;

/// `tcgetattr(fd)`—Get terminal attributes.
///
/// Also known as the `TCGETS` (or `TCGETS2` on Linux) operation with `ioctl`.
Expand Down Expand Up @@ -44,6 +45,11 @@ pub fn tcgetwinsize<Fd: AsFd>(fd: Fd) -> io::Result<Winsize> {
///
/// Also known as the `TIOCGPGRP` operation with `ioctl`.
///
/// On Linux, if `fd` is a pseudo-terminal, the underlying system call here can
/// return a pid of 0, which rustix's `Pid` type doesn't support. So rustix
/// instead handles this case by failing with [`io::Errno::OPNOTSUPP`] if the
/// pid is 0.
///
/// # References
/// - [POSIX]
/// - [Linux]
Expand Down
4 changes: 4 additions & 0 deletions tests/termios/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

#[cfg(not(windows))]
mod isatty;
#[cfg(not(any(windows, target_os = "wasi")))]
mod pgrp;
#[cfg(not(any(windows, target_os = "wasi")))]
mod sid;
#[cfg(all(not(windows), feature = "pty"))]
mod termios;
#[cfg(not(any(windows, target_os = "fuchsia")))]
Expand Down
55 changes: 55 additions & 0 deletions tests/termios/pgrp.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
use rustix::io::Errno;
use rustix::termios::{tcgetpgrp, tcsetpgrp, Pid};
use tempfile::tempdir;

#[cfg(feature = "fs")]
#[test]
fn pgrp_notty() {
let tmpdir = tempdir().unwrap();
let fd = rustix::fs::open(
tmpdir.path(),
rustix::fs::OFlags::RDONLY,
rustix::fs::Mode::empty(),
)
.unwrap();

// A file is not a tty.
assert_eq!(tcgetpgrp(&fd), Err(Errno::NOTTY));
assert_eq!(tcsetpgrp(&fd, Pid::INIT), Err(Errno::NOTTY));
}

// Disable on illumos where `tcgetattr` doesn't appear to support
// pseudoterminals.
#[cfg(not(target_os = "illumos"))]
#[cfg(feature = "pty")]
#[test]
fn pgrp_pseudoterminal() {
use rustix::pty::*;
use rustix::termios::*;

let pty = match openpt(OpenptFlags::NOCTTY) {
Ok(pty) => pty,
Err(rustix::io::Errno::NOSYS) => return,
Err(e) => Err(e).unwrap(),
};

// Linux's `tcgetpgrp` returns 0 here, which is not documented, so rustix
// translates it into `OPNOTSUPP`.
#[cfg(linux_kernel)]
assert_eq!(tcgetpgrp(&pty), Err(rustix::io::Errno::OPNOTSUPP));

// FreeBSD's `tcgetpgrp` returns 100000 here, or presumably some other
// number if that number is already taken, which is documented behavior,
// but impossible to test for reliably.
#[cfg(not(linux_kernel))]
assert!(matches!(tcgetpgrp(&pty), Ok(_)));

// We shouldn't be able to set the process group to pid 1.
match tcsetpgrp(&pty, rustix::termios::Pid::INIT).unwrap_err() {
#[cfg(freebsdlike)]
rustix::io::Errno::PERM => {}
#[cfg(any(apple, linux_kernel))]
rustix::io::Errno::NOTTY => {}
err => Err(err).unwrap(),
}
}
27 changes: 27 additions & 0 deletions tests/termios/sid.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
use rustix::io::Errno;
use rustix::termios::tcgetsid;
use tempfile::tempdir;

#[cfg(feature = "fs")]
#[test]
fn sid_notty() {
let tmpdir = tempdir().unwrap();
let fd = rustix::fs::open(
tmpdir.path(),
rustix::fs::OFlags::RDONLY,
rustix::fs::Mode::empty(),
)
.unwrap();

// A file is not a tty.
assert_eq!(tcgetsid(&fd), Err(Errno::NOTTY));
}

#[cfg(all(feature = "stdio", feature = "process"))]
#[test]
fn sid_match() {
match tcgetsid(rustix::stdio::stdin()) {
Ok(sid) => assert_eq!(sid, rustix::process::getsid(None).unwrap()),
Err(_err) => {}
}
}
92 changes: 92 additions & 0 deletions tests/termios/termios.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,86 @@
#[test]
fn test_termios_flush() {
use rustix::pty::*;
use rustix::termios::*;

let pty = match openpt(OpenptFlags::empty()) {
Ok(pty) => pty,
Err(rustix::io::Errno::NOSYS) => return,
Err(e) => Err(e).unwrap(),
};
let tio = match tcgetattr(&pty) {
Ok(tio) => tio,
Err(rustix::io::Errno::NOSYS) => return,
#[cfg(apple)]
Err(rustix::io::Errno::NOTTY) => return,
Err(e) => Err(e).unwrap(),
};
tcsetattr(&pty, OptionalActions::Now, &tio).unwrap();

tcflush(&pty, QueueSelector::IOFlush).unwrap();
}

#[test]
fn test_termios_drain() {
use rustix::pty::*;
use rustix::termios::*;

let pty = match openpt(OpenptFlags::empty()) {
Ok(pty) => pty,
Err(rustix::io::Errno::NOSYS) => return,
Err(e) => Err(e).unwrap(),
};
let tio = match tcgetattr(&pty) {
Ok(tio) => tio,
Err(rustix::io::Errno::NOSYS) => return,
#[cfg(apple)]
Err(rustix::io::Errno::NOTTY) => return,
Err(e) => Err(e).unwrap(),
};
tcsetattr(&pty, OptionalActions::Now, &tio).unwrap();

tcdrain(&pty).unwrap();
}

#[test]
fn test_termios_winsize() {
use rustix::pty::*;
use rustix::termios::*;

let pty = match openpt(OpenptFlags::empty()) {
Ok(pty) => pty,
Err(rustix::io::Errno::NOSYS) => return,
Err(e) => Err(e).unwrap(),
};

// Sizes for a pseudoterminal start out 0.
let mut sizes = match tcgetwinsize(&pty) {
Ok(sizes) => sizes,
// Apple doesn't appear to support `tcgetwinsize` on a pty.
#[cfg(apple)]
Err(rustix::io::Errno::NOTTY) => return,
Err(err) => Err(err).unwrap(),
};
assert_eq!(sizes.ws_row, 0);
assert_eq!(sizes.ws_col, 0);
assert_eq!(sizes.ws_xpixel, 0);
assert_eq!(sizes.ws_ypixel, 0);

// Set some arbitrary sizes.
sizes.ws_row = 28;
sizes.ws_col = 82;
sizes.ws_xpixel = 365;
sizes.ws_ypixel = 794;
tcsetwinsize(&pty, sizes).unwrap();

// Check that the sizes roundtripped.
let check_sizes = tcgetwinsize(&pty).unwrap();
assert_eq!(check_sizes.ws_row, sizes.ws_row);
assert_eq!(check_sizes.ws_col, sizes.ws_col);
assert_eq!(check_sizes.ws_xpixel, sizes.ws_xpixel);
assert_eq!(check_sizes.ws_ypixel, sizes.ws_ypixel);
}

// Disable on illumos where `tcgetattr` doesn't appear to support
// pseudoterminals.
#[cfg(not(target_os = "illumos"))]
Expand Down Expand Up @@ -86,3 +169,12 @@ fn test_termios_speeds() {
assert_eq!(new_tio.output_speed(), speed::B110);
}
}

#[test]
fn test_termios_tcgetattr_not_tty() {
let file = tempfile::tempfile().unwrap();
assert_eq!(
rustix::termios::tcgetattr(&file).unwrap_err(),
rustix::io::Errno::NOTTY
);
}

0 comments on commit bc4fa30

Please sign in to comment.