From: Todd C. Miller Date: Sat, 21 Jun 2008 00:34:47 +0000 (+0000) Subject: Change how the mailer is waited for. Instead of having a SIGCHLD X-Git-Tag: SUDO_1_7_0~103 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5756ddfbeb4b59ffae481b8c87c0d9f412b5db35;p=sudo Change how the mailer is waited for. Instead of having a SIGCHLD handler, use the double fork trick to orphan the child that opens the pipe to sendmail. Fixes a problem running su on some Linux distros. --- diff --git a/logging.c b/logging.c index d8b676cf1..4b29aa591 100644 --- a/logging.c +++ b/logging.c @@ -425,8 +425,8 @@ send_mail(line) { FILE *mail; char *p; - int pfd[2]; - pid_t pid; + int pfd[2], status; + pid_t pid, rv; sigaction_t sa, saved_sa_pipe; #ifndef NO_ROOT_MAILER static char *root_envp[] = { @@ -443,19 +443,58 @@ send_mail(line) if (!def_mailerpath || !def_mailto) return; + /* Fork a child so we can be asyncronous. */ + switch (pid = fork()) { + case -1: + /* Error. */ + error(1, "cannot fork"); + break; + case 0: + /* Child continues below. */ + break; + default: + /* Parent waits and returns. */ + do { +#ifdef sudo_waitpid + rv = sudo_waitpid(pid, &status, 0); +#else + rv = wait(&status); +#endif + } while (rv == -1 && errno == EINTR); + return; + } + + /* Fork again and orphan the grandchild so parent can continue. */ + switch (pid = fork()) { + case -1: + /* Error. */ + warning("cannot fork"); + _exit(1); + break; + case 0: + /* Grandchild continues below. */ + break; + default: + /* Orphan grandchild. */ + _exit(0); + } + /* Ignore SIGPIPE in case mailer exits prematurely (or is missing). */ sigemptyset(&sa.sa_mask); sa.sa_flags = 0; sa.sa_handler = SIG_IGN; (void) sigaction(SIGPIPE, &sa, &saved_sa_pipe); - if (pipe(pfd) == -1) - error(1, "cannot open pipe"); + if (pipe(pfd) == -1) { + warning("cannot open pipe"); + _exit(1); + } switch (pid = fork()) { case -1: /* Error. */ - error(1, "cannot fork"); + warning("cannot fork"); + _exit(1); break; case 0: { @@ -463,7 +502,7 @@ send_mail(line) char *mpath, *mflags; int i; - /* Child, set stdin to output side of the pipe */ + /* Great-grandchild, set stdin to output side of the pipe */ if (pfd[0] != STDIN_FILENO) { (void) dup2(pfd[0], STDIN_FILENO); (void) close(pfd[0]); @@ -533,7 +572,13 @@ send_mail(line) (void) fprintf(mail, "\n\n%s : %s : %s : %s\n\n", user_host, get_timestr(), user_name, line); fclose(mail); - + do { +#ifdef sudo_waitpid + rv = sudo_waitpid(pid, &status, 0); +#else + rv = wait(&status); +#endif + } while (rv == -1 && errno == EINTR); (void) sigaction(SIGPIPE, &saved_sa_pipe, NULL); } @@ -551,26 +596,6 @@ should_mail(status) (def_mail_no_perms && !ISSET(status, VALIDATE_OK))); } -/* - * SIGCHLD sig handler--wait for children as they die. - */ -RETSIGTYPE -reapchild(sig) - int sig; -{ - int status, serrno = errno; -#ifdef sudo_waitpid - pid_t pid; - - do { - pid = sudo_waitpid(-1, &status, WNOHANG); - } while (pid != 0 && (pid != -1 || errno == EINTR)); -#else - (void) wait(&status); -#endif - errno = serrno; -} - /* * Return an ascii string with the current date + time * Uses strftime() if available, else falls back to ctime(). diff --git a/sudo.c b/sudo.c index a4ed54613..abf9c37ac 100644 --- a/sudo.c +++ b/sudo.c @@ -153,7 +153,7 @@ login_cap_t *lc; #ifdef HAVE_BSD_AUTH_H char *login_style; #endif /* HAVE_BSD_AUTH_H */ -sigaction_t saved_sa_int, saved_sa_quit, saved_sa_tstp, saved_sa_chld; +sigaction_t saved_sa_int, saved_sa_quit, saved_sa_tstp; static char *runas_user; static char *runas_group; static struct sudo_nss_list *snl; @@ -208,8 +208,6 @@ main(argc, argv, envp) (void) sigaction(SIGINT, &sa, &saved_sa_int); (void) sigaction(SIGQUIT, &sa, &saved_sa_quit); (void) sigaction(SIGTSTP, &sa, &saved_sa_tstp); - sa.sa_handler = reapchild; - (void) sigaction(SIGCHLD, &sa, &saved_sa_chld); /* * Turn off core dumps and make sure fds 0-2 are open. @@ -492,7 +490,6 @@ main(argc, argv, envp) (void) sigaction(SIGINT, &saved_sa_int, NULL); (void) sigaction(SIGQUIT, &saved_sa_quit, NULL); (void) sigaction(SIGTSTP, &saved_sa_tstp, NULL); - (void) sigaction(SIGCHLD, &saved_sa_chld, NULL); /* Close the password and group files and free up memory. */ sudo_endpwent(); diff --git a/sudo_edit.c b/sudo_edit.c index ca6771615..8548d7a50 100644 --- a/sudo_edit.c +++ b/sudo_edit.c @@ -59,7 +59,7 @@ __unused static const char rcsid[] = "$Sudo$"; #endif /* lint */ -extern sigaction_t saved_sa_int, saved_sa_quit, saved_sa_tstp, saved_sa_chld; +extern sigaction_t saved_sa_int, saved_sa_quit, saved_sa_tstp; extern char **environ; /* @@ -226,11 +226,10 @@ int sudo_edit(argc, argv, envp) nargv[ac++] = tf[i++].tfile; nargv[ac] = NULL; - /* We wait for our own children and can be suspended. */ + /* Allow the editor to be suspended. */ sigemptyset(&sa.sa_mask); sa.sa_flags = SA_RESTART; sa.sa_handler = SIG_DFL; - (void) sigaction(SIGCHLD, &sa, NULL); (void) sigaction(SIGTSTP, &saved_sa_tstp, NULL); /* @@ -246,7 +245,6 @@ int sudo_edit(argc, argv, envp) /* child */ (void) sigaction(SIGINT, &saved_sa_int, NULL); (void) sigaction(SIGQUIT, &saved_sa_quit, NULL); - (void) sigaction(SIGCHLD, &saved_sa_chld, NULL); set_perms(PERM_FULL_USER); closefrom(def_closefrom + 1); execvp(nargv[0], nargv); diff --git a/visudo.c b/visudo.c index dc6d11ffb..433614ef6 100644 --- a/visudo.c +++ b/visudo.c @@ -651,19 +651,13 @@ run_command(path, argv) char **argv; { int status; - pid_t pid; - sigset_t set, oset; - - (void) sigemptyset(&set); - (void) sigaddset(&set, SIGCHLD); - (void) sigprocmask(SIG_BLOCK, &set, &oset); + pid_t pid, rv; switch (pid = fork()) { case -1: error(1, "unable to run %s", path); break; /* NOTREACHED */ case 0: - (void) sigprocmask(SIG_SETMASK, &oset, NULL); sudo_endpwent(); sudo_endgrent(); closefrom(STDERR_FILENO + 1); @@ -673,15 +667,15 @@ run_command(path, argv) break; /* NOTREACHED */ } + do { #ifdef sudo_waitpid - pid = sudo_waitpid(pid, &status, 0); + rv = sudo_waitpid(pid, &status, 0); #else - pid = wait(&status); + rv = wait(&status); #endif + } while (rv == -1 && errno == EINTR); - (void) sigprocmask(SIG_SETMASK, &oset, NULL); - - if (pid == -1 || !WIFEXITED(status)) + if (rv == -1 || !WIFEXITED(status)) return(-1); return(WEXITSTATUS(status)); }