Merge pull request #12855 from tanyifeng:pty-sighup-on-master-close

PiperOrigin-RevId: 900446775

Author: gvisor-bot

pkg/sentry/fsimpl/devpts/devpts.go

@@ -286,6 +286,10 @@ func (i *rootInode) allocateTerminal(ctx context.Context, creds *auth.Credential
 
 // masterClose is called when the master end of t is closed.
 func (i *rootInode) masterClose(ctx context.Context, t *Terminal) {
+	// When the master is closed, hang up the slave (replica) side.
+	// This corresponds to Linux's pty_close() calling tty_vhangup(tty->link).
+	t.replicaKTTY.Hangup(ctx)
+
 	i.mu.Lock()
 	defer i.mu.Unlock()
 

pkg/sentry/kernel/tty.go

@@ -158,3 +158,22 @@ func (tty *TTY) CheckChange(ctx context.Context, sig linux.Signal) error {
 	_ = pg.SendSignal(SignalInfoPriv(sig))
 	return linuxerr.ERESTARTSYS
 }
+
+// Hangup releases the controlling terminal and sends SIGHUP/SIGCONT to
+// the foreground process group. This is called on the replica (slave) TTY
+// when the PTY master is closed, corresponding to Linux's pty_close()
+// calling tty_vhangup(tty->link).
+func (tty *TTY) Hangup(ctx context.Context) {
+	tty.mu.Lock()
+	tg := tty.tg
+	tty.mu.Unlock()
+
+	if tg == nil {
+		// This TTY is not a controlling terminal.
+		return
+	}
+
+	// Reuse the existing ReleaseControllingTTY logic which handles
+	// sending SIGHUP/SIGCONT and clearing the controlling terminal.
+	_ = tg.ReleaseControllingTTY(tty)
+}

test/syscalls/linux/pty.cc

@@ -425,6 +425,12 @@ TEST(BasicPtyTest, OpenDevTTY) {
     // which will be opened by /dev/tty.
     setsid();
 
+    // Ignore SIGHUP: when the master fd is closed during cleanup, the
+    // kernel sends SIGHUP to the foreground process group of the
+    // controlling terminal session. Since this child *is* the session leader,
+    // it would be killed by the default SIGHUP disposition before _exit(0).
+    TEST_PCHECK(signal(SIGHUP, SIG_IGN) != SIG_ERR);
+
     FileDescriptor master =
         TEST_CHECK_NO_ERRNO_AND_VALUE(Open("/dev/ptmx", O_RDWR));
 
@@ -2449,6 +2455,63 @@ TEST_F(PtyTest, SignalCharConsumedWhenISIGEnabled) {
   EXPECT_EQ(buf, 'a');
 }
 
+// When the PTY master is closed, SIGHUP should be sent to the foreground
+// process group of the session that has this PTY as its controlling terminal.
+// This matches Linux pty_close() -> tty_vhangup() behavior.
+TEST_F(JobControlTest, SIGHUPOnMasterClose) {
+  // Use a pipe to synchronize: the child signals readiness after setting up
+  // its session and controlling terminal.
+  int sync_pipe[2];
+  ASSERT_THAT(pipe(sync_pipe), SyscallSucceeds());
+
+  pid_t child = fork();
+  if (child == 0) {
+    close(sync_pipe[0]);  // Close read end in child.
+
+    // Close the inherited master fd so that the parent holds the only
+    // reference. pty_close/tty_vhangup fires only when the last master
+    // fd is closed.
+    close(master_.release());
+
+    // Create new session and set the replica as controlling terminal.
+    TEST_PCHECK(setsid() >= 0);
+    TEST_PCHECK(ioctl(replica_.get(), TIOCSCTTY, 0) >= 0);
+
+    // Install a SIGHUP handler that exits with a known status.
+    struct sigaction sa = {};
+    sa.sa_handler = [](int) { _exit(42); };
+    sigemptyset(&sa.sa_mask);
+    TEST_PCHECK(sigaction(SIGHUP, &sa, nullptr) >= 0);
+
+    // Notify the parent that setup is complete.
+    char c = 'r';
+    TEST_PCHECK(WriteFd(sync_pipe[1], &c, 1) == 1);
+    close(sync_pipe[1]);
+
+    // Sleep waiting for the signal. Use a timeout to avoid hanging the test.
+    sleep(10);  // NOLINT(runtime/sleep): sleep() alone is async-signal-safe.
+    // If we get here, SIGHUP was not received.
+    _exit(1);
+  }
+  ASSERT_GT(child, 0);
+  close(sync_pipe[1]);  // Close write end in parent.
+
+  // Wait for the child to finish setting up its session and controlling
+  // terminal.
+  char c;
+  ASSERT_THAT(ReadFd(sync_pipe[0], &c, 1), SyscallSucceedsWithValue(1));
+  close(sync_pipe[0]);
+
+  // Close the master end. This should trigger SIGHUP to the child.
+  master_.reset();
+
+  // Wait for the child and verify it received SIGHUP (exited with 42).
+  int wstatus;
+  ASSERT_THAT(waitpid(child, &wstatus, 0), SyscallSucceedsWithValue(child));
+  ASSERT_TRUE(WIFEXITED(wstatus));
+  EXPECT_EQ(WEXITSTATUS(wstatus), 42);
+}
+
 }  // namespace
 }  // namespace testing
 }  // namespace gvisor