From 62e4d29f8ee42aa520a68eb0241a53c7a2afe243 Mon Sep 17 00:00:00 2001
From: "Todd C. Miller" <Todd.Miller@sudo.ws>
Date: Sat, 18 May 2024 19:15:14 -0600
Subject: [PATCH] When revoking the pty, kill the foreground process from the
 parent sudo.

There's no need to send messages back and forth to the monitor
when the main process can just do it.  GitHub issue #367.
---
 src/exec_monitor.c | 45 ++-------------------------------------------
 src/exec_pty.c     | 43 ++++++++++++++++++++++++++++---------------
 src/sudo.h         |  1 -
 3 files changed, 30 insertions(+), 59 deletions(-)

diff --git a/src/exec_monitor.c b/src/exec_monitor.c
index 47d41604f..c5c4a14b6 100644
--- a/src/exec_monitor.c
+++ b/src/exec_monitor.c
@@ -310,41 +310,6 @@ mon_errsock_cb(int fd, int what, void *v)
     debug_return;
 }
 
-/*
- * Called when the user's terminal has gone away but before our pty is
- * actually revoked.  We simulate the effect of the session leader
- * (sudo) exiting by sending SIGHUP to the foreground process group.
- * The monitor process will not actually exit until the command exits.
- */
-static void
-mon_handle_revoke(int fd, pid_t pgrp, struct command_status *cstat)
-{
-    debug_decl(mon_handle_revoke, SUDO_DEBUG_EXEC);
-
-    /*
-     * Signal the foreground process group (or the command's process group
-     * if no pty).  We must do this before the pty is revoked by the main
-     * sudo process so we can determine the foreground process group.
-     * Otherwise, if the foreground process group is different from the
-     * command's process group, it will not be signaled.  This fixes a
-     * problem on Linux with, e.g. "sudo su" where su(1) blocks SIGHUP.
-     */
-    if (io_fds[SFD_FOLLOWER] != -1) {
-	const pid_t tcpgrp = tcgetpgrp(io_fds[SFD_FOLLOWER]);
-	if (tcpgrp != -1)
-	    pgrp = tcpgrp;
-    }
-    sudo_debug_printf(SUDO_DEBUG_NOTICE, "%s: killpg(%d, SIGHUP)",
-	__func__, (int)pgrp);
-    killpg(pgrp, SIGHUP);
-
-    /*
-     * Now that the foreground process as been signaled, send the
-     * parent CMD_REVOKE to close the pty leader, revoking the pty.
-     */
-    send_status(fd, cstat);
-}
-
 static void
 mon_backchannel_cb(int fd, int what, void *v)
 {
@@ -371,17 +336,11 @@ mon_backchannel_cb(int fd, int what, void *v)
 	mc->cstat->val = n ? EIO : ECONNRESET;
 	sudo_ev_loopbreak(mc->evbase);
     } else {
-	switch (cstmp.type) {
-	case CMD_REVOKE:
-	    mon_handle_revoke(fd, mc->cmnd_pid, &cstmp);
-	    break;
-	case CMD_SIGNO:
+	if (cstmp.type == CMD_SIGNO) {
 	    deliver_signal(mc, cstmp.val, true);
-	    break;
-	default:
+	} else {
 	    sudo_warnx(U_("unexpected reply type on backchannel: %d"),
 		cstmp.type);
-	    break;
 	}
     }
     debug_return;
diff --git a/src/exec_pty.c b/src/exec_pty.c
index 4dd5915ed..8a5b9446f 100644
--- a/src/exec_pty.c
+++ b/src/exec_pty.c
@@ -332,6 +332,32 @@ sigttin(int signo)
     got_sigttin = 1;
 }
 
+/*
+ * Close the leader fd to revoke the pty and signal the foreground pgrp.
+ * We send SIGHUP to the foreground process group (or the command's
+ * process group if no pty) after closing the leader fd.  We cannot
+ * just forward the SIGHUP we receive from the kernel since the
+ * command may not be the foreground process.  This fixes a problem
+ * on Linux with, e.g. "sudo su" where su(1) blocks SIGHUP.
+ */
+static void
+revoke_pty(struct exec_closure *ec)
+{
+    pid_t pgrp = ec->cmnd_pid;
+    debug_decl(revoke_pty, SUDO_DEBUG_EXEC);
+
+    sudo_debug_printf(SUDO_DEBUG_NOTICE, "user's tty revoked");
+    if (io_fds[SFD_LEADER] != -1) {
+	const pid_t tcpgrp = tcgetpgrp(io_fds[SFD_LEADER]);
+	if (tcpgrp != -1)
+	    pgrp = tcpgrp;
+	close(io_fds[SFD_LEADER]);
+    }
+    sudo_debug_printf(SUDO_DEBUG_NOTICE, "%s: killpg(%d, SIGHUP)",
+	__func__, (int)pgrp);
+    kill(pgrp, SIGHUP);
+}
+
 /*
  * Read an iobuf that is ready.
  */
@@ -390,8 +416,7 @@ read_callback(int fd, int what, void *v)
 		 */
 		const int wfd = sudo_ev_get_fd(iob->wevent);
 		if (wfd == io_fds[SFD_LEADER]) {
-		    sudo_debug_printf(SUDO_DEBUG_NOTICE, "user's tty revoked");
-		    send_command_status(iob->ec, CMD_REVOKE, 0);
+		    revoke_pty(iob->ec);
 		} else {
 		    safe_close(wfd);
 		}
@@ -478,8 +503,7 @@ write_callback(int fd, int what, void *v)
 		 */
 		const int rfd = sudo_ev_get_fd(iob->revent);
 		if (rfd == io_fds[SFD_LEADER]) {
-		    sudo_debug_printf(SUDO_DEBUG_NOTICE, "user's tty revoked");
-		    send_command_status(iob->ec, CMD_REVOKE, 0);
+		    revoke_pty(iob->ec);
 		} else {
 		    safe_close(rfd);
 		}
@@ -684,17 +708,6 @@ backchannel_cb(int fd, int what, void *v)
 	    sudo_ev_loopbreak(ec->evbase);
 	    *ec->cstat = cstat;
 	    break;
-	case CMD_REVOKE:
-	    if (io_fds[SFD_LEADER] != -1) {
-		/*
-		 * Monitor requests that we revoke the user's terminal.
-		 * This must happen after the monitor has signaled the
-		 * controlling terminal's process group.
-		 */
-		close(io_fds[SFD_LEADER]);
-		io_fds[SFD_LEADER] = -1;
-	    }
-	    break;
 	case CMD_PID:
 	    ec->cmnd_pid = cstat.val;
 	    sudo_debug_printf(SUDO_DEBUG_INFO, "executed %s, pid %d",
diff --git a/src/sudo.h b/src/sudo.h
index a0e092666..bc42827a5 100644
--- a/src/sudo.h
+++ b/src/sudo.h
@@ -224,7 +224,6 @@ struct command_status {
 #define CMD_WSTATUS	2
 #define CMD_SIGNO	3
 #define CMD_PID		4
-#define CMD_REVOKE	5
     int type;
     int val;
 };