Attachment 3523 Details for Bug 2158 – use pselect in server_accept_loop and wait_until_can_do_something

[patch] use pselect in server_accept_loop and wait_until_can_do_something

sshd-pselect.patch (text/plain), 8.83 KB, created by Darren Tucker on 2021-05-21 17:07:11 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Darren Tucker

Created: 2021-05-21 17:07:11 AEST

Size: 8.83 KB

patch

obsolete

>Index: serverloop.c
>===================================================================
>RCS file: /export/cvs/src/usr.bin/ssh/serverloop.c,v
>retrieving revision 1.226
>diff -u -p -r1.226 serverloop.c
>--- serverloop.c	3 Apr 2021 06:18:41 -0000	1.226
>+++ serverloop.c	21 May 2021 06:08:05 -0000
>@@ -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 *
> 	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 *
> 			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 *
> 			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,11 +332,15 @@ server_loop2(struct ssh *ssh, Authctxt *
> 	int max_fd;
> 	u_int nalloc = 0, connection_in, connection_out;
> 	u_int64_t rekey_timeout_ms = 0;
>+	sigset_t blocksigset, osigset;
> 
> 	debug("Entering interactive session for SSH2.");
> 
>+	sigemptyset(&blocksigset);
>+	sigaddset(&blocksigset, SIGCHLD);
> 	ssh_signal(SIGCHLD, sigchld_handler);
> 	child_terminated = 0;
>+
> 	connection_in = ssh_packet_get_connection_in(ssh);
> 	connection_out = ssh_packet_get_connection_out(ssh);
> 
>@@ -408,10 +350,7 @@ server_loop2(struct ssh *ssh, 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 +368,17 @@ server_loop2(struct ssh *ssh, 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().
>+		 */
>+		sigprocmask(SIG_BLOCK, &blocksigset, &osigset);
>+		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);
>+		sigprocmask(SIG_SETMASK, &osigset, NULL);
> 
> 		if (received_sigterm) {
> 			logit("Exiting on signal %d", (int)received_sigterm);
>@@ -438,7 +386,6 @@ server_loop2(struct ssh *ssh, 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)
>Index: sshd.c
>===================================================================
>RCS file: /export/cvs/src/usr.bin/ssh/sshd.c,v
>retrieving revision 1.573
>diff -u -p -r1.573 sshd.c
>--- sshd.c	7 May 2021 03:09:38 -0000	1.573
>+++ sshd.c	21 May 2021 07:02:04 -0000
>@@ -1087,6 +1087,7 @@ server_accept_loop(int *sock_in, int *so
> 	struct sockaddr_storage from;
> 	socklen_t fromlen;
> 	pid_t pid;
>+	sigset_t nsigset, osigset;
> 
> 	/* setup fd set for accept */
> 	fdset = NULL;
>@@ -1101,6 +1102,18 @@ server_accept_loop(int *sock_in, int *so
> 		startup_pipes[i] = -1;
> 
> 	/*
>+	 * Block signals except when in pselect.  This guarantees that we
>+	 * will wake up if a signal would have been received after we check
>+	 * the corresponding flag.
>+	 */
>+	sigemptyset(&nsigset);
>+	sigaddset(&nsigset, SIGHUP);
>+	sigaddset(&nsigset, SIGCHLD);
>+	sigaddset(&nsigset, SIGTERM);
>+	sigaddset(&nsigset, SIGQUIT);
>+	sigprocmask(SIG_BLOCK, &nsigset, &osigset);
>+
>+	/*
> 	 * Stay listening for connections until the system crashes or
> 	 * the daemon is killed with a signal.
> 	 */
>@@ -1130,10 +1143,10 @@ server_accept_loop(int *sock_in, int *so
> 			if (startup_pipes[i] != -1)
> 				FD_SET(startup_pipes[i], fdset);
> 
>-		/* Wait in select until there is a connection. */
>-		ret = select(maxfd+1, fdset, NULL, NULL, NULL);
>+		/* Wait until a connection arrives or a child exits. */
>+		ret = pselect(maxfd+1, fdset, NULL, NULL, NULL, &osigset);
> 		if (ret == -1 && errno != EINTR)
>-			error("select: %.100s", strerror(errno));
>+			error("pselect: %.100s", strerror(errno));
> 		if (received_sigterm) {
> 			logit("Received signal %d; terminating.",
> 			    (int) received_sigterm);
>@@ -1226,6 +1239,7 @@ server_accept_loop(int *sock_in, int *so
> 			 * Got connection.  Fork a child to handle it, unless
> 			 * we are in debugging mode.
> 			 */
>+			sigprocmask(SIG_SETMASK, &osigset, NULL);
> 			if (debug_flag) {
> 				/*
> 				 * In debugging mode.  Close the listening

Actions: View | Diff

Attachments on bug 2158: 3023 | 3520 | 3523