From: Todd C. Miller Date: Wed, 25 Jan 2012 21:29:08 +0000 (-0500) Subject: The change in 818e82ecbbfc that caused to exit when the monitor X-Git-Tag: SUDO_1_8_4~41^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1877c455d1f2683b64fe71e262e2aeba539a773e;p=sudo 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. --- diff --git a/src/exec.c b/src/exec.c index 9b63eae5a..80e87177a 100644 --- a/src/exec.c +++ b/src/exec.c @@ -74,7 +74,7 @@ struct sigforward { TQ_DECLARE(sigforward) 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); static void forward_signals(int fd); static void schedule_signal(int signo); @@ -343,7 +343,7 @@ sudo_execute(struct command_details *details, struct command_status *cstat) forward_signals(sv[0]); } 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) { /* Child has exited, cstat is set, we are done. */ 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. */ 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; 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 * and select() will return with sv[0] readable. - * On Linux that doesn't appear to happen so we - * treat the monitor dying as a fatal error. - * Note that the wait status we return is that of - * the monitor and not the command; unfortunately - * that is the best that we can do here. + * On Linux that doesn't appear to happen so if the + * monitor dies, shut down the socketpair to force a + * select() notification. */ - if (!WIFSTOPPED(status)) { - cstat->type = CMD_WSTATUS; - cstat->val = status; - debug_return_int(0); - } + (void) shutdown(sv[0], SHUT_WR); } else { if (WIFSTOPPED(status)) { /* diff --git a/src/sudo.c b/src/sudo.c index 49d042e36..5eebad7a7 100644 --- a/src/sudo.c +++ b/src/sudo.c @@ -1020,10 +1020,12 @@ run_command(struct command_details *details) switch (cstat.type) { case CMD_ERRNO: /* 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); 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); } exitcode = 1; @@ -1031,11 +1033,11 @@ run_command(struct command_details *details) case CMD_WSTATUS: /* Command ran, exited or was killed. */ 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); tq_foreach_fwd(&io_plugins, plugin) { 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); } if (WIFEXITED(cstat.val))