openssh unix dev - Nov 2001 - [PATCH]: Patch to fix hang on exit bug under Linux and add optional exit delay

The hang-on-exit bug still hasn't been fixed in OpenSSH-3.0p1... :-(

Here again is the patch to fix this bug under Linux, updated for OpenSSH-3.0p1.
This has been exhaustively tested for six months now.  It also add an exit
delay option that can be useful.

The patch does not lead to data loss under Linux. 

Please see the Secure NFS page (SNFS) for further details:

http://www.math.ualberta.ca/imaging/snfs/

-- John Bowman
University of Alberta

diff -ur openssh-3.0p1/channels.c openssh-3.0p1J/channels.c
--- openssh-3.0p1/channels.c	Thu Oct 11 19:35:05 2001
+++ openssh-3.0p1J/channels.c	Tue Nov 13 16:02:32 2001
@@ -1553,8 +1553,18 @@
 		c = channels[i];
 		if (c == NULL)
 			continue;
-		if (ftab[c->type] != NULL)
+		if (ftab[c->type] != NULL) {
+			if(c->istate == CHAN_INPUT_OPEN && c->rfd == -1) {
+			  int type=c->type;
+			  c->type=SSH_CHANNEL_CLOSED;
+			  if(channel_find_open() == -1)
+			    shutdown(packet_get_connection_out(),
+				     SHUT_RDWR);
+			  c->type=type;
+			  continue;
+			}
 			(*ftab[c->type])(c, readset, writeset);
+		}
 		channel_garbage_collect(c);
 	}
 }
diff -ur openssh-3.0p1/channels.h openssh-3.0p1J/channels.h
--- openssh-3.0p1/channels.h	Thu Oct 11 19:35:06 2001
+++ openssh-3.0p1J/channels.h	Tue Nov 13 15:55:52 2001
@@ -218,6 +218,7 @@
 void	 chan_mark_dead(Channel *);
 void 	 chan_init_iostates(Channel *);
 void	 chan_init(void);
+void	 chan_shutdown_read(Channel *);
 
 typedef void    chan_event_fn(Channel *);
 
diff -ur openssh-3.0p1/clientloop.c openssh-3.0p1J/clientloop.c
--- openssh-3.0p1/clientloop.c	Thu Oct 11 19:36:09 2001
+++ openssh-3.0p1J/clientloop.c	Tue Nov 13 16:06:36 2001
@@ -84,6 +84,7 @@
 
 /* import options */
 extern Options options;
+extern int no_tty_flag;
 
 /* Flag indicating that stdin should be redirected from /dev/null. */
 extern int stdin_null_flag;
@@ -122,7 +123,8 @@
 static int connection_in;	/* Connection to server (input). */
 static int connection_out;	/* Connection to server (output). */
 static int need_rekeying;	/* Set to non-zero if rekeying is requested. */
-static int session_closed = 0;	/* In SSH2: login session closed. */
+enum SessionStatus {SessionOpen, SessionClose, SessionWait};
+static int session_status = SessionOpen; /* In SSH2: login session closed. */
 
 static void client_init_dispatch(void);
 int	session_ident = -1;
@@ -320,6 +322,10 @@
 client_wait_until_can_do_something(fd_set **readsetp, fd_set **writesetp,
     int *maxfdp, int *nallocp, int rekeying)
 {
+	struct timeval timer;
+	struct timeval *timerp;
+	int rc;
+	
 	/* Add any selections by the channel mechanism. */
 	channel_prepare_select(readsetp, writesetp, maxfdp, nallocp, rekeying);
 
@@ -343,7 +349,7 @@
 			FD_SET(fileno(stderr), *writesetp);
 	} else {
 		/* channel_prepare_select could have closed the last channel */
-		if (session_closed && !channel_still_open() &&
+		if (session_status == SessionClose && !channel_still_open()
&&
 		    !packet_have_data_to_write()) {
 			/* clear mask since we did not call select() */
 			memset(*readsetp, 0, *maxfdp);
@@ -367,7 +373,17 @@
 	 * SSH_MSG_IGNORE packet when the timeout expires.
 	 */
 
-	if (select((*maxfdp)+1, *readsetp, *writesetp, NULL, NULL) < 0) {
+	if((session_status == SessionWait && options.sleep > 0) ||
+	    (no_tty_flag && options.sleep == -1)) {
+		timer.tv_sec=options.sleep > 0 ? options.sleep : 0;
+		timer.tv_usec=0;
+		timerp=&timer;
+	} else {
+		timerp=NULL;
+	}
+	
+	rc=select((*maxfdp)+1, *readsetp, *writesetp, NULL, timerp);
+	if (rc < 0) {
 		char buf[100];
 
 		/*
@@ -384,7 +400,8 @@
 		snprintf(buf, sizeof buf, "select: %s\r\n", strerror(errno));
 		buffer_append(&stderr_buffer, buf, strlen(buf));
 		quit_pending = 1;
-	}
+	} else if (rc == 0 && session_status == SessionWait)
+		session_status=SessionClose;
 }
 
 static void
@@ -445,9 +462,12 @@
 		len = read(connection_in, buf, sizeof(buf));
 		if (len == 0) {
 			/* Received EOF.  The remote host has closed the connection. */
+/* This message duplicates the one already in client_loop(). */
+#if 0 
 			snprintf(buf, sizeof buf, "Connection to %.300s closed by remote
host.\r\n",
 				 host);
 			buffer_append(&stderr_buffer, buf, strlen(buf));
+#endif
 			quit_pending = 1;
 			return;
 		}
@@ -757,7 +777,7 @@
 		error("client_channel_closed: id %d != session_ident %d",
 		    id, session_ident);
 	channel_cancel_cleanup(id);
-	session_closed = 1;
+ 	session_status = (options.sleep >= 0) ? SessionWait : SessionClose;
 	if (in_raw_mode())
 		leave_raw_mode();
 }
@@ -782,6 +802,7 @@
 	start_time = get_current_time();
 
 	/* Initialize variables. */
+	if(!have_pty) session_status=SessionWait;
 	escape_pending = 0;
 	last_was_cr = 1;
 	exit_status = -1;
@@ -846,7 +867,8 @@
 		/* Process buffered packets sent by the server. */
 		client_process_buffered_input_packets();
 
-		if (compat20 && session_closed && !channel_still_open())
+		if (compat20 && (session_status == SessionClose)
+		    && !channel_still_open())
 			break;
 
 		rekeying = (xxx_kex != NULL && !xxx_kex->done);
diff -ur openssh-3.0p1/nchan.c openssh-3.0p1J/nchan.c
--- openssh-3.0p1/nchan.c	Thu Oct 11 19:35:06 2001
+++ openssh-3.0p1J/nchan.c	Tue Nov 13 15:55:52 2001
@@ -81,7 +81,7 @@
 
 /* helper */
 static void	chan_shutdown_write(Channel *);
-static void	chan_shutdown_read(Channel *);
+void		chan_shutdown_read(Channel *);
 
 /*
  * SSH1 specific implementation of event functions
@@ -533,7 +533,7 @@
 			    c->self, c->wfd, strerror(errno));
 	}
 }
-static void
+void
 chan_shutdown_read(Channel *c)
 {
 	if (compat20 && c->type == SSH_CHANNEL_LARVAL)
diff -ur openssh-3.0p1/readconf.c openssh-3.0p1J/readconf.c
--- openssh-3.0p1/readconf.c	Wed Oct  3 11:39:39 2001
+++ openssh-3.0p1J/readconf.c	Tue Nov 13 16:09:00 2001
@@ -115,7 +115,7 @@
 	oKbdInteractiveAuthentication, oKbdInteractiveDevices, oHostKeyAlias,
 	oDynamicForward, oPreferredAuthentications, oHostbasedAuthentication,
 	oHostKeyAlgorithms, oBindAddress, oSmartcardDevice,
-	oClearAllForwardings, oNoHostAuthenticationForLocalhost 
+	oClearAllForwardings, oNoHostAuthenticationForLocalhost, oSleep 
 } OpCodes;
 
 /* Textual representations of the tokens. */
@@ -187,6 +187,7 @@
 	{ "smartcarddevice", oSmartcardDevice },
 	{ "clearallforwardings", oClearAllForwardings }, 
 	{ "nohostauthenticationforlocalhost",
oNoHostAuthenticationForLocalhost },
+  	{ "sleep", oSleep },
 	{ NULL, 0 }
 };
 
@@ -528,6 +529,10 @@
 		intptr = &options->connection_attempts;
 		goto parse_int;
 
+	case oSleep:
+		intptr = &options->sleep;
+		goto parse_int;
+
 	case oCipher:
 		intptr = &options->cipher;
 		arg = strdelim(&s);
@@ -799,6 +804,7 @@
 	options->bind_address = NULL;
 	options->smartcard_device = NULL;
 	options->no_host_authentication_for_localhost = - 1;
+	options->sleep = -1;
 }
 
 /*
diff -ur openssh-3.0p1/readconf.h openssh-3.0p1J/readconf.h
--- openssh-3.0p1/readconf.h	Wed Oct  3 11:39:39 2001
+++ openssh-3.0p1J/readconf.h	Tue Nov 13 16:10:26 2001
@@ -102,6 +102,7 @@
 	Forward remote_forwards[SSH_MAX_FORWARDS_PER_DIRECTION];
 	int	clear_forwardings;
 	int	no_host_authentication_for_localhost;
+        int     sleep;		/* Exit delay in seconds */
 }       Options;
 
 
diff -ur openssh-3.0p1/session.c openssh-3.0p1J/session.c
--- openssh-3.0p1/session.c	Sun Oct 28 04:34:53 2001
+++ openssh-3.0p1J/session.c	Tue Nov 13 15:55:52 2001
@@ -1919,6 +1919,9 @@
 	 */
 	if (c->ostate != CHAN_OUTPUT_CLOSED)
 		chan_write_failed(c);
+	if (c->istate == CHAN_INPUT_OPEN && compat20) {
+		chan_shutdown_read(c);
+	}
 	s->chanid = -1;
 }
 
diff -ur openssh-3.0p1/ssh.c openssh-3.0p1J/ssh.c
--- openssh-3.0p1/ssh.c	Tue Oct  9 23:07:45 2001
+++ openssh-3.0p1J/ssh.c	Tue Nov 13 15:55:52 2001
@@ -195,6 +195,7 @@
 	fprintf(stderr, "              These cause %s to listen for connections
on a port, and\n", __progname);
 	fprintf(stderr, "              forward them to the other side by
connecting to host:port.\n");
 	fprintf(stderr, "  -D port     Enable dynamic application-level port
forwarding.\n");
+ 	fprintf(stderr, "  -S delay    Set exit delay (in seconds; 0 means wait
forever).\n");
 	fprintf(stderr, "  -C          Enable compression.\n");
 	fprintf(stderr, "  -N          Do not execute a shell or
command.\n");
 	fprintf(stderr, "  -g          Allow remote hosts to connect to forwarded
ports.\n");
@@ -320,7 +321,7 @@
 
 again:
 	while ((opt = getopt(ac, av,
-	    "1246ab:c:e:fgi:kl:m:no:p:qstvxACD:F:I:L:NPR:TVX")) != -1) {
+	    "1246ab:c:e:fgi:kl:m:no:p:qstvxACD:F:I:L:NPR:S:TVX")) != -1) {
 		switch (opt) {
 		case '1':
 			options.protocol = SSH_PROTO_1;
@@ -495,7 +496,13 @@
 				add_remote_forward(&options, fwd_port, buf,
 				     fwd_host_port);
 			break;
-
+		case 'S':
+			options.sleep = atoi(optarg);
+			if (options.sleep < 0) {
+				fprintf(stderr, "Bad delay value '%s'\n", optarg);
+				exit(1);
+			}
+			break;
 		case 'D':
 			fwd_port = a2port(optarg);
 			if (fwd_port == 0) {
diff -ur openssh-3.0p1/version.h openssh-3.0p1J/version.h
--- openssh-3.0p1/version.h	Wed Oct 24 09:20:57 2001
+++ openssh-3.0p1J/version.h	Tue Nov 13 16:11:16 2001
@@ -1,3 +1,3 @@
 /* $OpenBSD: version.h,v 1.25 2001/10/15 16:10:50 deraadt Exp $ */
 
-#define SSH_VERSION	"OpenSSH_3.0p1"
+#define SSH_VERSION	"OpenSSH_3.0p1J"

why do you repeat posting this broken patch to the list?

calling shutdown is wrong.

On Wed, Nov 14, 2001 at 12:31:43AM -0000, John Bowman
wrote:
> diff -ur openssh-3.0p1/session.c openssh-3.0p1J/session.c
> --- openssh-3.0p1/session.c	Sun Oct 28 04:34:53 2001
> +++ openssh-3.0p1J/session.c	Tue Nov 13 15:55:52 2001
> @@ -1919,6 +1919,9 @@
>  	 */
>  	if (c->ostate != CHAN_OUTPUT_CLOSED)
>  		chan_write_failed(c);
> +	if (c->istate == CHAN_INPUT_OPEN && compat20) {
> +		chan_shutdown_read(c);
> +	}
>  	s->chanid = -1;
>  }

> The hang-on-exit bug still hasn't been fixed in OpenSSH-3.0p1... :-(
> 
Has this issue been resolved in 3.02 ??

Michael
Michael at bizsystems.com

On Fri, Dec 07, 2001 at 02:18:10PM -0800, Michael wrote:
> Has this issue been resolved in 3.02 ??
no bugreport, no fix.  no bug, no fix.

> On Fri, Dec 07, 2001 at 02:18:10PM -0800, Michael wrote:
> > Has this issue been resolved in 3.02 ??
> 
> no bugreport, no fix.  no bug, no fix.
> 
so try this, guaranteed to hang the exit of the session for 
openssh, but ssh-1.2.27 likes it fine.

file tst.pl
#!/usr/bin/perl

while (1) {
  sleep 1;
}

# end

at the keyboard, just type
> perl tst.pl &
> exit

Like I said, ssh-1.2.27 works fine
OpenSSH_2.9.9p2 is guaranteed to hang

This means that a daemon can never be stopped and restarted from the 
keyboard without a hang or screwing around with special exit 
procedures. Looks, sounds, smells like a bug to me.

Michael
Michael at Insulin-Pumpers.org

> so try this, guaranteed to hang the exit of the session for 
> openssh, but ssh-1.2.27 likes it fine.
> file tst.pl
> #!/usr/bin/perl
> while (1) {
>   sleep 1;
> }
> # end
> at the keyboard, just type
> > perl tst.pl &
> > exit
> This means that a daemon can never be stopped and restarted from the 
> keyboard without a hang or screwing around with special exit 
> procedures. Looks, sounds, smells like a bug to me.
That is not a proper daemon program and therefore not a good example
of a problem.  Proper daemons should always close any tty file
descriptor, detach from any controlling terminals, call setsid() to
form a new process group, etc.  This is more representative of a
program that could loose data.

Perhaps one of the reasons this issue has been hard to get agreement
on is that everyone is testing with different daemons and some of
those are ill behaved?

Bob

On Mon, Dec 10, 2001 at 09:10:08AM -0800, Michael wrote:
> > On Fri, Dec 07, 2001 at 02:18:10PM -0800, Michael wrote:
> > > Has this issue been resolved in 3.02 ??
> > 
> > no bugreport, no fix.  no bug, no fix.
> > 
> 
> so try this, guaranteed to hang the exit of the session for 
> openssh, but ssh-1.2.27 likes it fine.
> 
> file tst.pl
> #!/usr/bin/perl
> 
> while (1) {
>   sleep 1;
> }
> 
> # end
> 
> at the keyboard, just type
> 
> > perl tst.pl &
> > exit
> 
> 
> Like I said, ssh-1.2.27 works fine
ssh-1.2.27 loses data. this has been changed in later
versions, e.g ssh-1.2.32.
> OpenSSH_2.9.9p2 is guaranteed to hang
> 
> This means that a daemon can never be stopped and restarted from the 
> keyboard without a hang or screwing around with special exit 
> procedures. Looks, sounds, smells like a bug to me.
it works unless the daemon is broken.

-m