Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent Pty::pts() from attaching via O_NOCTTY #16

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cpick
Copy link

@cpick cpick commented Oct 15, 2024

If a process doesn't have a controlling terminal (for example if it is in a new session) then open(ptsname()) will automatically make the slave the controlling terminal of the process.

Later on, command.spawn(&pts)s call to ioctl_tiocsctty() within session_leader() within command.pre_exec() will fail as with EPERM since the parent process (and session) is alread using it as its controlling terminal.

Use O_NOCTTY to prevent this.

Tested on macOS and Linux.

If a process doesn't have a controlling terminal (for example if it is
in a new session) then `open(ptsname())` will automatically make the
slave the controlling terminal of the process.

Later on, `command.spawn(&pts)`s call to `ioctl_tiocsctty()` within
`session_leader()` within `command.pre_exec()` will fail as with EPERM
since the parent process (and session) is alread using it as its
controlling terminal.

Use `O_NOCTTY` to prevent this.
@cpick
Copy link
Author

cpick commented Oct 15, 2024

To reproduce the issue compile:

fn main() {
    if std::env::args().len() > 1 {
        let _pid = rustix::process::setsid().expect("set sid");
    }

    let pty = pty_process::blocking::Pty::new().expect("new pty");
    let pts = pty.pts().expect("pty pts");

    let _child = pty_process::blocking::Command::new("true")
        .spawn(&pts)
        .expect("spawn command");
}

Then running it like so works:

sh -c '(target/debug/binary)'

But the following doesn't:

sh -c '(target/debug/binary broken)'
thread 'main' panicked at src/main.rs:11:10:
spawn command: Io(Os { code: 1, kind: PermissionDenied, message: "Operation not permitted" })
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Note: running it in a sub shell is needed so that it doesn't start out as a process group leader (which would cause setsid() to fail before ever getting to spawn()). Alternatively, the example could fork() and run in a child or call ioctl(1, TIOCNOTTY), but that would be more complicated example code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant