Defer registration of the SIGCHLD handler until just before we exec

the command.  Fixes a problem where pam_gnome_keyring installs its
own SIGCHLD handler and may not restore the original one.  As a
result, we now have to explicitly wait for the askpass helper to
finish.  Bug #657

diff --git a/src/exec.c b/src/exec.c
index 7e993b3ef90bd4a448b1bae5ce649f2405ae0527..d29000538e7f7f14d8a49aded874c928fea53b0c 100644 (file)
--- a/src/exec.c
+++ b/src/exec.c
@@ -119,6 +119,8 @@ static int fork_cmnd(struct command_details *details, int sv[2])
 #else
     sa.sa_handler = handler;
 #endif
+    if (sudo_sigaction(SIGCHLD, &sa, NULL) != 0)
+       sudo_warn(U_("unable to set handler for signal %d"), SIGCHLD);
     if (sudo_sigaction(SIGCONT, &sa, NULL) != 0)
        sudo_warn(U_("unable to set handler for signal %d"), SIGCONT);
 #ifdef SA_SIGINFO
@@ -127,13 +129,6 @@ static int fork_cmnd(struct command_details *details, int sv[2])
     if (sudo_sigaction(SIGTSTP, &sa, NULL) != 0)
        sudo_warn(U_("unable to set handler for signal %d"), SIGTSTP);
 
-    /*
-     * The policy plugin's session init must be run before we fork
-     * or certain pam modules won't be able to track their state.
-     */
-    if (policy_init_session(details) != true)
-       sudo_fatalx(U_("policy plugin failed session initialization"));
-
     cmnd_pid = sudo_debug_fork();
     switch (cmnd_pid) {
     case -1:
@@ -406,7 +401,7 @@ sudo_execute(struct command_details *details, struct command_status *cstat)
        sudo_fatal(U_("unable to create sockets"));
 
     /*
-     * Signals to forward to the child process (excluding SIGALRM and SIGCHLD).
+     * Signals to forward to the child process (excluding SIGALRM).
      * We block all other signals while running the signal handler.
      * Note: HP-UX select() will not be interrupted if SA_RESTART set.
      */
@@ -423,8 +418,6 @@ sudo_execute(struct command_details *details, struct command_status *cstat)
        sudo_warn(U_("unable to set handler for signal %d"), SIGTERM);
     if (sudo_sigaction(SIGALRM, &sa, NULL) != 0)
        sudo_warn(U_("unable to set handler for signal %d"), SIGALRM);
-    if (sudo_sigaction(SIGCHLD, &sa, NULL) != 0)
-       sudo_warn(U_("unable to set handler for signal %d"), SIGCHLD);
     if (sudo_sigaction(SIGPIPE, &sa, NULL) != 0)
        sudo_warn(U_("unable to set handler for signal %d"), SIGPIPE);
     if (sudo_sigaction(SIGUSR1, &sa, NULL) != 0)
@@ -456,6 +449,13 @@ sudo_execute(struct command_details *details, struct command_status *cstat)
     if (sudo_sigaction(SIGQUIT, &sa, NULL) != 0)
        sudo_warn(U_("unable to set handler for signal %d"), SIGQUIT);
 
+    /*
+     * The policy plugin's session init must be run before we fork
+     * or certain pam modules won't be able to track their state.
+     */
+    if (policy_init_session(details) != true)
+       sudo_fatalx(U_("policy plugin failed session initialization"));
+
     /*
      * Child will run the command in the pty, parent will pass data
      * to and from pty.

diff --git a/src/exec_pty.c b/src/exec_pty.c
index a31a02991fde46ec3d727a240b6654242f670e51..17b9340e0fdd2ffec133f35609b264e8376e9cbc 100644 (file)
--- a/src/exec_pty.c
+++ b/src/exec_pty.c
@@ -816,6 +816,8 @@ fork_pty(struct command_details *details, int sv[], sigset_t *omask)
 #else
     sa.sa_handler = handler;
 #endif
+    if (sudo_sigaction(SIGCHLD, &sa, NULL) != 0)
+       sudo_warn(U_("unable to set handler for signal %d"), SIGCHLD);
     if (sudo_sigaction(SIGTSTP, &sa, NULL) != 0)
        sudo_warn(U_("unable to set handler for signal %d"), SIGTSTP);
 
@@ -837,13 +839,6 @@ fork_pty(struct command_details *details, int sv[], sigset_t *omask)
        }
     }
 
-    /*
-     * The policy plugin's session init must be run before we fork
-     * or certain pam modules won't be able to track their state.
-     */
-    if (policy_init_session(details) != true)
-       sudo_fatalx(U_("policy plugin failed session initialization"));
-
     /*
      * Block some signals until cmnd_pid is set in the parent to avoid a
      * race between exec of the command and receipt of a fatal signal from it.

diff --git a/src/tgetpass.c b/src/tgetpass.c
index ba0ceef38dfcd11c9414f0d8fc6ccb40319553b1..ddd8638f5604daebc377b2d737d795d58d26269c 100644 (file)
--- a/src/tgetpass.c
+++ b/src/tgetpass.c
@@ -26,6 +26,7 @@
 #include <config.h>
 
 #include <sys/types.h>
+#include <sys/wait.h>
 #include <stdio.h>
 #ifdef STDC_HEADERS
 # include <stdlib.h>
@@ -210,17 +211,18 @@ sudo_askpass(const char *askpass, const char *prompt)
 {
     static char buf[SUDO_CONV_REPL_MAX + 1], *pass;
     sigaction_t sa, saved_sa_pipe;
-    int pfd[2];
-    pid_t pid;
+    int pfd[2], status;
+    pid_t child;
     debug_decl(sudo_askpass, SUDO_DEBUG_CONV, sudo_debug_instance)
 
     if (pipe(pfd) == -1)
        sudo_fatal(U_("unable to create pipe"));
 
-    if ((pid = fork()) == -1)
+    child = sudo_debug_fork();
+    if (child == -1)
        sudo_fatal(U_("unable to fork"));
 
-    if (pid == 0) {
+    if (child == 0) {
        /* child, point stdout to output side of the pipe and exec askpass */
        if (dup2(pfd[1], STDOUT_FILENO) == -1) {
            sudo_warn("dup2");
@@ -255,6 +257,10 @@ sudo_askpass(const char *askpass, const char *prompt)
     (void) close(pfd[0]);
     (void) sigaction(SIGPIPE, &saved_sa_pipe, NULL);
 
+    /* Wait for child to exit. */
+    while (waitpid(child, &status, 0) == -1 && errno == EINTR)
+       continue;
+
     if (pass == NULL)
        errno = EINTR;  /* make cancel button simulate ^C */