Attachment 3520 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.62 KB, created by Darren Tucker on 2021-05-14 15:14:59 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Darren Tucker

Created: 2021-05-14 15:14:59 AEST

Size: 8.62 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	14 May 2021 05:01:26 -0000
>@@ -84,12 +84,8 @@ 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. */
>+/* The child has terminated. */
>+static volatile sig_atomic_t child_terminated = 0;
> 
> /* Cleanup on signals (!use_privsep case only) */
> static volatile sig_atomic_t received_sigterm = 0;
>@@ -111,59 +107,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,7 +155,7 @@ client_alive_check(struct ssh *ssh)
> }
> 
> /*
>- * Sleep in select() until we can do something.  This will initialize the
>+ * Sleep in pselect() until we can do something.  This will initialize the
>  * select 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,9 +164,9 @@ 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;
>@@ -258,7 +206,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 +223,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 +253,6 @@ wait_until_can_do_something(struct ssh *
> 			last_client_time = now;
> 		}
> 	}
>-
>-	notify_done(*readsetp);
> }
> 
> /*
>@@ -365,17 +310,16 @@ process_buffered_input_packets(struct ss
> 	ssh_dispatch_run_fatal(ssh, DISPATCH_NONBLOCK, NULL);
> }
> 
>+/*
>+ * Reap child processes.  Because we block SIGCHLD except in pselect(),
>+ * this should not be subject to races.
>+ */
> 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 +328,6 @@ collect_children(struct ssh *ssh)
> 				session_close_by_pid(ssh, pid, status);
> 		child_terminated = 0;
> 	}
>-	sigprocmask(SIG_SETMASK, &oset, NULL);
> }
> 
> void
>@@ -392,13 +335,22 @@ server_loop2(struct ssh *ssh, Authctxt *
> {
> 	fd_set *readset = NULL, *writeset = NULL;
> 	int max_fd;
>+	sigset_t osigset, nsigset;
> 	u_int nalloc = 0, connection_in, connection_out;
> 	u_int64_t rekey_timeout_ms = 0;
> 
> 	debug("Entering interactive session for SSH2.");
> 
>+	/* Block CHLD/INT/TERM/QUIT except in pselect to prevent races. */
>+	sigemptyset(&nsigset);
>+	sigaddset(&nsigset, SIGCHLD);
>+	sigaddset(&nsigset, SIGINT);
>+	sigaddset(&nsigset, SIGTERM);
>+	sigaddset(&nsigset, SIGQUIT);
>+	sigprocmask(SIG_BLOCK, &nsigset, &osigset);
> 	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 +360,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);
> 
>@@ -430,7 +379,8 @@ server_loop2(struct ssh *ssh, Authctxt *
> 		}
> 
> 		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 (received_sigterm) {
> 			logit("Exiting on signal %d", (int)received_sigterm);
>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	14 May 2021 04:57:12 -0000
>@@ -1077,7 +1077,8 @@ server_listen(void)
>  * from this function are in a forked subprocess.
>  */
> static void
>-server_accept_loop(int *sock_in, int *sock_out, int *newsock, int *config_s)
>+server_accept_loop(int *sock_in, int *sock_out, int *newsock, int *config_s,
>+    sigset_t *sigmaskp)
> {
> 	fd_set *fdset;
> 	int i, j, ret, maxfd;
>@@ -1131,9 +1132,9 @@ server_accept_loop(int *sock_in, int *so
> 				FD_SET(startup_pipes[i], fdset);
> 
> 		/* Wait in select until there is a connection. */
>-		ret = select(maxfd+1, fdset, NULL, NULL, NULL);
>+		ret = pselect(maxfd+1, fdset, NULL, NULL, NULL, sigmaskp);
> 		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);
>@@ -1440,6 +1441,7 @@ main(int ac, char **av)
> 	int keytype;
> 	Authctxt *authctxt;
> 	struct connection_info *connection_info = NULL;
>+	sigset_t nsigset, osigset;
> 
> 	/* Save argv. */
> 	saved_argv = av;
>@@ -1897,6 +1899,13 @@ main(int ac, char **av)
> 	} else {
> 		server_listen();
> 
>+		/* Block signals except when in pselect to prevent races. */
>+		sigemptyset(&nsigset);
>+		sigaddset(&nsigset, SIGCHLD);
>+		sigaddset(&nsigset, SIGHUP);
>+		sigaddset(&nsigset, SIGTERM);
>+		sigaddset(&nsigset, SIGQUIT);
>+		sigprocmask(SIG_BLOCK, &nsigset, &osigset);	
> 		ssh_signal(SIGHUP, sighup_handler);
> 		ssh_signal(SIGCHLD, main_sigchld_handler);
> 		ssh_signal(SIGTERM, sigterm_handler);
>@@ -1920,10 +1929,11 @@ main(int ac, char **av)
> 
> 		/* Accept a connection and return in a forked child */
> 		server_accept_loop(&sock_in, &sock_out,
>-		    &newsock, config_s);
>+		    &newsock, config_s, &osigset);
> 	}
> 
> 	/* This is the child processing a new connection. */
>+	sigprocmask(SIG_SETMASK, &osigset, NULL);
> 	setproctitle("%s", "[accepted]");
> 
> 	/*

Actions: View | Diff

Attachments on bug 2158: 3023 | 3520 | 3523