From 5e953217f13b340d8a5fbcd771a1dbaf43354f20 Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Tue, 30 Jan 2001 09:14:00 +1100 Subject: [PATCH] - (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@ --- ChangeLog | 6 ++++ channels.c | 46 ++++++++++++++++------------ channels.h | 13 ++++---- clientloop.c | 67 ++++++++++++++++++++--------------------- serverloop.c | 85 ++++++++++++++++++++++++++-------------------------- 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 #include @@ -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);