- markus@cvs.openbsd.org 2001/12/19 16:09:39

     [serverloop.c]
     fix race between SIGCHLD and select with an additional pipe.  writing
     to the pipe on SIGCHLD wakes up select(). using pselect() is not
     portable and siglongjmp() ugly. W. R. Stevens suggests similar solution.
     initial idea by pmenage@ensim.com; ok deraadt@, djm@

2 files changed, 64 insertions, 2 deletions

diff --git a/ChangeLog b/ChangeLog
index 3b9814f5..7b821250 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -38,6 +38,12 @@
      [sshconnect1.c sshconnect2.c sshconnect.c sshd.8 sshd.c sshd_config]
      [ssh-keygen.c sshlogin.c sshpty.c sshtty.c ttymodes.c uidswap.c]
      basic KNF done while i was looking for something else
+   - markus@cvs.openbsd.org 2001/12/19 16:09:39
+     [serverloop.c]
+     fix race between SIGCHLD and select with an additional pipe.  writing
+     to the pipe on SIGCHLD wakes up select(). using pselect() is not
+     portable and siglongjmp() ugly. W. R. Stevens suggests similar solution.
+     initial idea by pmenage@ensim.com; ok deraadt@, djm@
 
 20011219
  - (stevesk) OpenBSD CVS sync X11 localhost display
@@ -7066,4 +7072,4 @@
  - Wrote replacements for strlcpy and mkdtemp
  - Released 1.0pre1
 
-$Id: ChangeLog,v 1.1701 2001/12/21 03:45:46 djm Exp $
+$Id: ChangeLog,v 1.1702 2001/12/21 03:53:11 djm Exp $
diff --git a/serverloop.c b/serverloop.c
index 1fa1f580..c876dc0c 100644
--- a/serverloop.c
+++ b/serverloop.c
@@ -35,7 +35,7 @@
  */
 
 #include "includes.h"
-RCSID("$OpenBSD: serverloop.c,v 1.86 2001/12/19 07:18:56 deraadt Exp $");
+RCSID("$OpenBSD: serverloop.c,v 1.87 2001/12/19 16:09:39 markus Exp $");
 
 #include "xmalloc.h"
 #include "packet.h"
@@ -92,6 +92,51 @@ static volatile sig_atomic_t child_terminated = 0;	/* The child has terminated.
 /* prototypes */
 static void server_init_dispatch(void);
 
+/*
+ * 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) < 0) {
+		error("pipe(notify_pipe) failed %s", strerror(errno));
+	} else if ((fcntl(notify_pipe[0], F_SETFD, 1) == -1) ||
+	    (fcntl(notify_pipe[1], F_SETFD, 1) == -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)
+		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("notify_done: reading");
+}
+
 static void
 sigchld_handler(int sig)
 {
@@ -99,6 +144,7 @@ sigchld_handler(int sig)
 	debug("Received SIGCHLD.");
 	child_terminated = 1;
 	mysignal(SIGCHLD, sigchld_handler);
+	notify_parent();
 	errno = save_errno;
 }
 
@@ -242,6 +288,7 @@ wait_until_can_do_something(fd_set **readsetp, fd_set **writesetp, int *maxfdp,
 		if (fdin != -1 && buffer_len(&stdin_buffer) > 0)
 			FD_SET(fdin, *writesetp);
 	}
+	notify_prepare(*readsetp);
 
 	/*
 	 * If we have buffered packet data going to the client, mark that
@@ -279,6 +326,8 @@ wait_until_can_do_something(fd_set **readsetp, fd_set **writesetp, int *maxfdp,
 			error("select: %.100s", strerror(errno));
 	} else if (ret == 0 && client_alive_scheduled)
 		client_alive_check();
+
+	notify_done(*readsetp);
 }
 
 /*
@@ -468,6 +517,8 @@ server_loop(pid_t pid, int fdin_arg, int fdout_arg, int fderr_arg)
 	connection_in = packet_get_connection_in();
 	connection_out = packet_get_connection_out();
 
+	notify_setup();
+
 	previous_stdout_buffer_bytes = 0;
 
 	/* Set approximate I/O buffer size. */
@@ -573,6 +624,7 @@ server_loop(pid_t pid, int fdin_arg, int fdout_arg, int fderr_arg)
 		max_fd = MAX(max_fd, fdin);
 		max_fd = MAX(max_fd, fdout);
 		max_fd = MAX(max_fd, fderr);
+		max_fd = MAX(max_fd, notify_pipe[0]);
 
 		/* Sleep in select() until we can do something. */
 		wait_until_can_do_something(&readset, &writeset, &max_fd,
@@ -697,7 +749,11 @@ server_loop2(Authctxt *authctxt)
 	connection_in = packet_get_connection_in();
 	connection_out = packet_get_connection_out();
 
+	notify_setup();
+
 	max_fd = MAX(connection_in, connection_out);
+	max_fd = MAX(max_fd, notify_pipe[0]);
+
 	xxx_authctxt = authctxt;
 
 	server_init_dispatch();