Skip to content

Commit 5e83a26

Browse files
tanyifenggvisor-bot
authored andcommitted
pty: Send SIGHUP to foreground process group when master is closed
When the PTY master is closed, send SIGHUPto the foreground process group of the session that has the slave as its controlling terminal. This matches Linux's pty_close() -> tty_vhangup(tty->link) behavior (drivers/tty/pty.c). Without this, child processes spawned via PTY (e.g. by expect/telnet) are never notified of the hangup and remain running indefinitely. FUTURE_COPYBARA_INTEGRATE_REVIEW=#12855 from tanyifeng:pty-sighup-on-master-close 1686126 PiperOrigin-RevId: 900302848
1 parent 486f37f commit 5e83a26

3 files changed

Lines changed: 86 additions & 0 deletions

File tree

pkg/sentry/fsimpl/devpts/devpts.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,10 @@ func (i *rootInode) allocateTerminal(ctx context.Context, creds *auth.Credential
286286

287287
// masterClose is called when the master end of t is closed.
288288
func (i *rootInode) masterClose(ctx context.Context, t *Terminal) {
289+
// When the master is closed, hang up the slave (replica) side.
290+
// This corresponds to Linux's pty_close() calling tty_vhangup(tty->link).
291+
t.replicaKTTY.Hangup(ctx)
292+
289293
i.mu.Lock()
290294
defer i.mu.Unlock()
291295

pkg/sentry/kernel/tty.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,3 +158,22 @@ func (tty *TTY) CheckChange(ctx context.Context, sig linux.Signal) error {
158158
_ = pg.SendSignal(SignalInfoPriv(sig))
159159
return linuxerr.ERESTARTSYS
160160
}
161+
162+
// Hangup releases the controlling terminal and sends SIGHUP/SIGCONT to
163+
// the foreground process group. This is called on the replica (slave) TTY
164+
// when the PTY master is closed, corresponding to Linux's pty_close()
165+
// calling tty_vhangup(tty->link).
166+
func (tty *TTY) Hangup(ctx context.Context) {
167+
tty.mu.Lock()
168+
tg := tty.tg
169+
tty.mu.Unlock()
170+
171+
if tg == nil {
172+
// This TTY is not a controlling terminal.
173+
return
174+
}
175+
176+
// Reuse the existing ReleaseControllingTTY logic which handles
177+
// sending SIGHUP/SIGCONT and clearing the controlling terminal.
178+
_ = tg.ReleaseControllingTTY(tty)
179+
}

test/syscalls/linux/pty.cc

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,12 @@ TEST(BasicPtyTest, OpenDevTTY) {
425425
// which will be opened by /dev/tty.
426426
setsid();
427427

428+
// Ignore SIGHUP: when the master fd is closed during cleanup, the
429+
// kernel sends SIGHUP to the foreground process group of the
430+
// controlling terminal session. Since this child *is* the session leader,
431+
// it would be killed by the default SIGHUP disposition before _exit(0).
432+
TEST_PCHECK(signal(SIGHUP, SIG_IGN) != SIG_ERR);
433+
428434
FileDescriptor master =
429435
TEST_CHECK_NO_ERRNO_AND_VALUE(Open("/dev/ptmx", O_RDWR));
430436

@@ -2449,6 +2455,63 @@ TEST_F(PtyTest, SignalCharConsumedWhenISIGEnabled) {
24492455
EXPECT_EQ(buf, 'a');
24502456
}
24512457

2458+
// When the PTY master is closed, SIGHUP should be sent to the foreground
2459+
// process group of the session that has this PTY as its controlling terminal.
2460+
// This matches Linux pty_close() -> tty_vhangup() behavior.
2461+
TEST_F(JobControlTest, SIGHUPOnMasterClose) {
2462+
// Use a pipe to synchronize: the child signals readiness after setting up
2463+
// its session and controlling terminal.
2464+
int sync_pipe[2];
2465+
ASSERT_THAT(pipe(sync_pipe), SyscallSucceeds());
2466+
2467+
pid_t child = fork();
2468+
if (child == 0) {
2469+
close(sync_pipe[0]); // Close read end in child.
2470+
2471+
// Close the inherited master fd so that the parent holds the only
2472+
// reference. pty_close/tty_vhangup fires only when the last master
2473+
// fd is closed.
2474+
close(master_.release());
2475+
2476+
// Create new session and set the replica as controlling terminal.
2477+
TEST_PCHECK(setsid() >= 0);
2478+
TEST_PCHECK(ioctl(replica_.get(), TIOCSCTTY, 0) >= 0);
2479+
2480+
// Install a SIGHUP handler that exits with a known status.
2481+
struct sigaction sa = {};
2482+
sa.sa_handler = [](int) { _exit(42); };
2483+
sigemptyset(&sa.sa_mask);
2484+
TEST_PCHECK(sigaction(SIGHUP, &sa, nullptr) >= 0);
2485+
2486+
// Notify the parent that setup is complete.
2487+
char c = 'r';
2488+
TEST_PCHECK(WriteFd(sync_pipe[1], &c, 1) == 1);
2489+
close(sync_pipe[1]);
2490+
2491+
// Sleep waiting for the signal. Use a timeout to avoid hanging the test.
2492+
sleep(10); // NOLINT(runtime/sleep): sleep() alone is async-signal-safe.
2493+
// If we get here, SIGHUP was not received.
2494+
_exit(1);
2495+
}
2496+
ASSERT_GT(child, 0);
2497+
close(sync_pipe[1]); // Close write end in parent.
2498+
2499+
// Wait for the child to finish setting up its session and controlling
2500+
// terminal.
2501+
char c;
2502+
ASSERT_THAT(ReadFd(sync_pipe[0], &c, 1), SyscallSucceedsWithValue(1));
2503+
close(sync_pipe[0]);
2504+
2505+
// Close the master end. This should trigger SIGHUP to the child.
2506+
master_.reset();
2507+
2508+
// Wait for the child and verify it received SIGHUP (exited with 42).
2509+
int wstatus;
2510+
ASSERT_THAT(waitpid(child, &wstatus, 0), SyscallSucceedsWithValue(child));
2511+
ASSERT_TRUE(WIFEXITED(wstatus));
2512+
EXPECT_EQ(WEXITSTATUS(wstatus), 42);
2513+
}
2514+
24522515
} // namespace
24532516
} // namespace testing
24542517
} // namespace gvisor

0 commit comments

Comments
 (0)