Multiplexing bug on client exit

[Damien Miller]

On Wed, 27 Jan 2010, Damien Miller wrote:

> > Could you try to catch this with both the master and slave in debug
> > mode? "ssh -ddd"
> > 
> > I'll see if I can find it in the meantime.
> 
> You could also try this diff, but I'd appreciate seeing a debug trace from
> before you apply it if possible.

better diff; this fixes one crash case I could reproduce pretty easily:

ssh -nNf host
ssh host "sleep 20" </dev/null
^C
(wait)


Index: mux.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/mux.c,v
retrieving revision 1.11
diff -u -p -r1.11 mux.c
--- mux.c	26 Jan 2010 02:15:20 -0000	1.11
+++ mux.c	27 Jan 2010 03:56:25 -0000
@@ -193,7 +193,13 @@ mux_master_control_cleanup_cb(int cid, v
 			    __func__, c->self, c->remote_id);
 		c->remote_id = -1;
 		sc->ctl_chan = -1;
-		chan_mark_dead(sc);
+		if (sc->type != SSH_CHANNEL_OPEN) {
+			debug2("%s: channel %d: not open", __func__, sc->self);
+			chan_mark_dead(c);
+		} else {
+			chan_read_failed(sc);
+			chan_write_failed(sc);
+		}
 	}
 	channel_cancel_cleanup(c->self);
 }
Index: nchan.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/nchan.c,v
retrieving revision 1.63
diff -u -p -r1.63 nchan.c
--- nchan.c	26 Jan 2010 01:28:35 -0000	1.63
+++ nchan.c	27 Jan 2010 03:56:25 -0000
@@ -159,7 +159,7 @@ chan_ibuf_empty(Channel *c)
 	switch (c->istate) {
 	case CHAN_INPUT_WAIT_DRAIN:
 		if (compat20) {
-			if (!(c->flags & (CHAN_CLOSE_SENT|CHAN_LOCAL)))
+			if (!(c->flags & CHAN_CLOSE_SENT))
 				chan_send_eof2(c);
 			chan_set_istate(c, CHAN_INPUT_CLOSED);
 		} else {
@@ -240,6 +240,10 @@ chan_send_ieof1(Channel *c)
 	switch (c->istate) {
 	case CHAN_INPUT_OPEN:
 	case CHAN_INPUT_WAIT_DRAIN:
+		if ((c->flags & CHAN_LOCAL)) {
+			debug3("channel %d: local, not sending ieof", c->self);
+			return;
+		}
 		packet_start(SSH_MSG_CHANNEL_INPUT_EOF);
 		packet_put_int(c->remote_id);
 		packet_send();
@@ -257,6 +261,11 @@ chan_send_oclose1(Channel *c)
 	switch (c->ostate) {
 	case CHAN_OUTPUT_OPEN:
 	case CHAN_OUTPUT_WAIT_DRAIN:
+		if ((c->flags & CHAN_LOCAL)) {
+			debug3("channel %d: local, not sending oclose",
+			    c->self);
+			return;
+		}
 		buffer_clear(&c->output);
 		packet_start(SSH_MSG_CHANNEL_OUTPUT_CLOSE);
 		packet_put_int(c->remote_id);
@@ -303,8 +312,7 @@ chan_rcvd_close2(Channel *c)
 		chan_set_istate(c, CHAN_INPUT_CLOSED);
 		break;
 	case CHAN_INPUT_WAIT_DRAIN:
-		if (!(c->flags & CHAN_LOCAL))
-			chan_send_eof2(c);
+		chan_send_eof2(c);
 		chan_set_istate(c, CHAN_INPUT_CLOSED);
 		break;
 	}
@@ -353,6 +361,10 @@ chan_send_eof2(Channel *c)
 	debug2("channel %d: send eof", c->self);
 	switch (c->istate) {
 	case CHAN_INPUT_WAIT_DRAIN:
+		if ((c->flags & CHAN_LOCAL)) {
+			debug3("channel %d: local, not sending close", c->self);
+			return;
+		}
 		packet_start(SSH2_MSG_CHANNEL_EOF);
 		packet_put_int(c->remote_id);
 		packet_send();
@@ -372,6 +384,8 @@ chan_send_close2(Channel *c)
 	    c->istate != CHAN_INPUT_CLOSED) {
 		error("channel %d: cannot send close for istate/ostate %d/%d",
 		    c->self, c->istate, c->ostate);
+	} else if ((c->flags & CHAN_LOCAL)) {
+		debug3("channel %d: local, not sending close", c->self);
 	} else if (c->flags & CHAN_CLOSE_SENT) {
 		error("channel %d: already sent close", c->self);
 	} else {
@@ -388,6 +402,9 @@ chan_send_eow2(Channel *c)
 	if (c->ostate == CHAN_OUTPUT_CLOSED) {
 		error("channel %d: must not sent eow on closed output",
 		    c->self);
+		return;
+	} else if ((c->flags & CHAN_LOCAL)) {
+		debug3("channel %d: local, not sending close", c->self);
 		return;
 	}
 	if (!(datafellows & SSH_NEW_OPENSSH))