Move SIGCHLD handling into handle_sigchld() functions and move the

remaining bits of dispatch_signal() into signal_pipe_cb()

diff --git a/src/exec_monitor.c b/src/exec_monitor.c
index 0e6e74939b6769dd8b49c5bd35b6dcb10c21d3fd..5650c562cdd42373d2a438f8eb669284c289db32 100644 (file)
--- a/src/exec_monitor.c
+++ b/src/exec_monitor.c
@@ -195,12 +195,12 @@ send_status(int fd, struct command_status *cstat)
  * Otherwise, cstat is filled in but not sent.
  */
 static void
-handle_sigchld(int backchannel, struct command_status *cstat)
+mon_handle_sigchld(int backchannel, struct command_status *cstat)
 {
     char signame[SIG2STR_MAX];
     int status;
     pid_t pid;
-    debug_decl(handle_sigchld, SUDO_DEBUG_EXEC);
+    debug_decl(mon_handle_sigchld, SUDO_DEBUG_EXEC);
 
     /* Read command status. */
     do {
@@ -283,7 +283,7 @@ mon_signal_pipe_cb(int fd, int what, void *v)
         * directly to the command.
         */
        if (signo == SIGCHLD) {
-           handle_sigchld(mc->backchannel, mc->cstat);
+           mon_handle_sigchld(mc->backchannel, mc->cstat);
            if (cmnd_pid == -1) {
                /* Remove all but the errpipe event. */
                sudo_ev_del(mc->evbase, mc->backchannel_event);

diff --git a/src/exec_nopty.c b/src/exec_nopty.c
index 0fb72f94e643892b8565415f9beb53235612bead..3380d1eef228e26179915b6f2b4e078f09071869 100644 (file)
--- a/src/exec_nopty.c
+++ b/src/exec_nopty.c
@@ -301,127 +301,127 @@ exec_nopty(struct command_details *details, struct command_status *cstat)
 }
 
 /*
- * Forward a signal to the command (non-pty version) or handle
- * changes to the command's status (SIGCHLD).
- * XXX - separate SIGCHLD code?
+ * Wait for command status after receiving SIGCHLD.
+ * If the command exits, fill in cstat and stop the event loop.
+ * If the command stops, save the tty pgrp, suspend sudo, then restore
+ * the tty pgrp when sudo resumes.
  */
 static void
-dispatch_signal(struct exec_closure_nopty *ec, int signo, char *signame)
+handle_sigchld_nopty(struct exec_closure_nopty *ec)
 {
-    debug_decl(dispatch_signal, SUDO_DEBUG_EXEC)
-
-    sudo_debug_printf(SUDO_DEBUG_INFO,
-       "%s: evbase %p, child: %d, signo %s(%d), cstat %p",
-       __func__, ec->evbase, (int)ec->child, signame, signo, ec->cstat);
+    pid_t pid;
+    int status;
+    char signame[SIG2STR_MAX];
+    debug_decl(handle_sigchld_nopty, SUDO_DEBUG_EXEC)
 
-    if (ec->child == -1)
-       goto done;
+    /* Read command status. */
+    do {
+       pid = waitpid(ec->child, &status, WUNTRACED|WNOHANG);
+    } while (pid == -1 && errno == EINTR);
+    switch (pid) {
+    case 0:
+       /* waitpid() will return 0 for SIGCONT, which we don't care about */
+       debug_return;
+    case -1:
+       sudo_warn(U_("%s: %s"), __func__, "waitpid");
+       debug_return;
+    }
 
-    if (signo == SIGCHLD) {
-       pid_t pid;
-       int status;
+    if (WIFSTOPPED(status)) {
        /*
-        * The command stopped or exited.
+        * Save the controlling terminal's process group so we can restore it
+        * after we resume, if needed.  Most well-behaved shells change the
+        * pgrp back to its original value before suspending so we must
+        * not try to restore in that case, lest we race with the child upon
+        * resume, potentially stopping sudo with SIGTTOU while the command
+        * continues to run.
         */
-       do {
-           pid = waitpid(ec->child, &status, WUNTRACED|WNOHANG);
-       } while (pid == -1 && errno == EINTR);
-       if (pid == ec->child) {
-           if (WIFSTOPPED(status)) {
-               /*
-                * Save the controlling terminal's process group
-                * so we can restore it after we resume, if needed.
-                * Most well-behaved shells change the pgrp back to
-                * its original value before suspending so we must
-                * not try to restore in that case, lest we race with
-                * the child upon resume, potentially stopping sudo
-                * with SIGTTOU while the command continues to run.
-                */
-               sigaction_t sa, osa;
-               pid_t saved_pgrp = -1;
-               int signo = WSTOPSIG(status);
-               int fd = open(_PATH_TTY, O_RDWR);
-               if (fd != -1) {
-                   saved_pgrp = tcgetpgrp(fd);
-                   if (saved_pgrp == -1) {
-                       close(fd);
-                       fd = -1;
-                   }
-               }
-               if (saved_pgrp != -1) {
-                   /*
-                    * Child was stopped trying to access controlling
-                    * terminal.  If the child has a different pgrp
-                    * and we own the controlling terminal, give it
-                    * to the child's pgrp and let it continue.
-                    */
-                   if (signo == SIGTTOU || signo == SIGTTIN) {
-                       if (saved_pgrp == ppgrp) {
-                           pid_t child_pgrp = getpgid(ec->child);
-                           if (child_pgrp != ppgrp) {
-                               if (tcsetpgrp(fd, child_pgrp) == 0) {
-                                   if (killpg(child_pgrp, SIGCONT) != 0) {
-                                       sudo_warn("kill(%d, SIGCONT)",
-                                           (int)child_pgrp);
-                                   }
-                                   close(fd);
-                                   goto done;
-                               }
+       sigaction_t sa, osa;
+       pid_t saved_pgrp = -1;
+       int fd, signo = WSTOPSIG(status);
+
+       if (sig2str(signo, signame) == -1)
+           snprintf(signame, sizeof(signame), "%d", signo);
+       sudo_debug_printf(SUDO_DEBUG_INFO, "%s: command (%d) stopped, SIG%s",
+           __func__, (int)ec->child, signame);
+
+       fd = open(_PATH_TTY, O_RDWR);
+       if (fd != -1) {
+           saved_pgrp = tcgetpgrp(fd);
+           if (saved_pgrp == -1) {
+               close(fd);
+               fd = -1;
+           }
+       }
+       if (saved_pgrp != -1) {
+           /*
+            * Child was stopped trying to access the controlling terminal.
+            * If the child has a different pgrp and we own the controlling
+            * terminal, give it to the child's pgrp and let it continue.
+            */
+           if (signo == SIGTTOU || signo == SIGTTIN) {
+               if (saved_pgrp == ppgrp) {
+                   pid_t child_pgrp = getpgid(ec->child);
+                   if (child_pgrp != ppgrp) {
+                       if (tcsetpgrp_nobg(fd, child_pgrp) == 0) {
+                           if (killpg(child_pgrp, SIGCONT) != 0) {
+                               sudo_warn("kill(%d, SIGCONT)",
+                                   (int)child_pgrp);
                            }
+                           close(fd);
+                           goto done;
                        }
                    }
                }
-               if (signo == SIGTSTP) {
-                   memset(&sa, 0, sizeof(sa));
-                   sigemptyset(&sa.sa_mask);
-                   sa.sa_flags = SA_RESTART;
-                   sa.sa_handler = SIG_DFL;
-                   if (sudo_sigaction(SIGTSTP, &sa, &osa) != 0) {
-                       sudo_warn(U_("unable to set handler for signal %d"),
-                           SIGTSTP);
-                   }
-               }
-               if (kill(getpid(), signo) != 0)
-                   sudo_warn("kill(%d, SIG%s)", (int)getpid(), signame);
-               if (signo == SIGTSTP) {
-                   if (sudo_sigaction(SIGTSTP, &osa, NULL) != 0) {
-                       sudo_warn(U_("unable to restore handler for signal %d"),
-                           SIGTSTP);
-                   }
-               }
-               if (saved_pgrp != -1) {
-                   /*
-                    * Restore foreground process group, if different.
-                    * Otherwise, we cannot resume some shells (pdksh).
-                    *
-                    * It is possible that we are no longer the foreground
-                    * process so use tcsetpgrp_nobg() to avoid sudo
-                    * receiving SIGTTOU.
-                    */
-                   if (saved_pgrp != ppgrp)
-                       tcsetpgrp_nobg(fd, saved_pgrp);
-                   close(fd);
-               }
-           } else {
-               /* Child has exited or been killed, we are done. */
-               ec->child = -1;
-               /* Don't overwrite execve() failure with child exit status. */
-               if (ec->cstat->type != CMD_ERRNO) {
-                   ec->cstat->type = CMD_WSTATUS;
-                   ec->cstat->val = status;
-               }
-               sudo_ev_del(ec->evbase, ec->signal_event);
-               sudo_ev_loopexit(ec->evbase);
-               goto done;
            }
        }
+       if (signo == SIGTSTP) {
+           memset(&sa, 0, sizeof(sa));
+           sigemptyset(&sa.sa_mask);
+           sa.sa_flags = SA_RESTART;
+           sa.sa_handler = SIG_DFL;
+           if (sudo_sigaction(SIGTSTP, &sa, &osa) != 0) {
+               sudo_warn(U_("unable to set handler for signal %d"),
+                   SIGTSTP);
+           }
+       }
+       if (kill(getpid(), signo) != 0)
+           sudo_warn("kill(%d, SIG%s)", (int)getpid(), signame);
+       if (signo == SIGTSTP) {
+           if (sudo_sigaction(SIGTSTP, &osa, NULL) != 0) {
+               sudo_warn(U_("unable to restore handler for signal %d"),
+                   SIGTSTP);
+           }
+       }
+       if (saved_pgrp != -1) {
+           /*
+            * On resume, restore foreground process group, if different.
+            * Otherwise, we cannot resume some shells (pdksh).
+            *
+            * It is possible that we are no longer the foreground process so
+            * use tcsetpgrp_nobg() to prevent sudo from receiving SIGTTOU.
+            */
+           if (saved_pgrp != ppgrp)
+               tcsetpgrp_nobg(fd, saved_pgrp);
+           close(fd);
+       }
     } else {
-       /* Send signal to child. */
-       if (signo == SIGALRM) {
-           terminate_command(ec->child, false);
-       } else if (kill(ec->child, signo) != 0) {
-           sudo_warn("kill(%d, SIG%s)", (int)ec->child, signame);
+       /* Child has exited or been killed, we are done. */
+       if (WIFSIGNALED(status)) {
+           if (sig2str(WTERMSIG(status), signame) == -1)
+               snprintf(signame, sizeof(signame), "%d", WTERMSIG(status));
+           sudo_debug_printf(SUDO_DEBUG_INFO, "%s: command (%d) killed, SIG%s",
+               __func__, (int)ec->child, signame);
+       } else {
+           sudo_debug_printf(SUDO_DEBUG_INFO, "%s: command (%d) exited: %d",
+               __func__, (int)ec->child, WEXITSTATUS(status));
        }
+       /* Don't overwrite execve() failure with child exit status. */
+       if (ec->cstat->type != CMD_ERRNO) {
+           ec->cstat->type = CMD_WSTATUS;
+           ec->cstat->val = status;
+       }
+       ec->child = -1;
     }
 done:
     debug_return;
@@ -459,9 +459,25 @@ signal_pipe_cb(int fd, int what, void *v)
        }
        if (sig2str(signo, signame) == -1)
            snprintf(signame, sizeof(signame), "%d", signo);
-       sudo_debug_printf(SUDO_DEBUG_DIAG, "received SIG%s", signame);
-       /* XXX - deliver vs. SIGCHLD? */
-       dispatch_signal(ec, signo, signame);
+       sudo_debug_printf(SUDO_DEBUG_DIAG,
+           "%s: evbase %p, child: %d, signo %s(%d), cstat %p",
+           __func__, ec->evbase, (int)ec->child, signame, signo, ec->cstat);
+
+       if (signo == SIGCHLD) {
+           handle_sigchld_nopty(ec);
+           if (ec->child == -1) {
+               /* Command exited or was killed, exit event loop. */
+               sudo_ev_del(ec->evbase, ec->signal_event);
+               sudo_ev_loopexit(ec->evbase);
+           }
+       } else if (ec->child != -1) {
+           /* Send signal to child. */
+           if (signo == SIGALRM) {
+               terminate_command(ec->child, false);
+           } else if (kill(ec->child, signo) != 0) {
+               sudo_warn("kill(%d, SIG%s)", (int)ec->child, signame);
+           }
+       }
     } while (ec->child != -1);
     debug_return;
 }

diff --git a/src/exec_pty.c b/src/exec_pty.c
index dffdc59bed917fc98282f7efb0e46b1d3e551fd0..92e08e1b70a11323a519a83bf8dd8a302775e443 100644 (file)
--- a/src/exec_pty.c
+++ b/src/exec_pty.c
@@ -954,63 +954,49 @@ backchannel_cb(int fd, int what, void *v)
 }
 
 /*
- * Forward a signal to the monitor (pty version) or handle
- * changes to the monitors's status (SIGCHLD).
+ * Handle changes to the monitors's status (SIGCHLD).
  */
 static void
-dispatch_signal_pty(struct exec_closure_pty *ec, int signo, char *signame)
+handle_sigchld_pty(struct exec_closure_pty *ec)
 {
-    debug_decl(dispatch_signal_pty, SUDO_DEBUG_EXEC)
+    int n, status;
+    pid_t pid;
+    debug_decl(handle_sigchld_pty, SUDO_DEBUG_EXEC)
 
-    sudo_debug_printf(SUDO_DEBUG_INFO,
-       "%s: evbase %p, child: %d, signo %s(%d), cstat %p",
-       __func__, ec->evbase, (int)ec->child, signame, signo, ec->cstat);
-
-    if (ec->child == -1)
-       goto done;
-
-    if (signo == SIGCHLD) {
-       int n, status;
-       pid_t pid;
+    /*
+     * Monitor process was signaled; wait for it as needed.
+     */
+    do {
+       pid = waitpid(ec->child, &status, WUNTRACED|WNOHANG);
+    } while (pid == -1 && errno == EINTR);
+    if (pid == ec->child) {
        /*
-        * Monitor process was signaled; wait for it as needed.
+        * If the monitor dies we get notified via backchannel_cb().
+        * If it was stopped, we should stop too (the command keeps
+        * running in its pty) and continue it when we come back.
         */
-       do {
-           pid = waitpid(ec->child, &status, WUNTRACED|WNOHANG);
-       } while (pid == -1 && errno == EINTR);
-       if (pid == ec->child) {
-           /*
-            * If the monitor dies we get notified via backchannel_cb().
-            * If it was stopped, we should stop too (the command keeps
-            * running in its pty) and continue it when we come back.
-            */
-           if (WIFSTOPPED(status)) {
-               sudo_debug_printf(SUDO_DEBUG_INFO,
-                   "monitor stopped, suspending sudo");
-               n = suspend_sudo(WSTOPSIG(status));
-               kill(pid, SIGCONT);
-               schedule_signal(ec, n);
-               /* Re-enable I/O events and restart event loop. */
-               add_io_events(ec->evbase);
-               sudo_ev_loopcontinue(ec->evbase);
-               goto done;
-           } else if (WIFSIGNALED(status)) {
-               sudo_debug_printf(SUDO_DEBUG_INFO,
-                   "monitor killed, signal %d", WTERMSIG(status));
-               ec->child = -1;
-           } else {
-               sudo_debug_printf(SUDO_DEBUG_INFO,
-                   "monitor exited, status %d", WEXITSTATUS(status));
-               ec->child = -1;
-           }
+       if (WIFSTOPPED(status)) {
+           sudo_debug_printf(SUDO_DEBUG_INFO,
+               "monitor stopped, suspending sudo");
+           n = suspend_sudo(WSTOPSIG(status));
+           kill(pid, SIGCONT);
+           schedule_signal(ec, n);
+           /* Re-enable I/O events and restart event loop. */
+           add_io_events(ec->evbase);
+           sudo_ev_loopcontinue(ec->evbase);
+       } else if (WIFSIGNALED(status)) {
+           char signame[SIG2STR_MAX];
+           if (sig2str(WTERMSIG(status), signame) == -1)
+               snprintf(signame, sizeof(signame), "%d", WTERMSIG(status));
+           sudo_debug_printf(SUDO_DEBUG_INFO, "%s: monitor (%d) killed, SIG%s",
+               __func__, (int)ec->child, signame);
+           ec->child = -1;
+       } else {
+           sudo_debug_printf(SUDO_DEBUG_INFO,
+               "%s: monitor exited, status %d", __func__, WEXITSTATUS(status));
+           ec->child = -1;
        }
-    } else {
-       /* Schedule signo to be forwared to the child. */
-       schedule_signal(ec, signo);
-       /* Restart event loop to service signal immediately. */
-       sudo_ev_loopcontinue(ec->evbase);
     }
-done:
     debug_return;
 }
 
@@ -1024,6 +1010,7 @@ signal_pipe_cb(int fd, int what, void *v)
     ssize_t nread;
     debug_decl(signal_pipe_cb, SUDO_DEBUG_EXEC)
 
+    /* Process received signals until the child dies or the pipe is empty. */
     do {
        /* read signal pipe */
        nread = read(fd, &signo, sizeof(signo));
@@ -1045,8 +1032,18 @@ signal_pipe_cb(int fd, int what, void *v)
        }
        if (sig2str(signo, signame) == -1)
            snprintf(signame, sizeof(signame), "%d", signo);
-       sudo_debug_printf(SUDO_DEBUG_DIAG, "received SIG%s", signame);
-       dispatch_signal_pty(ec, signo, signame);
+       sudo_debug_printf(SUDO_DEBUG_DIAG,
+           "%s: evbase %p, child: %d, signo %s(%d), cstat %p",
+           __func__, ec->evbase, (int)ec->child, signame, signo, ec->cstat);
+
+       if (signo == SIGCHLD) {
+           handle_sigchld_pty(ec);
+       } else if (ec->child != -1) {
+           /* Schedule signo to be forwared to the child. */
+           schedule_signal(ec, signo);
+           /* Restart event loop to service signal immediately. */
+           sudo_ev_loopcontinue(ec->evbase);
+       }
     } while (ec->child != -1);
     debug_return;
 }