isa/sudo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Better signal handling.

Browse Source 
Instead of using a single variable to store the received signal, use
an array so we can't lose a signal when multiple are sent.
Fix process termination by SIGALRM in non-I/O logger mode.
Fix relaying terminal signals to the child in non-I/O logger mode.
This commit is contained in:
Todd C. Miller
2010-04-08 07:40:04 -04:00
parent a3f4278388
commit 5b3d150932
1 changed files with 93 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

145
src/script.c
View File

@@ -65,6 +65,16 @@
# include <selinux/selinux.h> # include <selinux/selinux.h>
#endif #endif
#if !defined(NSIG)
# if defined(_NSIG)
# define NSIG _NSIG
# elif defined(__NSIG)
# define NSIG __NSIG
# else
# define NSIG 64
# endif
#endif
#include "sudo.h" /* XXX? */ #include "sudo.h" /* XXX? */
#include "sudo_plugin.h" #include "sudo_plugin.h"
#include "sudo_plugin_int.h" #include "sudo_plugin_int.h"
@@ -93,8 +103,7 @@ struct script_buf {
static int script_fds[3] = { -1, -1, -1 }; static int script_fds[3] = { -1, -1, -1 };
static int ttyout; static int ttyout;
/* XXX - use an array of signals instead of just a single variable */ static sig_atomic_t recvsig[NSIG];
static sig_atomic_t recvsig = 0;
static sig_atomic_t ttymode = TERM_COOKED; static sig_atomic_t ttymode = TERM_COOKED;
static sig_atomic_t tty_initialized = 0; static sig_atomic_t tty_initialized = 0;
@@ -281,6 +290,26 @@ my_execve(const char *path, char *const argv[], char *const envp[])
return -1; return -1;
} }
static void
terminate_child(pid_t pid, int use_pgrp)
{
/*
* Kill child with increasing urgency.
* Note that SIGCHLD will interrupt the sleep()
*/
if (use_pgrp) {
killpg(pid, SIGHUP);
killpg(pid, SIGTERM);
sleep(2);
killpg(pid, SIGKILL);
} else {
kill(pid, SIGHUP);
kill(pid, SIGTERM);
sleep(2);
kill(pid, SIGKILL);
}
}
/* /*
* This is a little bit tricky due to how POSIX job control works and * This is a little bit tricky due to how POSIX job control works and
* we fact that we have two different controlling terminals to deal with. * we fact that we have two different controlling terminals to deal with.
@@ -302,7 +331,7 @@ script_execve(struct command_details *details, char *argv[], char *envp[],
sigaction_t sa; sigaction_t sa;
struct script_buf input, output; struct script_buf input, output;
int n, nready; int n, nready;
int relaysig = 0, sv[2]; int sv[2];
fd_set *fdsr, *fdsw; fd_set *fdsr, *fdsw;
int rbac_enabled = 0; int rbac_enabled = 0;
int log_io, maxfd; int log_io, maxfd;
@@ -369,13 +398,9 @@ script_execve(struct command_details *details, char *argv[], char *envp[],
/* If stdout is not a tty we handle post-processing differently. */ /* If stdout is not a tty we handle post-processing differently. */
ttyout = isatty(STDOUT_FILENO); ttyout = isatty(STDOUT_FILENO);
/* Signals to relay from parent to child. */ /* Job control signals to relay from parent to child. */
sa.sa_flags = 0; /* do not restart syscalls for these */ sa.sa_flags = 0; /* do not restart syscalls */
sa.sa_handler = handler; sa.sa_handler = handler;
sigaction(SIGHUP, &sa, NULL);
sigaction(SIGTERM, &sa, NULL);
sigaction(SIGINT, &sa, NULL);
sigaction(SIGQUIT, &sa, NULL);
sigaction(SIGTSTP, &sa, NULL); sigaction(SIGTSTP, &sa, NULL);
#if 0 /* XXX - keep these? */ #if 0 /* XXX - keep these? */
sigaction(SIGTTIN, &sa, NULL); sigaction(SIGTTIN, &sa, NULL);
@@ -400,6 +425,14 @@ script_execve(struct command_details *details, char *argv[], char *envp[],
} }
} }
/* Non-job control signals to relay from parent to child. */
sa.sa_flags = 0; /* do not restart syscalls */
sa.sa_handler = handler;
sigaction(SIGHUP, &sa, NULL);
sigaction(SIGTERM, &sa, NULL);
sigaction(SIGINT, &sa, NULL);
sigaction(SIGQUIT, &sa, NULL);
/* Set command timeout if specified. */ /* Set command timeout if specified. */
if (ISSET(details->flags, CD_SET_TIMEOUT)) if (ISSET(details->flags, CD_SET_TIMEOUT))
alarm(details->timeout); alarm(details->timeout);
@@ -473,28 +506,30 @@ script_execve(struct command_details *details, char *argv[], char *envp[],
zero_bytes(&output, sizeof(output)); zero_bytes(&output, sizeof(output));
for (;;) { for (;;) {
/* Wait for children as needed. */ /* Wait for children as needed. */
if (recvsig == SIGCHLD) { if (recvsig[SIGCHLD]) {
pid_t pid; pid_t pid;
int flags = WNOHANG; int flags = WNOHANG;
recvsig = 0; recvsig[SIGCHLD] = FALSE;
if (log_io) if (log_io)
flags |= WUNTRACED; flags |= WUNTRACED;
do { do {
do { pid = waitpid(child, &child_status, flags);
pid = waitpid(child, &child_status, flags); } while (pid == -1 && errno == EINTR);
} while (pid == -1 && errno == EINTR); if (pid == child) {
if (pid == child) { /*
if (!log_io) * If there is no I/O logger we are done. Otherwise,
recvsig = SIGCHLD; /* XXX - hacky, see below */ * we wait for ECONNRESET or EPIPE from the socketpair.
else if (WIFSTOPPED(child_status)) */
relaysig = WSTOPSIG(child_status); if (!log_io)
} recvsig[SIGCHLD] = TRUE; /* XXX - hacky, see below */
} while (pid > 0); else if (WIFSTOPPED(child_status))
recvsig[WSTOPSIG(child_status)] = TRUE;
}
} }
/* If not logging I/O and child has exited we are done. */ /* If not logging I/O and child has exited we are done. */
if (!log_io && recvsig == SIGCHLD) { if (!log_io && recvsig[SIGCHLD]) {
cstat->type = CMD_WSTATUS; cstat->type = CMD_WSTATUS;
cstat->val = child_status; cstat->val = child_status;
break; break;
@@ -519,12 +554,19 @@ script_execve(struct command_details *details, char *argv[], char *envp[],
FD_SET(script_fds[SFD_MASTER], fdsw); FD_SET(script_fds[SFD_MASTER], fdsw);
} }
FD_SET(sv[0], fdsr); FD_SET(sv[0], fdsr);
if (relaysig) { for (n = 0; n < NSIG; n++) {
if (log_io) { if (recvsig[n] && n != SIGCHLD) {
FD_SET(sv[0], fdsw); if (log_io) {
} else { FD_SET(sv[0], fdsw);
/* nothing listening on sv[0], send directly */ break;
deliver_signal(child, relaysig); } else {
/* nothing listening on sv[0], send directly */
if (n == SIGALRM) {
terminate_child(child, FALSE);
} else {
kill(child, n);
}
}
} }
} }
@@ -550,8 +592,9 @@ script_execve(struct command_details *details, char *argv[], char *envp[],
if (WIFSTOPPED(cstat->val)) { if (WIFSTOPPED(cstat->val)) {
/* Suspend parent and tell child how to resume on return. */ /* Suspend parent and tell child how to resume on return. */
sudo_debug(8, "child stopped, suspending parent"); sudo_debug(8, "child stopped, suspending parent");
relaysig = suspend_parent(WSTOPSIG(cstat->val), &output); n = suspend_parent(WSTOPSIG(cstat->val), &output);
/* XXX - write relaysig immediately? */ recvsig[n] = TRUE;
/* XXX - write sig immediately? */
continue; continue;
} else { } else {
/* Child exited or was killed, either way we are done. */ /* Child exited or was killed, either way we are done. */
@@ -563,16 +606,21 @@ script_execve(struct command_details *details, char *argv[], char *envp[],
} }
} }
if (FD_ISSET(sv[0], fdsw)) { if (FD_ISSET(sv[0], fdsw)) {
/* XXX - we rely on child to be suspended before we suspend us */ /* XXX - we rely on child to be suspended before we suspend ourselves */
sudo_debug(9, "sending signal %d to child over backchannel", relaysig); for (n = 0; n < NSIG; n++) {
cstat->type = CMD_SIGNO; if (!recvsig[n])
cstat->val = relaysig; continue;
relaysig = 0; recvsig[n] = FALSE;
do { sudo_debug(9, "sending signal %d to child over backchannel", n);
n = send(sv[0], cstat, sizeof(*cstat), 0); cstat->type = CMD_SIGNO;
} while (n == -1 && errno == EINTR); cstat->val = n;
if (n != sizeof(*cstat)) { do {
break; /* should not happen */ n = send(sv[0], cstat, sizeof(*cstat), 0);
} while (n == -1 && errno == EINTR);
if (n != sizeof(*cstat)) {
recvsig[n] = TRUE;
break;
}
} }
} }
if (!log_io) if (!log_io)
@@ -681,12 +729,7 @@ deliver_signal(pid_t pid, int signo)
killpg(pid, signo); killpg(pid, signo);
break; break;
case SIGALRM: case SIGALRM:
/* command time out, kill child with increasing urgency */ terminate_child(child, TRUE);
killpg(pid, SIGHUP);
sleep(1);
killpg(pid, SIGTERM);
sleep(1);
killpg(pid, SIGKILL);
break; break;
case SIGUSR1: case SIGUSR1:
/* foreground process, grant it controlling tty. */ /* foreground process, grant it controlling tty. */
@@ -717,8 +760,6 @@ script_child(const char *path, char *argv[], char *envp[], int backchannel, int
pid_t pid; pid_t pid;
int n, status; int n, status;
recvsig = 0;
/* Close unused fds. */ /* Close unused fds. */
close(script_fds[SFD_MASTER]); close(script_fds[SFD_MASTER]);
close(script_fds[SFD_USERTTY]); close(script_fds[SFD_USERTTY]);
@@ -820,9 +861,9 @@ script_child(const char *path, char *argv[], char *envp[], int backchannel, int
zero_bytes(fdsr, howmany(backchannel + 1, NFDBITS) * sizeof(fd_mask)); zero_bytes(fdsr, howmany(backchannel + 1, NFDBITS) * sizeof(fd_mask));
FD_SET(backchannel, fdsr); FD_SET(backchannel, fdsr);
for (;;) { for (;;) {
/* Read child status, assumes recvsig can only be SIGCHLD */ /* Read child status */
while (recvsig) { while (recvsig[SIGCHLD]) {
recvsig = 0; recvsig[SIGCHLD] = FALSE;
/* read child status and relay to parent */ /* read child status and relay to parent */
do { do {
pid = waitpid(child, &status, WUNTRACED|WNOHANG); pid = waitpid(child, &status, WUNTRACED|WNOHANG);
@@ -962,7 +1003,7 @@ sync_ttysize(int src, int dst)
static void static void
handler(int s) handler(int s)
{ {
recvsig = s; recvsig[s] = TRUE;
} }
/* /*