From: Todd C. Miller <Todd.Miller@sudo.ws>
Date: Mon, 20 Aug 2018 16:04:15 +0000 (-0600)
Subject: Close the pty slave in the parent so that when the command and
X-Git-Tag: SUDO_1_8_25^2~52
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5cca421867408df3e4f2f7feb46c68d6203d3fd2;p=sudo

Close the pty slave in the parent so that when the command and
monitor exit, the pty gets recycled without our having to close
it directly.
---

diff --git a/src/exec_monitor.c b/src/exec_monitor.c
index 4b4a7f92e..b0a362f50 100644
--- a/src/exec_monitor.c
+++ b/src/exec_monitor.c
@@ -594,6 +594,15 @@ exec_monitor(struct command_details *details, sigset_t *oset,
     if (pipe2(errpipe, O_CLOEXEC) != 0)
 	sudo_fatal(U_("unable to create pipe"));
 
+    /*
+     * Before forking, wait for the main sudo process to tell us to go.
+     * Avoids race conditions when the command exits quickly.
+     */
+    while (recv(backchannel, &cstat, sizeof(cstat), MSG_WAITALL) == -1) {
+	if (errno != EINTR && errno != EAGAIN)
+	    sudo_fatal(U_("unable to receive message from parent"));
+    }
+
     mc.cmnd_pid = sudo_debug_fork();
     switch (mc.cmnd_pid) {
     case -1:
diff --git a/src/exec_pty.c b/src/exec_pty.c
index d3f96edaf..36b9e7d5d 100644
--- a/src/exec_pty.c
+++ b/src/exec_pty.c
@@ -744,23 +744,15 @@ io_buf_new(int rfd, int wfd,
     debug_return;
 }
 
+/*
+ * We already closed the slave pty so reads from the master will not block.
+ */
 static void
-pty_close(struct command_status *cstat)
+pty_finish(struct command_status *cstat)
 {
     struct io_buffer *iob;
     int n;
-    debug_decl(pty_close, SUDO_DEBUG_EXEC);
-
-#ifndef _AIX
-    /*
-     * Close the pty slave first so reads from the master don't block.
-     * On AIX as it will cause us to lose *our* controlling tty too!
-     */
-    if (io_fds[SFD_SLAVE] != -1) {
-	close(io_fds[SFD_SLAVE]);
-	io_fds[SFD_SLAVE] = -1;
-    }
-#endif
+    debug_decl(pty_finish, SUDO_DEBUG_EXEC);
 
     /* Flush any remaining output (the plugin already got it). */
     if (io_fds[SFD_USERTTY] != -1) {
@@ -790,14 +782,6 @@ pty_close(struct command_status *cstat)
     if (utmp_user != NULL)
 	utmp_logout(slavename, cstat->type == CMD_WSTATUS ? cstat->val : 0);
 
-#ifndef _AIX
-    /* Close pty master. */
-    if (io_fds[SFD_MASTER] != -1) {
-	close(io_fds[SFD_MASTER]);
-	io_fds[SFD_MASTER] = -1;
-    }
-#endif
-
     debug_return;
 }
 
@@ -1421,12 +1405,13 @@ exec_pty(struct command_details *details, struct command_status *cstat)
 	if (!sudo_term_copy(io_fds[SFD_USERTTY], io_fds[SFD_SLAVE])) {
             sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
                 "%s: unable to copy terminal settings to pty", __func__);
-	}
-
-	/* Start out in raw mode unless part of a pipeline or backgrounded. */
-	if (!pipeline && !ISSET(details->flags, CD_EXEC_BG)) {
-	    if (sudo_term_raw(io_fds[SFD_USERTTY], 0))
-		ttymode = TERM_RAW;
+	    foreground = false;
+	} else {
+	    /* Start in raw mode unless part of a pipeline or backgrounded. */
+	    if (!pipeline && !ISSET(details->flags, CD_EXEC_BG)) {
+		if (sudo_term_raw(io_fds[SFD_USERTTY], 0))
+		    ttymode = TERM_RAW;
+	    }
 	}
     }
 
@@ -1475,6 +1460,24 @@ exec_pty(struct command_details *details, struct command_status *cstat)
 	_exit(1);
     }
 
+    /*
+     * We close the pty slave so only the monitor and command have a
+     * reference to it.  This ensures that we can don't block reading
+     * from the master when the command and monitor have exited.
+     */
+    if (io_fds[SFD_SLAVE] != -1) {
+	close(io_fds[SFD_SLAVE]);
+	io_fds[SFD_SLAVE] = -1;
+    }
+
+    /* Tell the monitor to continue now that the slave is closed. */
+    cstat->type = CMD_SIGNO;
+    cstat->val = 0;
+    while (send(sv[0], cstat, sizeof(*cstat), 0) == -1) {
+	if (errno != EINTR && errno != EAGAIN)
+	    sudo_fatal(U_("unable to send message to monitor process"));
+    }
+
     /* Close the other end of the stdin/stdout/stderr pipes and socketpair. */
     if (io_pipe[STDIN_FILENO][0] != -1)
 	close(io_pipe[STDIN_FILENO][0]);
@@ -1523,7 +1526,7 @@ exec_pty(struct command_details *details, struct command_status *cstat)
     }
 
     /* Flush any remaining output, free I/O bufs and events, do logout. */
-    pty_close(cstat);
+    pty_finish(cstat);
 
     /* Free things up. */
     free_exec_closure_pty(&ec);