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@

1 files changed, 28 insertions, 80 deletions

diff --git a/usr.bin/ssh/serverloop.c b/usr.bin/ssh/serverloop.c
index b1e8db5bb13..dd13b792029 100644
--- 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)