Simplify pselect shim and remove side effects.

Instead of maintaing state (pipe descriptors, signal handlers) across
pselect-on-select invocations, set up and restore them each call.
This prevents outside factors (eg a closefrom or signal handler
installation) from potentially causing problems.  This does result in a
drop in throughput of a couple of percent on geriatric platforms without
a native pselect due to the extra overhead.  Tweaks & ok djm@

diff --git a/openbsd-compat/bsd-pselect.c b/openbsd-compat/bsd-pselect.c
index b3632086368a6b5cc639a8e57c2d08006af440f4..26bdc3e08f039b0a650c161b0f6743a1954d5fc4 100644 (file)
--- a/openbsd-compat/bsd-pselect.c
+++ b/openbsd-compat/bsd-pselect.c
@@ -40,75 +40,47 @@
 #include <unistd.h>
 
 #include "log.h"
-#include "misc.h"      /* for set_nonblock */
 
 #ifndef HAVE_SIGHANDLER_T
 typedef void (*sighandler_t)(int);
 #endif
 
 static sighandler_t saved_sighandler[_NSIG];
+static int notify_pipe[2];     /* 0 = read end, 1 = write end */
 
 /*
- * Set up the descriptors.  Because they are close-on-exec, in the case
- * where sshd's re-exec fails notify_pipe will still point to a descriptor
- * that was closed by the exec attempt but if that descriptor has been
- * reopened then we'll attempt to use that.  Ensure that notify_pipe is
- * outside of the range used by sshd re-exec but within NFDBITS (so we don't
- * need to expand the fd_sets).
+ * Because the debugging for this is so noisy, we only output on the first
+ * call, and suppress it thereafter.
  */
-#define REEXEC_MIN_FREE_FD (STDERR_FILENO + 4)
-static int
-pselect_notify_setup_fd(int *fd)
+static int suppress_debug;
+
+static void
+pselect_set_nonblock(int fd)
 {
-       int r;
+       int val;
 
-       if ((r = fcntl(*fd, F_DUPFD, REEXEC_MIN_FREE_FD)) < 0 ||
-           fcntl(r, F_SETFD, FD_CLOEXEC) < 0 || r >= FD_SETSIZE)
-               return -1;
-       (void)close(*fd);
-       return (*fd = r);
+       if ((val = fcntl(fd, F_GETFL)) == -1 ||
+            fcntl(fd, F_SETFL, val|O_NONBLOCK) == -1)
+               error_f("fcntl: %s", strerror(errno));
 }
 
 /*
  * we write to this pipe if a SIGCHLD is caught in order to avoid
- * the race between select() and child_terminated
+ * the race between select() and child_terminated.
  */
-static pid_t notify_pid;
-static int notify_pipe[2];
-static void
+static int
 pselect_notify_setup(void)
 {
-       static int initialized;
-
-       if (initialized && notify_pid == getpid())
-               return;
-       if (notify_pid == 0)
-               debug3_f("initializing");
-       else {
-               debug3_f("pid changed, reinitializing");
-               if (notify_pipe[0] != -1)
-                       close(notify_pipe[0]);
-               if (notify_pipe[1] != -1)
-                       close(notify_pipe[1]);
-       }
        if (pipe(notify_pipe) == -1) {
                error("pipe(notify_pipe) failed %s", strerror(errno));
-       } else if (pselect_notify_setup_fd(&notify_pipe[0]) == -1 ||
-           pselect_notify_setup_fd(&notify_pipe[1]) == -1) {
-               error("fcntl(notify_pipe, ...) failed %s", strerror(errno));
-               close(notify_pipe[0]);
-               close(notify_pipe[1]);
-       } else {
-               set_nonblock(notify_pipe[0]);
-               set_nonblock(notify_pipe[1]);
-               notify_pid = getpid();
-               debug3_f("pid %d saved %d pipe0 %d pipe1 %d", getpid(),
-                   notify_pid, notify_pipe[0], notify_pipe[1]);
-               initialized = 1;
-               return;
+               notify_pipe[0] = notify_pipe[1] = -1;
+               return -1;
        }
-       notify_pipe[0] = -1;    /* read end */
-       notify_pipe[1] = -1;    /* write end */
+       pselect_set_nonblock(notify_pipe[0]);
+       pselect_set_nonblock(notify_pipe[1]);
+       if (!suppress_debug)
+               debug3_f("pipe0 %d pipe1 %d", notify_pipe[0], notify_pipe[1]);
+       return 0;
 }
 static void
 pselect_notify_parent(void)
@@ -132,6 +104,8 @@ pselect_notify_done(fd_set *readset)
                        debug2_f("reading");
                FD_CLR(notify_pipe[0], readset);
        }
+       (void)close(notify_pipe[0]);
+       (void)close(notify_pipe[1]);
 }
 
 /*ARGSUSED*/
@@ -167,26 +141,29 @@ pselect(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
        if (mask == NULL)  /* no signal mask, just call select */
                return select(nfds, readfds, writefds, exceptfds, tvp);
 
-       /* For each signal we're unmasking, install our handler if needed. */
+       /* For each signal unmasked, save old handler and install ours. */
        for (sig = 0; sig < _NSIG; sig++) {
+               saved_sighandler[sig] = NULL;
                if (sig == SIGKILL || sig == SIGSTOP || sigismember(mask, sig))
                        continue;
                if (sigaction(sig, NULL, &sa) == 0 &&
                    sa.sa_handler != SIG_IGN && sa.sa_handler != SIG_DFL) {
                        unmasked = 1;
-                       if (sa.sa_handler == pselect_sig_handler)
-                               continue;
                        sa.sa_handler = pselect_sig_handler;
                        if (sigaction(sig, &sa, &osa) == 0) {
-                               debug3_f("installing signal handler for %s, "
-                                   "previous %p", strsignal(sig),
-                                    osa.sa_handler);
+                               if (!suppress_debug)
+                                       debug3_f("installed signal handler for"
+                                           " %s, previous 0x%p",
+                                           strsignal(sig), osa.sa_handler);
                                saved_sighandler[sig] = osa.sa_handler;
                        }
                }
        }
        if (unmasked) {
-               pselect_notify_setup();
+               if ((ret = pselect_notify_setup()) == -1) {
+                       saved_errno = ENOMEM;
+                       goto out;
+               }
                pselect_notify_prepare(readfds);
                nfds = MAX(nfds, notify_pipe[0] + 1);
        }
@@ -199,6 +176,25 @@ pselect(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
 
        if (unmasked)
                pselect_notify_done(readfds);
+
+ out:
+       /* Restore signal handlers. */
+       for (sig = 0; sig < _NSIG; sig++) {
+               if (saved_sighandler[sig] == NULL)
+                       continue;
+               if (sigaction(sig, NULL, &sa) == 0) {
+                       sa.sa_handler = saved_sighandler[sig];
+                       if (sigaction(sig, &sa, NULL) == 0) {
+                               if (!suppress_debug)
+                                       debug3_f("restored signal handler for "
+                                           "%s", strsignal(sig));
+                       } else {
+                               error_f("failed to restore signal handler for "
+                                   "%s: %s", strsignal(sig), strerror(errno));
+                       }
+               }
+       }
+       suppress_debug = 1;
        errno = saved_errno;
        return ret;
 }