When execve() of the command fails, it is possible to receive SIGCHLD

before we've read the error status from the pipe.  Re-order things
such that we send the final status at the very end and prefer error
status over wait status.
This commit is contained in:
Todd C. Miller
2010-05-20 07:33:14 -04:00
parent d2b8bad2a5
commit f8ff268318

View File

@@ -496,7 +496,7 @@ script_execve(struct command_details *details, char *argv[], char *envp[],
int rbac_enabled = 0; int rbac_enabled = 0;
int log_io, maxfd; int log_io, maxfd;
cstat->type = 0; /* XXX */ cstat->type = CMD_INVALID;
log_io = !tq_empty(&io_plugins); log_io = !tq_empty(&io_plugins);
@@ -918,26 +918,69 @@ deliver_signal(pid_t pid, int signo)
static int static int
send_status(int fd, struct command_status *cstat) send_status(int fd, struct command_status *cstat)
{ {
int n; int n = -1;
do { if (cstat->type != CMD_INVALID) {
n = send(fd, cstat, sizeof(*cstat), 0); do {
} while (n == -1 && errno == EINTR); n = send(fd, cstat, sizeof(*cstat), 0);
if (n != sizeof(*cstat)) { } while (n == -1 && errno == EINTR);
sudo_debug(8, "unable to send status to parent: %s", strerror(errno)); if (n != sizeof(*cstat)) {
} else { sudo_debug(8, "unable to send status to parent: %s", strerror(errno));
sudo_debug(8, "sent status to parent"); } else {
sudo_debug(8, "sent status to parent");
}
cstat->type = CMD_INVALID; /* prevent re-sending */
} }
return n; return n;
} }
/*
* Wait for child status after receiving SIGCHLD.
* If the child was stopped, the status is send back
* to the parent.
* Otherwise, cstat is filled in but not sent.
* Returns TRUE if child is still alive, else false.
*/
static int
handle_sigchld(int backchannel, struct command_status *cstat)
{
int status, alive = TRUE;
pid_t pid;
/* read child status */
do {
pid = waitpid(child, &status, WUNTRACED|WNOHANG);
} while (pid == -1 && errno == EINTR);
if (pid == child) {
if (cstat->type != CMD_ERRNO) {
cstat->type = CMD_WSTATUS;
cstat->val = status;
if (WIFSTOPPED(status)) {
sudo_debug(8, "command stopped, signal %d",
WSTOPSIG(status));
if (send_status(backchannel, cstat) == -1)
return alive; /* XXX */
} else if (WIFSIGNALED(status)) {
sudo_debug(8, "command killed, signal %d",
WTERMSIG(status));
} else {
sudo_debug(8, "command exited: %d",
WEXITSTATUS(status));
}
}
if (!WIFSTOPPED(status))
alive = FALSE;
}
return alive;
}
int int
script_child(const char *path, char *argv[], char *envp[], int backchannel, int rbac) script_child(const char *path, char *argv[], char *envp[], int backchannel, int rbac)
{ {
struct command_status cstat; struct command_status cstat;
struct timeval tv;
fd_set *fdsr; fd_set *fdsr;
sigaction_t sa; sigaction_t sa;
pid_t pid;
int errpipe[2], maxfd, n, status; int errpipe[2], maxfd, n, status;
int alive = TRUE; int alive = TRUE;
@@ -1068,38 +1111,13 @@ script_child(const char *path, char *argv[], char *envp[], int backchannel, int
fdsr = (fd_set *)emalloc2(howmany(maxfd + 1, NFDBITS), sizeof(fd_mask)); fdsr = (fd_set *)emalloc2(howmany(maxfd + 1, NFDBITS), sizeof(fd_mask));
zero_bytes(fdsr, howmany(maxfd + 1, NFDBITS) * sizeof(fd_mask)); zero_bytes(fdsr, howmany(maxfd + 1, NFDBITS) * sizeof(fd_mask));
zero_bytes(&cstat, sizeof(cstat)); zero_bytes(&cstat, sizeof(cstat));
tv.tv_sec = 0;
tv.tv_usec = 0;
for (;;) { for (;;) {
/* Read child status */ /* Read child status. */
if (recvsig[SIGCHLD]) { if (recvsig[SIGCHLD]) {
recvsig[SIGCHLD] = FALSE; recvsig[SIGCHLD] = FALSE;
/* read child status and relay to parent */ alive = handle_sigchld(backchannel, &cstat);
do {
pid = waitpid(child, &status, WUNTRACED|WNOHANG);
} while (pid == -1 && errno == EINTR);
if (pid == child) {
if (WIFSTOPPED(status)) {
sudo_debug(8, "command stopped, signal %d",
WSTOPSIG(status));
} else {
if (WIFSIGNALED(status))
sudo_debug(8, "command killed, signal %d",
WTERMSIG(status));
else
sudo_debug(8, "command exited: %d",
WEXITSTATUS(status));
alive = FALSE;
}
/* Send wait status unless we previously sent errno. */
if (cstat.type != CMD_ERRNO) {
cstat.type = CMD_WSTATUS;
cstat.val = status;
n = send_status(backchannel, &cstat);
if (n == -1)
goto done;
}
if (!alive)
goto done;
}
} }
/* Check for signal on backchannel or errno on errpipe. */ /* Check for signal on backchannel or errno on errpipe. */
@@ -1110,8 +1128,11 @@ script_child(const char *path, char *argv[], char *envp[], int backchannel, int
if (recvsig[SIGCHLD]) if (recvsig[SIGCHLD])
continue; continue;
n = select(maxfd + 1, fdsr, NULL, NULL, NULL); /* If command exited we just poll, there may be data on errpipe. */
if (n == -1) { n = select(maxfd + 1, fdsr, NULL, NULL, alive ? NULL : &tv);
if (n <= 0) {
if (n == 0)
goto done;
if (errno == EINTR) if (errno == EINTR)
continue; continue;
error(1, "select failed"); error(1, "select failed");
@@ -1126,15 +1147,6 @@ script_child(const char *path, char *argv[], char *envp[], int backchannel, int
warning("error reading from pipe"); warning("error reading from pipe");
goto done; goto done;
} }
if (n == sizeof(cstat)) {
/* execve() failed, relay errno back to parent */
if (cstat.type == CMD_ERRNO) {
n = send_status(backchannel, &cstat);
if (n == -1)
goto done;
} else
warningx("unexpected reply type on pipe: %d", cstat.type);
}
/* Got errno or EOF, either way we are done with errpipe. */ /* Got errno or EOF, either way we are done with errpipe. */
FD_CLR(errpipe[0], fdsr); FD_CLR(errpipe[0], fdsr);
close(errpipe[0]); close(errpipe[0]);
@@ -1158,8 +1170,13 @@ script_child(const char *path, char *argv[], char *envp[], int backchannel, int
} }
done: done:
if (alive) if (alive) {
/* XXX An error occurred, should send an error back. */
kill(child, SIGKILL); kill(child, SIGKILL);
} else {
/* Send parent status. */
send_status(backchannel, &cstat);
}
_exit(1); _exit(1);
bad: bad: