OpenSSH
[Top] [All Lists]

Re: sshd gets stuck: select() in packet_read_seqnr waits indefinitely

To: openssh-unix-dev@mindrot.org
Subject: Re: sshd gets stuck: select() in packet_read_seqnr waits indefinitely
From: Matt Day <opensshbugs@fjarlq.com>
Date: Thu, 15 Mar 2007 02:40:12 -0600
Delivered-to: sp-com-lists@consult.net
Delivered-to: openssh-unix-dev-list1@securepoint.com
Delivered-to: openssh-unix-dev-tmda@mindrot.org
Delivered-to: openssh-unix-dev@mindrot.org
In-reply-to: <20070315055242.GA15006@gate.dtucker.net>
List-archive: <http://lists.mindrot.org/pipermail/openssh-unix-dev>
List-help: <mailto:openssh-unix-dev-request@mindrot.org?subject=help>
List-id: Development of portable OpenSSH <openssh-unix-dev.mindrot.org>
List-post: <mailto:openssh-unix-dev@mindrot.org>
List-subscribe: <http://lists.mindrot.org/mailman/listinfo/openssh-unix-dev>, <mailto:openssh-unix-dev-request@mindrot.org?subject=subscribe>
List-unsubscribe: <http://lists.mindrot.org/mailman/listinfo/openssh-unix-dev>, <mailto:openssh-unix-dev-request@mindrot.org?subject=unsubscribe>
References: <20070314185309.GA40932@fjarlq.com> <20070315001416.GA28880@gate.dtucker.net> <20070315011208.GA48513@fjarlq.com> <20070315024347.GA10841@gate.dtucker.net> <20070315025904.GA50787@fjarlq.com> <20070315032213.GA12369@gate.dtucker.net> <20070315055242.GA15006@gate.dtucker.net>
Sender: openssh-unix-dev-bounces+openssh-unix-dev-list1=securepoint.com@mindrot.org
User-agent: Mutt/1.4.2.1i
On Thu, Mar 15, 2007 at 04:52:42PM +1100, Darren Tucker wrote:
> Like so (still untested):
> 
> [..]

Awesome, thanks!

I applied the patch and ran into one problem during testing: outbound
ssh connections immediately received the new "timed out" error.

The problem was in packet_set_timeout: packet_wait_tvp was only
getting set to NULL when *both* timeout and count were 0. The default
is interval=0 count=3, so it used the timeval with tv_sec = 0.

I fixed that (use || instead of && in packet_set_timeout) and also
made the two new "timed out" error messages unique. That might be
handy someday if one is wondering if it timed out waiting to read
or to write (which I found myself wondering while debugging this).

I have installed these changes on my server and will let you know
when I get confirmation in my logfile that the fix is working.

Thank you very much!
Matt

===================================================================
diff -u -p packet.c.orig packet.c
--- packet.c.orig       Sun Sep 11 09:50:34 2005
+++ packet.c    Thu Mar 15 01:08:22 2007
@@ -122,6 +122,10 @@ static int server_side = 0;
 /* Set to true if we are authenticated. */
 static int after_authentication = 0;
 
+/* Set to the maximum time that we will wait to send or receive a packet */
+static struct timeval packet_wait_tv;
+static struct timeval *packet_wait_tvp = NULL;
+
 /* Session key information for Encryption and MAC */
 Newkeys *newkeys[MODE_MAX];
 static struct packet_state {
@@ -175,6 +179,21 @@ packet_set_connection(int fd_in, int fd_
        }
 }
 
+void
+packet_set_timeout(int timeout, int count)
+{
+       if (timeout == 0 || count == 0) {
+               packet_wait_tvp = NULL;
+               return;
+       }
+       if (LONG_MAX / count < timeout)
+               packet_wait_tv.tv_sec = LONG_MAX;
+       else
+               packet_wait_tv.tv_sec = timeout * count;
+       packet_wait_tv.tv_usec = 0;
+       packet_wait_tvp = &packet_wait_tv;
+}
+
 /* Returns 1 if remote host is connected via socket, 0 if not. */
 
 int
@@ -863,7 +882,7 @@ packet_send(void)
 int
 packet_read_seqnr(u_int32_t *seqnr_p)
 {
-       int type, len;
+       int type, len, ret;
        fd_set *setp;
        char buf[8192];
        DBG(debug("packet_read()"));
@@ -898,10 +917,15 @@ packet_read_seqnr(u_int32_t *seqnr_p)
                FD_SET(connection_in, setp);
 
                /* Wait for some data to arrive. */
-               while (select(connection_in + 1, setp, NULL, NULL, NULL) == -1 
&&
+               while ((ret = select(connection_in + 1, setp, NULL, NULL,
+                   packet_wait_tvp)) == -1 &&
                    (errno == EAGAIN || errno == EINTR))
                        ;
-
+               if (ret == 0) {
+                       logit("Connection to %.200s timed out while waiting to 
read",
+                           get_remote_ipaddr());
+                       cleanup_exit(255);
+               }
                /* Read data from the socket. */
                len = read(connection_in, buf, sizeof(buf));
                if (len == 0) {
@@ -1411,6 +1435,7 @@ void
 packet_write_wait(void)
 {
        fd_set *setp;
+       int ret;
 
        setp = (fd_set *)xmalloc(howmany(connection_out + 1, NFDBITS) *
            sizeof(fd_mask));
@@ -1419,9 +1444,15 @@ packet_write_wait(void)
                memset(setp, 0, howmany(connection_out + 1, NFDBITS) *
                    sizeof(fd_mask));
                FD_SET(connection_out, setp);
-               while (select(connection_out + 1, NULL, setp, NULL, NULL) == -1 
&&
+               while ((ret = select(connection_out + 1, NULL, setp, NULL,
+                   packet_wait_tvp)) == -1 &&
                    (errno == EAGAIN || errno == EINTR))
                        ;
+               if (ret == 0) {
+                       logit("Connection to %.200s timed out while waiting to 
write",
+                           get_remote_ipaddr());
+                       cleanup_exit(255);
+               }
                packet_write_poll();
        }
        xfree(setp);
===================================================================
diff -u -p packet.h.orig packet.h
--- packet.h.orig       Sun Sep 11 09:50:34 2005
+++ packet.h    Thu Mar 15 00:05:29 2007
@@ -19,6 +19,7 @@
 #include <openssl/bn.h>
 
 void     packet_set_connection(int, int);
+void     packet_set_timeout(int, int);
 void     packet_set_nonblocking(void);
 int      packet_get_connection_in(void);
 int      packet_get_connection_out(void);
===================================================================
diff -u -p sshconnect.c.orig sshconnect.c
--- sshconnect.c.orig   Sun Sep 11 09:50:35 2005
+++ sshconnect.c        Thu Mar 15 00:05:29 2007
@@ -139,6 +139,8 @@ ssh_proxy_connect(const char *host, u_sh
 
        /* Set the connection file descriptors. */
        packet_set_connection(pout[0], pin[1]);
+       packet_set_timeout(options.server_alive_interval,
+           options.server_alive_count_max);
 
        /* Indicate OK return */
        return 0;
@@ -381,6 +383,8 @@ ssh_connect(const char *host, struct soc
 
        /* Set the connection. */
        packet_set_connection(sock, sock);
+       packet_set_timeout(options.server_alive_interval,
+           options.server_alive_count_max);
 
        return 0;
 }
===================================================================
diff -u -p sshd.c.orig sshd.c
--- sshd.c.orig Sun Sep 11 09:50:35 2005
+++ sshd.c      Thu Mar 15 00:05:29 2007
@@ -1643,6 +1643,8 @@ main(int ac, char **av)
         * not have a key.
         */
        packet_set_connection(sock_in, sock_out);
+       packet_set_timeout(options.client_alive_interval,
+           options.client_alive_count_max);
        packet_set_server();
 
        /* Set SO_KEEPALIVE if requested. */
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
http://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

<Prev in Thread] Current Thread [Next in Thread>