bug & patch in ServerAliveInterval (openssh 4.3-p2)

[Darren Tucker]

On Thu, May 04, 2006 at 10:59:48AM +0300, catamus wrote:
> On Thu, 4 May 2006, Darren Tucker wrote:
> >Could you please give an example of the circumstances under which this
> >might happen?  Repeatedly banging on the keyboard generating traffic
> >on stdin?
> 
> Well, yes keyboard is one, forwarded tcp is other.. (anything that
> make select return before timeout expire)

Makes sense, thanks.

> I think only
> 	if(FD_ISSET(connection_in, *readsetp))
> 
> we reset the timer when we have data from remote peer..

Yeah that seems reasonable.  The other thing is that the diff sets a
select timeout even with Protocol 1 (not a big deal but not necessary).

Index: clientloop.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/clientloop.c,v
retrieving revision 1.162
diff -u -p -r1.162 clientloop.c
--- clientloop.c	2006/04/20 09:27:09	1.162
+++ clientloop.c	2006/05/04 08:23:09
@@ -465,7 +465,8 @@ static void
 client_wait_until_can_do_something(fd_set **readsetp, fd_set **writesetp,
     int *maxfdp, u_int *nallocp, int rekeying)
 {
-	struct timeval tv, *tvp;
+	static struct timeval tv;
+	struct timeval *tvp = NULL;
 	int ret;
 
 	/* Add any selections by the channel mechanism. */
@@ -515,14 +516,21 @@ client_wait_until_can_do_something(fd_se
 	 * event pending.
 	 */
 
-	if (options.server_alive_interval == 0 || !compat20)
-		tvp = NULL;
-	else {
+	if (options.server_alive_interval != 0 && compat20 &&
+	    tv.tv_sec == 0 && tv.tv_usec == 0) {
 		tv.tv_sec = options.server_alive_interval;
 		tv.tv_usec = 0;
 		tvp = &tv;
 	}
+
 	ret = select((*maxfdp)+1, *readsetp, *writesetp, NULL, tvp);
+
+	if (options.server_alive_interval != 0 && compat20 &&
+	    FD_ISSET(connection_in, *readsetp)) {
+		tv.tv_sec = 0;
+		tv.tv_usec = 0;
+	}
+
 	if (ret < 0) {
 		char buf[100];
 

-- 
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.