From: Todd C. Miller <Todd.Miller@courtesan.com>
Date: Thu, 20 Apr 2017 22:12:53 +0000 (-0600)
Subject: rewrite errpipe callbacks
X-Git-Tag: SUDO_1_8_20^2~39
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c60259bd637fa1186d2d5279bdb61c229d27fdb7;p=sudo

rewrite errpipe callbacks
---

diff --git a/src/exec_monitor.c b/src/exec_monitor.c
index b006d767f..15e6ff893 100644
--- a/src/exec_monitor.c
+++ b/src/exec_monitor.c
@@ -301,46 +301,44 @@ static void
 mon_errpipe_cb(int fd, int what, void *v)
 {
     struct monitor_closure *mc = v;
-    ssize_t n;
+    ssize_t nread;
     int errval;
     debug_decl(mon_errpipe_cb, SUDO_DEBUG_EXEC);
 
-    /* read errno from child or EOF when command is executed. */
-    n = read(fd, &errval, sizeof(errval));
-    switch (n) {
+    /*
+     * Read errno from child or EOF when command is executed.
+     * Note that the error pipe is *blocking*.
+     */
+    do {
+	nread = read(fd, &errval, sizeof(errval));
+    } while (nread == -1 && errno == EINTR);
+
+    switch (nread) {
     case -1:
-	switch (errno) {
-	case EINTR:
-	    /* got a signal, restart loop to service it. */
-	    sudo_ev_loopcontinue(mc->evbase);
-	    break;
-	case EAGAIN:
-	    /* not ready after all... */
-	    break;
-	default:
-	    sudo_debug_printf(SUDO_DEBUG_ERROR,
-		"failed to read error pipe: %s", strerror(errno));
-	    mc->cstat->type = CMD_ERRNO;
-	    mc->cstat->val = errno;
+	if (errno != EAGAIN) {
+	    if (mc->cstat->val == CMD_INVALID) {
+		/* XXX - need a way to distinguish non-exec error. */
+		mc->cstat->type = CMD_ERRNO;
+		mc->cstat->val = errno;
+	    }
+	    sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
+		"%s: failed to read error pipe", __func__);
 	    sudo_ev_loopbreak(mc->evbase);
-	    break;
 	}
 	break;
-    case 0:
-	/*
-	 * We get EOF when the command is executed and the other
-	 * end of the error pipe is closed.  Just remove the event.
-	 */
-	sudo_debug_printf(SUDO_DEBUG_INFO, "EOF on error pipe, removing event");
-	sudo_ev_del(mc->evbase, mc->errpipe_event);
-	break;
     default:
-	/* Errno value when child is unable to execute command. */
-	sudo_debug_printf(SUDO_DEBUG_INFO, "errno from child: %s",
-	    strerror(errval));
-	mc->cstat->type = CMD_ERRNO;
-	mc->cstat->val = errval;
+	if (nread == 0) {
+	    /* The error pipe closes when the command is executed. */
+	    sudo_debug_printf(SUDO_DEBUG_INFO, "EOF on error pipe");
+	} else {
+	    /* Errno value when child is unable to execute command. */
+	    sudo_debug_printf(SUDO_DEBUG_INFO, "errno from child: %s",
+		strerror(errval));
+	    mc->cstat->type = CMD_ERRNO;
+	    mc->cstat->val = errval;
+	}
 	sudo_ev_del(mc->evbase, mc->errpipe_event);
+	close(fd);
 	break;
     }
     debug_return;
diff --git a/src/exec_nopty.c b/src/exec_nopty.c
index b88d4a57d..dfb0f4d23 100644
--- a/src/exec_nopty.c
+++ b/src/exec_nopty.c
@@ -56,46 +56,44 @@ static void
 errpipe_cb(int fd, int what, void *v)
 {
     struct exec_closure_nopty *ec = v;
-    ssize_t n;
+    ssize_t nread;
     int errval;
-    debug_decl(errpipe_cb, SUDO_DEBUG_EXEC)
+    debug_decl(errpipe_cb, SUDO_DEBUG_EXEC);
+
+    /*
+     * Read errno from child or EOF when command is executed.
+     * Note that the error pipe is *blocking*.
+     */
+    do {
+	nread = read(fd, &errval, sizeof(errval));
+    } while (nread == -1 && errno == EINTR);
 
-    /* read errno from child or EOF when command is executed. */
-    n = read(fd, &errval, sizeof(errval));
-    switch (n) {
+    switch (nread) {
     case -1:
-	switch (errno) {
-	case EINTR:
-	    /* got a signal, restart loop to service it. */
-	    sudo_ev_loopcontinue(ec->evbase);
-	    break;
-	case EAGAIN:
-	    /* not ready after all... */
-	    break;
-	default:
-	    sudo_debug_printf(SUDO_DEBUG_ERROR,
-		"failed to read error pipe: %s", strerror(errno));
-	    ec->cstat->type = CMD_ERRNO;
-	    ec->cstat->val = errno;
+	if (errno != EAGAIN) {
+	    if (ec->cstat->val == CMD_INVALID) {
+		/* XXX - need a way to distinguish non-exec error. */
+		ec->cstat->type = CMD_ERRNO;
+		ec->cstat->val = errno;
+	    }
+	    sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_ERRNO,
+		"%s: failed to read error pipe", __func__);
 	    sudo_ev_loopbreak(ec->evbase);
-	    break;
 	}
 	break;
-    case 0:
-	/*
-	 * We get EOF when the command is executed and the other
-	 * end of the error pipe is closed.  Just remove the event.
-	 */
-	sudo_debug_printf(SUDO_DEBUG_INFO, "EOF on error pipe, removing event");
-	sudo_ev_del(ec->evbase, ec->errpipe_event);
-	break;
     default:
-	/* Errno value when child is unable to execute command. */
-	sudo_debug_printf(SUDO_DEBUG_INFO, "errno from child: %s",
-	    strerror(errval));
-	ec->cstat->type = CMD_ERRNO;
-	ec->cstat->val = errval;
+	if (nread == 0) {
+	    /* The error pipe closes when the command is executed. */
+	    sudo_debug_printf(SUDO_DEBUG_INFO, "EOF on error pipe");
+	} else {
+	    /* Errno value when child is unable to execute command. */
+	    sudo_debug_printf(SUDO_DEBUG_INFO, "errno from child: %s",
+		strerror(errval));
+	    ec->cstat->type = CMD_ERRNO;
+	    ec->cstat->val = errval;
+	}
 	sudo_ev_del(ec->evbase, ec->errpipe_event);
+	close(fd);
 	break;
     }
     debug_return;