- (djm) OpenBSD CVS Sync:

   - markus@cvs.openbsd.org  2001/01/29 09:55:37
     [channels.c channels.h clientloop.c serverloop.c]
     fix select overflow; ok deraadt@ and stevesk@

5 files changed, 114 insertions, 103 deletions

diff --git a/ChangeLog b/ChangeLog
index 0bda0f7e6..ce08540bc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+20000130
+ - (djm) OpenBSD CVS Sync:
+   - markus@cvs.openbsd.org  2001/01/29 09:55:37
+     [channels.c channels.h clientloop.c serverloop.c]
+     fix select overflow; ok deraadt@ and stevesk@
+
 20000129
  - (stevesk) sftp-server.c: use %lld vs. %qd
 
diff --git a/channels.c b/channels.c
index 8aaf8c6e6..6aafc3dc3 100644
--- a/channels.c
+++ b/channels.c
@@ -40,7 +40,7 @@
  */
 
 #include "includes.h"
-RCSID("$OpenBSD: channels.c,v 1.83 2001/01/24 21:03:50 stevesk Exp $");
+RCSID("$OpenBSD: channels.c,v 1.84 2001/01/29 16:55:36 markus Exp $");
 
 #include <openssl/rsa.h>
 #include <openssl/dsa.h>
@@ -84,7 +84,7 @@ static int channels_alloc = 0;
  * Maximum file descriptor value used in any of the channels.  This is
  * updated in channel_allocate.
  */
-static int channel_max_fd_value = 0;
+static int channel_max_fd = 0;
 
 /* Name and directory of socket for authentication agent forwarding. */
 static char *channel_forwarded_auth_socket_name = NULL;
@@ -181,12 +181,10 @@ channel_register_fds(Channel *c, int rfd, int wfd, int efd,
     int extusage, int nonblock)
 {
         /* Update the maximum file descriptor value. */
-        if (rfd > channel_max_fd_value)
-                channel_max_fd_value = rfd;
-        if (wfd > channel_max_fd_value)
-                channel_max_fd_value = wfd;
-        if (efd > channel_max_fd_value)
-                channel_max_fd_value = efd;
+        channel_max_fd = MAX(channel_max_fd, rfd);
+        channel_max_fd = MAX(channel_max_fd, wfd);
+        channel_max_fd = MAX(channel_max_fd, efd);
+
         /* XXX set close-on-exec -markus */
 
         c->rfd = rfd;
@@ -965,9 +963,27 @@ channel_handler(chan_fn *ftab[], fd_set * readset, fd_set * writeset)
 }
 
 void
-channel_prepare_select(fd_set * readset, fd_set * writeset)
+channel_prepare_select(fd_set **readsetp, fd_set **writesetp, int *maxfdp)
 {
-        channel_handler(channel_pre, readset, writeset);
+        int n;
+        u_int sz;
+
+        n = MAX(*maxfdp, channel_max_fd);
+
+        sz = howmany(n+1, NFDBITS) * sizeof(fd_mask);
+        if (*readsetp == NULL || n > *maxfdp) {
+                if (*readsetp)
+                        xfree(*readsetp);
+                if (*writesetp)
+                        xfree(*writesetp);
+                *readsetp = xmalloc(sz);
+                *writesetp = xmalloc(sz);
+                *maxfdp = n;
+        }
+        memset(*readsetp, 0, sz);
+        memset(*writesetp, 0, sz);
+
+        channel_handler(channel_pre, *readsetp, *writesetp);
 }
 
 void
@@ -976,7 +992,7 @@ channel_after_select(fd_set * readset, fd_set * writeset)
         channel_handler(channel_post, readset, writeset);
 }
 
-/* If there is data to send to the connection, send some of it now. */
+/* If there is data to send to the connection, enqueue some of it now. */
 
 void
 channel_output_poll()
@@ -1417,14 +1433,6 @@ channel_close_all()
                         channel_close_fds(&channels[i]);
 }
 
-/* Returns the maximum file descriptor number used by the channels. */
-
-int
-channel_max_fd()
-{
-        return channel_max_fd_value;
-}
-
 /* Returns true if any channel is still open. */
 
 int
diff --git a/channels.h b/channels.h
index 45b783fb3..5e030a44b 100644
--- a/channels.h
+++ b/channels.h
@@ -32,7 +32,7 @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
-/* RCSID("$OpenBSD: channels.h,v 1.24 2000/12/05 20:34:10 markus Exp $"); */
+/* RCSID("$OpenBSD: channels.h,v 1.25 2001/01/29 16:55:36 markus Exp $"); */
 
 #ifndef CHANNELS_H
 #define CHANNELS_H
@@ -163,8 +163,12 @@ int     channel_allocate(int type, int sock, char *remote_name);
 /* Free the channel and close its socket. */
 void    channel_free(int channel);
 
-/* Add any bits relevant to channels in select bitmasks. */
-void    channel_prepare_select(fd_set * readset, fd_set * writeset);
+/*
+ * Allocate/update select bitmasks and add any bits relevant to channels in
+ * select bitmasks.
+ */
+void
+channel_prepare_select(fd_set **readsetp, fd_set **writesetp, int *maxfdp);
 
 /*
  * After select, perform any appropriate operations for channels which have
@@ -188,9 +192,6 @@ void    channel_stop_listening(void);
  */
 void    channel_close_all(void);
 
-/* Returns the maximum file descriptor number used by the channels. */
-int     channel_max_fd(void);
-
 /* Returns true if there is still an open channel over the connection. */
 int     channel_still_open(void);
 
diff --git a/clientloop.c b/clientloop.c
index aade8606b..49a943a73 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -59,7 +59,7 @@
  */
 
 #include "includes.h"
-RCSID("$OpenBSD: clientloop.c,v 1.45 2001/01/21 19:05:47 markus Exp $");
+RCSID("$OpenBSD: clientloop.c,v 1.46 2001/01/29 16:55:36 markus Exp $");
 
 #include "ssh.h"
 #include "ssh1.h"
@@ -124,7 +124,6 @@ static Buffer stdout_buffer;	/* Buffer for stdout data. */
 static Buffer stderr_buffer;    /* Buffer for stderr data. */
 static u_long stdin_bytes, stdout_bytes, stderr_bytes;
 static u_int buffer_high;/* Soft max buffer size. */
-static int max_fd;              /* Maximum file descriptor number in select(). */
 static int connection_in;       /* Connection to server (input). */
 static int connection_out;      /* Connection to server (output). */
 
@@ -365,45 +364,37 @@ client_check_window_change()
  */
 
 void
-client_wait_until_can_do_something(fd_set * readset, fd_set * writeset)
+client_wait_until_can_do_something(fd_set **readsetp, fd_set **writesetp,
+    int *maxfdp)
 {
-        /* Initialize select masks. */
-        FD_ZERO(readset);
-        FD_ZERO(writeset);
+        /* Add any selections by the channel mechanism. */
+        channel_prepare_select(readsetp, writesetp, maxfdp);
 
         if (!compat20) {
                 /* Read from the connection, unless our buffers are full. */
                 if (buffer_len(&stdout_buffer) < buffer_high &&
                     buffer_len(&stderr_buffer) < buffer_high &&
                     channel_not_very_much_buffered_data())
-                        FD_SET(connection_in, readset);
+                        FD_SET(connection_in, *readsetp);
                 /*
                  * Read from stdin, unless we have seen EOF or have very much
                  * buffered data to send to the server.
                  */
                 if (!stdin_eof && packet_not_very_much_data_to_write())
-                        FD_SET(fileno(stdin), readset);
+                        FD_SET(fileno(stdin), *readsetp);
 
                 /* Select stdout/stderr if have data in buffer. */
                 if (buffer_len(&stdout_buffer) > 0)
-                        FD_SET(fileno(stdout), writeset);
+                        FD_SET(fileno(stdout), *writesetp);
                 if (buffer_len(&stderr_buffer) > 0)
-                        FD_SET(fileno(stderr), writeset);
+                        FD_SET(fileno(stderr), *writesetp);
         } else {
-                FD_SET(connection_in, readset);
+                FD_SET(connection_in, *readsetp);
         }
 
-        /* Add any selections by the channel mechanism. */
-        channel_prepare_select(readset, writeset);
-
         /* Select server connection if have data to write to the server. */
         if (packet_have_data_to_write())
-                FD_SET(connection_out, writeset);
-
-/* move UP XXX */
-        /* Update maximum file descriptor number, if appropriate. */
-        if (channel_max_fd() > max_fd)
-                max_fd = channel_max_fd();
+                FD_SET(connection_out, *writesetp);
 
         /*
          * Wait for something to happen.  This will suspend the process until
@@ -414,11 +405,8 @@ client_wait_until_can_do_something(fd_set * readset, fd_set * writeset)
          * SSH_MSG_IGNORE packet when the timeout expires.
          */
 
-        if (select(max_fd + 1, readset, writeset, NULL, NULL) < 0) {
+        if (select((*maxfdp)+1, *readsetp, *writesetp, NULL, NULL) < 0) {
                 char buf[100];
-                /* Some systems fail to clear these automatically. */
-                FD_ZERO(readset);
-                FD_ZERO(writeset);
                 if (errno == EINTR)
                         return;
                 /* Note: we might still have data in the buffers. */
@@ -797,6 +785,8 @@ simple_escape_filter(Channel *c, char *buf, int len)
 int
 client_loop(int have_pty, int escape_char_arg, int ssh2_chan_id)
 {
+        fd_set *readset = NULL, *writeset = NULL;
+        int max_fd = 0;
         double start_time, total_time;
         int len;
         char buf[100];
@@ -813,9 +803,13 @@ client_loop(int have_pty, int escape_char_arg, int ssh2_chan_id)
         buffer_high = 64 * 1024;
         connection_in = packet_get_connection_in();
         connection_out = packet_get_connection_out();
-        max_fd = connection_in;
-        if (connection_out > max_fd)
-                max_fd = connection_out;
+        max_fd = MAX(connection_in, connection_out);
+
+        if (!compat20) {
+                max_fd = MAX(max_fd, fileno(stdin));
+                max_fd = MAX(max_fd, fileno(stdout));
+                max_fd = MAX(max_fd, fileno(stderr));
+        }
         stdin_bytes = 0;
         stdout_bytes = 0;
         stderr_bytes = 0;
@@ -849,7 +843,6 @@ client_loop(int have_pty, int escape_char_arg, int ssh2_chan_id)
 
         /* Main loop of the client for the interactive session mode. */
         while (!quit_pending) {
-                fd_set readset, writeset;
 
                 /* Process buffered packets sent by the server. */
                 client_process_buffered_input_packets();
@@ -867,7 +860,7 @@ client_loop(int have_pty, int escape_char_arg, int ssh2_chan_id)
                         client_make_packets_from_stdin_data();
 
                 /*
-                 * Make packets from buffered channel data, and buffer them
+                 * Make packets from buffered channel data, and enqueue them
                  * for sending to the server.
                  */
                 if (packet_not_very_much_data_to_write())
@@ -886,34 +879,38 @@ client_loop(int have_pty, int escape_char_arg, int ssh2_chan_id)
                  * Wait until we have something to do (something becomes
                  * available on one of the descriptors).
                  */
-                client_wait_until_can_do_something(&readset, &writeset);
+                client_wait_until_can_do_something(&readset, &writeset, &max_fd);
 
                 if (quit_pending)
                         break;
 
                 /* Do channel operations. */
-                channel_after_select(&readset, &writeset);
+                channel_after_select(readset, writeset);
 
                 /* Buffer input from the connection.  */
-                client_process_net_input(&readset);
+                client_process_net_input(readset);
 
                 if (quit_pending)
                         break;
 
                 if (!compat20) {
                         /* Buffer data from stdin */
-                        client_process_input(&readset);
+                        client_process_input(readset);
                         /*
                          * Process output to stdout and stderr.  Output to
                          * the connection is processed elsewhere (above).
                          */
-                        client_process_output(&writeset);
+                        client_process_output(writeset);
                 }
 
                 /* Send as much buffered packet data as possible to the sender. */
-                if (FD_ISSET(connection_out, &writeset))
+                if (FD_ISSET(connection_out, writeset))
                         packet_write_poll();
         }
+        if (readset)
+                xfree(readset);
+        if (writeset)
+                xfree(writeset);
 
         /* Terminate the session. */
 
diff --git a/serverloop.c b/serverloop.c
index a7f8e72b5..bdac6a0d2 100644
--- a/serverloop.c
+++ b/serverloop.c
@@ -35,7 +35,7 @@
  */
 
 #include "includes.h"
-RCSID("$OpenBSD: serverloop.c,v 1.42 2001/01/21 19:05:55 markus Exp $");
+RCSID("$OpenBSD: serverloop.c,v 1.43 2001/01/29 16:55:37 markus Exp $");
 
 #include "xmalloc.h"
 #include "packet.h"
@@ -73,7 +73,6 @@ static int fderr_eof = 0;	/* EOF encountered readung from fderr. */
 static int connection_in;       /* Connection to client (input). */
 static int connection_out;      /* Connection to client (output). */
 static u_int buffer_high;/* "Soft" max buffer size. */
-static int max_fd;              /* Max file descriptor number for select(). */
 
 /*
  * This SIGCHLD kludge is used to detect when the child exits.  The server
@@ -180,8 +179,8 @@ make_packets_from_stdout_data()
  * for the duration of the wait (0 = infinite).
  */
 void
-wait_until_can_do_something(fd_set * readset, fd_set * writeset,
-                            u_int max_time_milliseconds)
+wait_until_can_do_something(fd_set **readsetp, fd_set **writesetp, int *maxfdp,
+    u_int max_time_milliseconds)
 {
         struct timeval tv, *tvp;
         int ret;
@@ -189,14 +188,13 @@ wait_until_can_do_something(fd_set * readset, fd_set * writeset,
         /* When select fails we restart from here. */
 retry_select:
 
-        /* Initialize select() masks. */
-        FD_ZERO(readset);
-        FD_ZERO(writeset);
+        /* Allocate and update select() masks for channel descriptors. */
+        channel_prepare_select(readsetp, writesetp, maxfdp);
 
         if (compat20) {
                 /* wrong: bad condition XXX */
                 if (channel_not_very_much_buffered_data())
-                        FD_SET(connection_in, readset);
+                        FD_SET(connection_in, *readsetp);
         } else {
                 /*
                  * Read packets from the client unless we have too much
@@ -204,37 +202,31 @@ retry_select:
                  */
                 if (buffer_len(&stdin_buffer) < buffer_high &&
                     channel_not_very_much_buffered_data())
-                        FD_SET(connection_in, readset);
+                        FD_SET(connection_in, *readsetp);
                 /*
                  * If there is not too much data already buffered going to
                  * the client, try to get some more data from the program.
                  */
                 if (packet_not_very_much_data_to_write()) {
                         if (!fdout_eof)
-                                FD_SET(fdout, readset);
+                                FD_SET(fdout, *readsetp);
                         if (!fderr_eof)
-                                FD_SET(fderr, readset);
+                                FD_SET(fderr, *readsetp);
                 }
                 /*
                  * If we have buffered data, try to write some of that data
                  * to the program.
                  */
                 if (fdin != -1 && buffer_len(&stdin_buffer) > 0)
-                        FD_SET(fdin, writeset);
+                        FD_SET(fdin, *writesetp);
         }
-        /* Set masks for channel descriptors. */
-        channel_prepare_select(readset, writeset);
 
         /*
          * If we have buffered packet data going to the client, mark that
          * descriptor.
          */
         if (packet_have_data_to_write())
-                FD_SET(connection_out, writeset);
-
-        /* Update the maximum descriptor number if appropriate. */
-        if (channel_max_fd() > max_fd)
-                max_fd = channel_max_fd();
+                FD_SET(connection_out, *writesetp);
 
         /*
          * If child has terminated and there is enough buffer space to read
@@ -255,7 +247,7 @@ retry_select:
                 debug2("tvp!=NULL kid %d mili %d", child_terminated, max_time_milliseconds);
 
         /* Wait for something to happen, or the timeout to expire. */
-        ret = select(max_fd + 1, readset, writeset, NULL, tvp);
+        ret = select((*maxfdp)+1, *readsetp, *writesetp, NULL, tvp);
 
         if (ret < 0) {
                 if (errno != EINTR)
@@ -400,7 +392,8 @@ process_buffered_input_packets()
 void
 server_loop(pid_t pid, int fdin_arg, int fdout_arg, int fderr_arg)
 {
-        fd_set readset, writeset;
+        fd_set *readset = NULL, *writeset = NULL;
+        int max_fd;
         int wait_status;        /* Status returned by wait(). */
         pid_t wait_pid;         /* pid returned by wait(). */
         int waiting_termination = 0;    /* Have displayed waiting close message. */
@@ -441,15 +434,11 @@ server_loop(pid_t pid, int fdin_arg, int fdout_arg, int fderr_arg)
                 buffer_high = 64 * 1024;
 
         /* Initialize max_fd to the maximum of the known file descriptors. */
-        max_fd = fdin;
-        if (fdout > max_fd)
-                max_fd = fdout;
-        if (fderr != -1 && fderr > max_fd)
-                max_fd = fderr;
-        if (connection_in > max_fd)
-                max_fd = connection_in;
-        if (connection_out > max_fd)
-                max_fd = connection_out;
+        max_fd = MAX(fdin, fdout);
+        if (fderr != -1)
+                max_fd = MAX(max_fd, fderr);
+        max_fd = MAX(max_fd, connection_in);
+        max_fd = MAX(max_fd, connection_out);
 
         /* Initialize Initialize buffers. */
         buffer_init(&stdin_buffer);
@@ -536,18 +525,22 @@ server_loop(pid_t pid, int fdin_arg, int fdout_arg, int fderr_arg)
                         }
                 }
                 /* Sleep in select() until we can do something. */
-                wait_until_can_do_something(&readset, &writeset,
-                                            max_time_milliseconds);
+                wait_until_can_do_something(&readset, &writeset, &max_fd,
+                    max_time_milliseconds);
 
                 /* Process any channel events. */
-                channel_after_select(&readset, &writeset);
+                channel_after_select(readset, writeset);
 
                 /* Process input from the client and from program stdout/stderr. */
-                process_input(&readset);
+                process_input(readset);
 
                 /* Process output to the client and to program stdin. */
-                process_output(&writeset);
+                process_output(writeset);
         }
+        if (readset)
+                xfree(readset);
+        if (writeset)
+                xfree(writeset);
 
         /* Cleanup and termination code. */
 
@@ -638,7 +631,8 @@ server_loop(pid_t pid, int fdin_arg, int fdout_arg, int fderr_arg)
 void
 server_loop2(void)
 {
-        fd_set readset, writeset;
+        fd_set *readset = NULL, *writeset = NULL;
+        int max_fd;
         int had_channel = 0;
         int status;
         pid_t pid;
@@ -650,9 +644,9 @@ server_loop2(void)
         child_terminated = 0;
         connection_in = packet_get_connection_in();
         connection_out = packet_get_connection_out();
-        max_fd = connection_in;
-        if (connection_out > max_fd)
-                max_fd = connection_out;
+
+        max_fd = MAX(connection_in, connection_out);
+
         server_init_dispatch();
 
         for (;;) {
@@ -665,16 +659,21 @@ server_loop2(void)
                 }
                 if (packet_not_very_much_data_to_write())
                         channel_output_poll();
-                wait_until_can_do_something(&readset, &writeset, 0);
+                wait_until_can_do_something(&readset, &writeset, &max_fd, 0);
                 if (child_terminated) {
                         while ((pid = waitpid(-1, &status, WNOHANG)) > 0)
                                 session_close_by_pid(pid, status);
                         child_terminated = 0;
                 }
-                channel_after_select(&readset, &writeset);
-                process_input(&readset);
-                process_output(&writeset);
+                channel_after_select(readset, writeset);
+                process_input(readset);
+                process_output(writeset);
         }
+        if (readset)
+                xfree(readset);
+        if (writeset)
+                xfree(writeset);
+
         signal(SIGCHLD, SIG_DFL);
         while ((pid = waitpid(-1, &status, WNOHANG)) > 0)
                 session_close_by_pid(pid, status);