sshd gets stuck: select() in packet_read_seqnr waits indefinitely
Darren Tucker
dtucker at zip.com.au
Thu Mar 15 11:14:16 EST 2007
On Wed, Mar 14, 2007 at 12:53:09PM -0600, Matt Day wrote:
> Dear OpenSSH Portable sshd developers,
>
> I'm having a problem where sshd login sessions are occasionally
> (as often as once a day) getting stuck indefinitely. I enabled debug
> messages and got a backtrace of a stuck sshd, and I think I've found
> the bug. I wanted to run it by the list once before filing.
[...]
> The select() in packet_read_seqnr() waits indefinitely, resulting
> in stuck SSH sessions when networking problems interfere with
> key exchange. Would like to be able to set a timeout there, or
> send SSH keepalives during key exchange.
[...]
> The select call in packet_read_seqnr passes NULL for a timeout,
> meaning it will wait forever. That explains why the comment above
> it says "Note that no other data is processed until this returns,
> so this function should not be used during the interactive session."
> But, this was an interactive session.
>
> I've set ClientAliveInterval in sshd_config so that SSH sessions
> die in a timely manner when networking problems arise, but the
> keepalive is apparently not sent during key exchange. The default
> TCP keepalive on FreeBSD is unhelpful here; it only kicks in after
> 2 hours, and I need stuck SSH sessions to die a lot sooner. I want
> to keep the FreeBSD TCP keepalive defaults.
>
> Would it be possible for the select() in packet_read_seqnr to use
> an optional timeout? Similarly, I believe the select() in
> packet_write_wait has the same problem. Upon timeout, it would be
> fine with me if the session died with an error logged.
You could try the attached patch.
The timeout should perhaps be ClientAliveInterval * ClientAliveCountMax,
(though you'd need to be wary of integer overflow).
> Alternatively,
> if SSH keepalives were sent during key exchange, that would suffice.
You're not allowed to send most types of packets (this would include
keepalives) once you've initiated a key exchange until it completes.
Index: packet.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh/packet.c,v
retrieving revision 1.146
diff -u -p -r1.146 packet.c
--- packet.c 17 Jan 2007 00:00:14 -0000 1.146
+++ packet.c 14 Mar 2007 23:48:23 -0000
@@ -136,6 +136,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 for (or to send) 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 {
@@ -166,7 +170,7 @@ TAILQ_HEAD(, packet) outgoing;
* packet_set_encryption_key is called.
*/
void
-packet_set_connection(int fd_in, int fd_out)
+packet_set_connection(int fd_in, int fd_out, int timeout)
{
Cipher *none = cipher_by_name("none");
@@ -187,6 +191,15 @@ packet_set_connection(int fd_in, int fd_
buffer_init(&incoming_packet);
TAILQ_INIT(&outgoing);
}
+
+ if (timeout == 0)
+ packet_wait_tvp = NULL;
+ else {
+ packet_wait_tv.tv_sec = timeout;
+ packet_wait_tv.tv_usec = 0;
+ packet_wait_tvp = &packet_wait_tv;
+ }
+
}
/* Returns 1 if remote host is connected via socket, 0 if not. */
@@ -923,7 +936,8 @@ 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 (select(connection_in + 1, setp, NULL, NULL,
+ packet_wait_tvp) == -1 &&
(errno == EAGAIN || errno == EINTR))
;
@@ -1449,7 +1463,8 @@ 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 (select(connection_out + 1, NULL, setp, NULL,
+ packet_wait_tvp) == -1 &&
(errno == EAGAIN || errno == EINTR))
;
packet_write_poll();
Index: packet.h
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh/packet.h,v
retrieving revision 1.47
diff -u -p -r1.47 packet.h
--- packet.h 26 Mar 2006 03:30:02 -0000 1.47
+++ packet.h 14 Mar 2007 23:48:38 -0000
@@ -20,7 +20,7 @@
#include <openssl/bn.h>
-void packet_set_connection(int, int);
+void packet_set_connection(int, int, int);
void packet_set_nonblocking(void);
int packet_get_connection_in(void);
int packet_get_connection_out(void);
Index: ssh-keyscan.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh/ssh-keyscan.c,v
retrieving revision 1.91
diff -u -p -r1.91 ssh-keyscan.c
--- ssh-keyscan.c 23 Oct 2006 17:01:16 -0000 1.91
+++ ssh-keyscan.c 14 Mar 2007 23:50:23 -0000
@@ -358,7 +358,7 @@ keygrab_ssh2(con *c)
{
int j;
- packet_set_connection(c->c_fd, c->c_fd);
+ packet_set_connection(c->c_fd, c->c_fd, timeout);
enable_compat20();
myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = c->c_keytype == KT_DSA?
"ssh-dss": "ssh-rsa";
Index: sshconnect.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh/sshconnect.c,v
retrieving revision 1.171
diff -u -p -r1.171 sshconnect.c
--- sshconnect.c 23 Oct 2006 17:02:24 -0000 1.171
+++ sshconnect.c 14 Mar 2007 23:46:27 -0000
@@ -157,7 +157,7 @@ ssh_proxy_connect(const char *host, u_sh
xfree(command_string);
/* Set the connection file descriptors. */
- packet_set_connection(pout[0], pin[1]);
+ packet_set_connection(pout[0], pin[1], options.server_alive_interval);
/* Indicate OK return */
return 0;
@@ -385,7 +385,7 @@ ssh_connect(const char *host, struct soc
error("setsockopt SO_KEEPALIVE: %.100s", strerror(errno));
/* Set the connection. */
- packet_set_connection(sock, sock);
+ packet_set_connection(sock, sock, options.server_alive_interval);
return 0;
}
Index: sshd.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh/sshd.c,v
retrieving revision 1.362
diff -u -p -r1.362 sshd.c
--- sshd.c 25 Feb 2007 09:37:22 -0000 1.362
+++ sshd.c 14 Mar 2007 23:45:09 -0000
@@ -1707,7 +1707,7 @@ main(int ac, char **av)
* Register our connection. This turns encryption off because we do
* not have a key.
*/
- packet_set_connection(sock_in, sock_out);
+ packet_set_connection(sock_in, sock_out, options.client_alive_interval);
packet_set_server();
/* Set SO_KEEPALIVE if requested. */
--
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4 37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
More information about the openssh-unix-dev
mailing list