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

Fix reuse of pty slave devices #2449

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions fs/pty.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ static int pty_slave_open(struct tty *tty) {
return _EIO;
if (tty->pty.locked)
return _EIO;
// If userland's reference count on the pty slave is now 1, clear
// hang_up on the pty master. But the session leader may have a
// reference, and the pty master always has a reference.
if (tty->refcount - 1 == (tty->session ? 2 : 1)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found this check a bit unsettling in the last PR but figured it could be fixed later. Seeing it again, I'm thinking much the same thing... Ideally we would define explicitly which references count for this and which ones don't, and with that knowledge the likely solution is to create a second refcount field for tracking just those.

tty->pty.other->hung_up = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs locking, right?

}
return 0;
}

Expand Down
1 change: 1 addition & 0 deletions fs/tty.c
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ static ssize_t tty_read(struct fd *fd, void *buf, size_t bufsize) {
lock(&tty->lock);
if (tty->hung_up || pty_is_half_closed_master(tty)) {
unlock(&pids_lock);
err = -1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an errno constant

goto error;
}

Expand Down
4 changes: 2 additions & 2 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ endif

subdir('tools')

gdb_scripts = ['ish-gdb.gdb']
foreach script : gdb_scripts
debugger_scripts = ['ish-gdb.gdb', 'ish-lldb.lldb']
foreach script : debugger_scripts
custom_target(script,
output: script, input: script,
command: ['ln', '-sf', '@INPUT@', '@OUTPUT@'],
Expand Down
Loading