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
This commit is contained in:
Todd C. Miller
2014-11-18 14:05:51 -07:00
parent 552dbe5253
commit 29039859b7
3 changed files with 22 additions and 21 deletions

View File

@@ -119,6 +119,8 @@ static int fork_cmnd(struct command_details *details, int sv[2])
#else #else
sa.sa_handler = handler; sa.sa_handler = handler;
#endif #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) if (sudo_sigaction(SIGCONT, &sa, NULL) != 0)
sudo_warn(U_("unable to set handler for signal %d"), SIGCONT); sudo_warn(U_("unable to set handler for signal %d"), SIGCONT);
#ifdef SA_SIGINFO #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) if (sudo_sigaction(SIGTSTP, &sa, NULL) != 0)
sudo_warn(U_("unable to set handler for signal %d"), SIGTSTP); 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(); cmnd_pid = sudo_debug_fork();
switch (cmnd_pid) { switch (cmnd_pid) {
case -1: case -1:
@@ -406,7 +401,7 @@ sudo_execute(struct command_details *details, struct command_status *cstat)
sudo_fatal(U_("unable to create sockets")); 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. * We block all other signals while running the signal handler.
* Note: HP-UX select() will not be interrupted if SA_RESTART set. * 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); sudo_warn(U_("unable to set handler for signal %d"), SIGTERM);
if (sudo_sigaction(SIGALRM, &sa, NULL) != 0) if (sudo_sigaction(SIGALRM, &sa, NULL) != 0)
sudo_warn(U_("unable to set handler for signal %d"), SIGALRM); 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) if (sudo_sigaction(SIGPIPE, &sa, NULL) != 0)
sudo_warn(U_("unable to set handler for signal %d"), SIGPIPE); sudo_warn(U_("unable to set handler for signal %d"), SIGPIPE);
if (sudo_sigaction(SIGUSR1, &sa, NULL) != 0) 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) if (sudo_sigaction(SIGQUIT, &sa, NULL) != 0)
sudo_warn(U_("unable to set handler for signal %d"), SIGQUIT); 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 * Child will run the command in the pty, parent will pass data
* to and from pty. * to and from pty.

View File

@@ -816,6 +816,8 @@ fork_pty(struct command_details *details, int sv[], sigset_t *omask)
#else #else
sa.sa_handler = handler; sa.sa_handler = handler;
#endif #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) if (sudo_sigaction(SIGTSTP, &sa, NULL) != 0)
sudo_warn(U_("unable to set handler for signal %d"), SIGTSTP); 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 * 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. * race between exec of the command and receipt of a fatal signal from it.

View File

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