Replace SIGCHLD/notify_pipe kludge with pselect.

Previously sshd's SIGCHLD handler would wake up select() by writing a
byte to notify_pipe.  We can remove this by blocking SIGCHLD, checking
for child terminations then passing the original signal mask through
to pselect.  This ensures that the pselect will immediately wake up if
a child terminates between wait()ing on them and the pselect.

In -portable, for platforms that do not have pselect the kludge is still
there but is hidden behind a pselect interface.

Based on other changes for bz#2158, ok djm@

diff --git a/usr.bin/ssh/serverloop.c b/usr.bin/ssh/serverloop.c
index b1e8db5..dd13b79 100644 (file)
--- a/usr.bin/ssh/serverloop.c
+++ b/usr.bin/ssh/serverloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: serverloop.c,v 1.226 2021/04/03 06:18:41 djm Exp $ */
+/* $OpenBSD: serverloop.c,v 1.227 2021/06/25 03:38:17 dtucker Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -84,11 +84,6 @@ extern int use_privsep;
 
 static int no_more_sessions = 0; /* Disallow further sessions. */
 
-/*
- * This SIGCHLD kludge is used to detect when the child exits.  The server
- * will exit after that, as soon as forwarded connections have terminated.
- */
-
 static volatile sig_atomic_t child_terminated = 0;     /* The child has terminated. */
 
 /* Cleanup on signals (!use_privsep case only) */
@@ -111,59 +106,11 @@ bind_permitted(int port, uid_t uid)
        return 1;
 }
 
-/*
- * we write to this pipe if a SIGCHLD is caught in order to avoid
- * the race between select() and child_terminated
- */
-static int notify_pipe[2];
-static void
-notify_setup(void)
-{
-       if (pipe(notify_pipe) == -1) {
-               error("pipe(notify_pipe) failed %s", strerror(errno));
-       } else if ((fcntl(notify_pipe[0], F_SETFD, FD_CLOEXEC) == -1) ||
-           (fcntl(notify_pipe[1], F_SETFD, FD_CLOEXEC) == -1)) {
-               error("fcntl(notify_pipe, F_SETFD) failed %s", strerror(errno));
-               close(notify_pipe[0]);
-               close(notify_pipe[1]);
-       } else {
-               set_nonblock(notify_pipe[0]);
-               set_nonblock(notify_pipe[1]);
-               return;
-       }
-       notify_pipe[0] = -1;    /* read end */
-       notify_pipe[1] = -1;    /* write end */
-}
-static void
-notify_parent(void)
-{
-       if (notify_pipe[1] != -1)
-               (void)write(notify_pipe[1], "", 1);
-}
-static void
-notify_prepare(fd_set *readset)
-{
-       if (notify_pipe[0] != -1)
-               FD_SET(notify_pipe[0], readset);
-}
-static void
-notify_done(fd_set *readset)
-{
-       char c;
-
-       if (notify_pipe[0] != -1 && FD_ISSET(notify_pipe[0], readset))
-               while (read(notify_pipe[0], &c, 1) != -1)
-                       debug2_f("reading");
-}
-
 /*ARGSUSED*/
 static void
 sigchld_handler(int sig)
 {
-       int save_errno = errno;
        child_terminated = 1;
-       notify_parent();
-       errno = save_errno;
 }
 
 /*ARGSUSED*/
@@ -207,8 +154,8 @@ client_alive_check(struct ssh *ssh)
 }
 
 /*
- * Sleep in select() until we can do something.  This will initialize the
- * select masks.  Upon return, the masks will indicate which descriptors
+ * Sleep in pselect() until we can do something.  This will initialize the
+ * pselect masks.  Upon return, the masks will indicate which descriptors
  * have data or can accept data.  Optionally, a maximum time can be specified
  * for the duration of the wait (0 = infinite).
  */
@@ -216,16 +163,16 @@ static void
 wait_until_can_do_something(struct ssh *ssh,
     int connection_in, int connection_out,
     fd_set **readsetp, fd_set **writesetp, int *maxfdp,
-    u_int *nallocp, u_int64_t max_time_ms)
+    u_int *nallocp, u_int64_t max_time_ms, sigset_t *sigsetp)
 {
-       struct timeval tv, *tvp;
+       struct timespec ts, *tsp;
        int ret;
        time_t minwait_secs = 0;
        int client_alive_scheduled = 0;
        /* time we last heard from the client OR sent a keepalive */
        static time_t last_client_time;
 
-       /* Allocate and update select() masks for channel descriptors. */
+       /* Allocate and update pselect() masks for channel descriptors. */
        channel_prepare_select(ssh, readsetp, writesetp, maxfdp,
            nallocp, &minwait_secs);
 
@@ -258,7 +205,6 @@ wait_until_can_do_something(struct ssh *ssh,
        if (channel_not_very_much_buffered_data())
 #endif
        FD_SET(connection_in, *readsetp);
-       notify_prepare(*readsetp);
 
        /*
         * If we have buffered packet data going to the client, mark that
@@ -276,26 +222,26 @@ wait_until_can_do_something(struct ssh *ssh,
                        max_time_ms = 100;
 
        if (max_time_ms == 0)
-               tvp = NULL;
+               tsp = NULL;
        else {
-               tv.tv_sec = max_time_ms / 1000;
-               tv.tv_usec = 1000 * (max_time_ms % 1000);
-               tvp = &tv;
+               ts.tv_sec = max_time_ms / 1000;
+               ts.tv_nsec = 1000000 * (max_time_ms % 1000);
+               tsp = &ts;
        }
 
        /* Wait for something to happen, or the timeout to expire. */
-       ret = select((*maxfdp)+1, *readsetp, *writesetp, NULL, tvp);
+       ret = pselect((*maxfdp)+1, *readsetp, *writesetp, NULL, tsp, sigsetp);
 
        if (ret == -1) {
                memset(*readsetp, 0, *nallocp);
                memset(*writesetp, 0, *nallocp);
                if (errno != EINTR)
-                       error("select: %.100s", strerror(errno));
+                       error("pselect: %.100s", strerror(errno));
        } else if (client_alive_scheduled) {
                time_t now = monotime();
 
                /*
-                * If the select timed out, or returned for some other reason
+                * If the pselect timed out, or returned for some other reason
                 * but we haven't heard from the client in time, send keepalive.
                 */
                if (ret == 0 || (last_client_time != 0 && last_client_time +
@@ -306,8 +252,6 @@ wait_until_can_do_something(struct ssh *ssh,
                        last_client_time = now;
                }
        }
-
-       notify_done(*readsetp);
 }
 
 /*
@@ -369,13 +313,8 @@ static void
 collect_children(struct ssh *ssh)
 {
        pid_t pid;
-       sigset_t oset, nset;
        int status;
 
-       /* block SIGCHLD while we check for dead children */
-       sigemptyset(&nset);
-       sigaddset(&nset, SIGCHLD);
-       sigprocmask(SIG_BLOCK, &nset, &oset);
        if (child_terminated) {
                debug("Received SIGCHLD.");
                while ((pid = waitpid(-1, &status, WNOHANG)) > 0 ||
@@ -384,7 +323,6 @@ collect_children(struct ssh *ssh)
                                session_close_by_pid(ssh, pid, status);
                child_terminated = 0;
        }
-       sigprocmask(SIG_SETMASK, &oset, NULL);
 }
 
 void
@@ -394,9 +332,12 @@ server_loop2(struct ssh *ssh, Authctxt *authctxt)
        int max_fd;
        u_int nalloc = 0, connection_in, connection_out;
        u_int64_t rekey_timeout_ms = 0;
+       sigset_t bsigset, osigset;
 
        debug("Entering interactive session for SSH2.");
 
+       if (sigemptyset(&bsigset) == -1 || sigaddset(&bsigset, SIGCHLD) == -1)
+               error_f("bsigset setup: %s", strerror(errno));
        ssh_signal(SIGCHLD, sigchld_handler);
        child_terminated = 0;
        connection_in = ssh_packet_get_connection_in(ssh);
@@ -408,10 +349,7 @@ server_loop2(struct ssh *ssh, Authctxt *authctxt)
                ssh_signal(SIGQUIT, sigterm_handler);
        }
 
-       notify_setup();
-
        max_fd = MAXIMUM(connection_in, connection_out);
-       max_fd = MAXIMUM(max_fd, notify_pipe[0]);
 
        server_init_dispatch(ssh);
 
@@ -429,8 +367,19 @@ server_loop2(struct ssh *ssh, Authctxt *authctxt)
                        rekey_timeout_ms = 0;
                }
 
+               /*
+                * Block SIGCHLD while we check for dead children, then pass
+                * the old signal mask through to pselect() so that it'll wake
+                * up immediately if a child exits after we've called waitpid().
+                */
+               if (sigprocmask(SIG_BLOCK, &bsigset, &osigset) == -1)
+                       error_f("bsigset sigprocmask: %s", strerror(errno));
+               collect_children(ssh);
                wait_until_can_do_something(ssh, connection_in, connection_out,
-                   &readset, &writeset, &max_fd, &nalloc, rekey_timeout_ms);
+                   &readset, &writeset, &max_fd, &nalloc, rekey_timeout_ms,
+                   &osigset);
+               if (sigprocmask(SIG_UNBLOCK, &bsigset, &osigset) == -1)
+                       error_f("osigset sigprocmask: %s", strerror(errno));
 
                if (received_sigterm) {
                        logit("Exiting on signal %d", (int)received_sigterm);
@@ -438,7 +387,6 @@ server_loop2(struct ssh *ssh, Authctxt *authctxt)
                        cleanup_exit(255);
                }
 
-               collect_children(ssh);
                if (!ssh_packet_is_rekeying(ssh))
                        channel_after_select(ssh, readset, writeset);
                if (process_input(ssh, readset, connection_in) < 0)