The change in 818e82ecbbfc that caused to exit when the monitor

dies created a race condition between the monitor exiting and the
status being read.  All we really want to do is make sure that
select() notifies us that there is a status change when the monitor
dies unexpectedly so shutdown the socketpair connected to the monitor
for writing when it dies.  That way we can still read the status
that is pending on the socket and select() on Linux will tell us
that the fd is ready.
This commit is contained in:
Todd C. Miller
2012-01-25 16:29:08 -05:00
parent d11e7febbc
commit 1877c455d1
2 changed files with 13 additions and 17 deletions

View File

@@ -74,7 +74,7 @@ struct sigforward {
TQ_DECLARE(sigforward) TQ_DECLARE(sigforward)
static struct sigforward_list sigfwd_list; static struct sigforward_list sigfwd_list;
static int handle_signals(int fd, pid_t child, int log_io, static int handle_signals(int sv[2], pid_t child, int log_io,
struct command_status *cstat); struct command_status *cstat);
static void forward_signals(int fd); static void forward_signals(int fd);
static void schedule_signal(int signo); static void schedule_signal(int signo);
@@ -343,7 +343,7 @@ sudo_execute(struct command_details *details, struct command_status *cstat)
forward_signals(sv[0]); forward_signals(sv[0]);
} }
if (FD_ISSET(signal_pipe[0], fdsr)) { if (FD_ISSET(signal_pipe[0], fdsr)) {
n = handle_signals(signal_pipe[0], child, log_io, cstat); n = handle_signals(sv, child, log_io, cstat);
if (n == 0) { if (n == 0) {
/* Child has exited, cstat is set, we are done. */ /* Child has exited, cstat is set, we are done. */
break; break;
@@ -442,7 +442,7 @@ sudo_execute(struct command_details *details, struct command_status *cstat)
* Returns -1 on error, 0 on child exit, else 1. * Returns -1 on error, 0 on child exit, else 1.
*/ */
static int static int
handle_signals(int fd, pid_t child, int log_io, struct command_status *cstat) handle_signals(int sv[2], pid_t child, int log_io, struct command_status *cstat)
{ {
unsigned char signo; unsigned char signo;
ssize_t nread; ssize_t nread;
@@ -483,17 +483,11 @@ handle_signals(int fd, pid_t child, int log_io, struct command_status *cstat)
/* /*
* On BSD we get ECONNRESET on sv[0] if monitor dies * On BSD we get ECONNRESET on sv[0] if monitor dies
* and select() will return with sv[0] readable. * and select() will return with sv[0] readable.
* On Linux that doesn't appear to happen so we * On Linux that doesn't appear to happen so if the
* treat the monitor dying as a fatal error. * monitor dies, shut down the socketpair to force a
* Note that the wait status we return is that of * select() notification.
* the monitor and not the command; unfortunately
* that is the best that we can do here.
*/ */
if (!WIFSTOPPED(status)) { (void) shutdown(sv[0], SHUT_WR);
cstat->type = CMD_WSTATUS;
cstat->val = status;
debug_return_int(0);
}
} else { } else {
if (WIFSTOPPED(status)) { if (WIFSTOPPED(status)) {
/* /*

View File

@@ -1020,10 +1020,12 @@ run_command(struct command_details *details)
switch (cstat.type) { switch (cstat.type) {
case CMD_ERRNO: case CMD_ERRNO:
/* exec_setup() or execve() returned an error. */ /* exec_setup() or execve() returned an error. */
sudo_debug_printf(SUDO_DEBUG_DEBUG, "calling policy close with errno"); sudo_debug_printf(SUDO_DEBUG_DEBUG,
"calling policy close with errno %d", cstat.val);
policy_close(&policy_plugin, 0, cstat.val); policy_close(&policy_plugin, 0, cstat.val);
tq_foreach_fwd(&io_plugins, plugin) { tq_foreach_fwd(&io_plugins, plugin) {
sudo_debug_printf(SUDO_DEBUG_DEBUG, "calling I/O close with errno"); sudo_debug_printf(SUDO_DEBUG_DEBUG,
"calling I/O close with errno %d", cstat.val);
iolog_close(plugin, 0, cstat.val); iolog_close(plugin, 0, cstat.val);
} }
exitcode = 1; exitcode = 1;
@@ -1031,11 +1033,11 @@ run_command(struct command_details *details)
case CMD_WSTATUS: case CMD_WSTATUS:
/* Command ran, exited or was killed. */ /* Command ran, exited or was killed. */
sudo_debug_printf(SUDO_DEBUG_DEBUG, sudo_debug_printf(SUDO_DEBUG_DEBUG,
"calling policy close with wait status"); "calling policy close with wait status %d", cstat.val);
policy_close(&policy_plugin, cstat.val, 0); policy_close(&policy_plugin, cstat.val, 0);
tq_foreach_fwd(&io_plugins, plugin) { tq_foreach_fwd(&io_plugins, plugin) {
sudo_debug_printf(SUDO_DEBUG_DEBUG, sudo_debug_printf(SUDO_DEBUG_DEBUG,
"calling I/O close with wait status"); "calling I/O close with wait status %d", cstat.val);
iolog_close(plugin, cstat.val, 0); iolog_close(plugin, cstat.val, 0);
} }
if (WIFEXITED(cstat.val)) if (WIFEXITED(cstat.val))