On Fri, Feb 29, 2008 at 03:32:52PM -0600, Steven Hein wrote:
> Steven Hein wrote:
> >Hello--
> >
> >I have not seen any new information on this issue since the thread
> >from last December:
> >
> >http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/2007q4/000672.html
> >
> >Does anyone have any updates or workarounds?     I just upgraded
> >to 0.50 about 2 weeks ago and found that this is causing me troubles.
> >
...
> I spent some time digging in to this today   (rather
> unscientifically, by sprinkling dropbear_log() messages throughout
> svr-chansession.c).  I found that the child process didn't seem to
> terminate before that client requested that the session be closed.
> (Specifically, the child was still running when send_exitsignalstatus()
> was called, so exitpid was still -1.   I couldn't see any obvious
> reason for this, and the child process always terminated
> "very soon" (milliseconds) after send_exitsignalstatus() was done.

Yep, that seems to be the cause. I've attached a patch that
will wait for he process to exit before closing the channel,
which should fix the problem. I've put a testing release at
http://matt.ucc.asn.au/dropbear/testing/dropbear-0.51test2.tar.bz2
with that fix and a couple of other things.

I'd appreciate if anyone who has had odd channel-closing
issues could test that. I suspect that it might alter some
behaviour such as 'ssh hostname "sleep 10 & echo hello"',
but I can't see a better way to do it - if during cleanup a
process's stdin/stdout is closed before the process
terminates, then there'll always be that race. It seems
safer to just wait for the spawned shell to exit.

Cheers,
Matt
#
# old_revision [6107b89c1188975d0c60f50621afe593cb6e554f]
#
# patch "common-channel.c"
#  from [375ff10b1da7d82baf11af5e032ac82184639ce4]
#    to [7370ebc3b83fd52b5114385329e439624ddee60a]
#
============================================================
--- common-channel.c	375ff10b1da7d82baf11af5e032ac82184639ce4
+++ common-channel.c	7370ebc3b83fd52b5114385329e439624ddee60a
@@ -261,6 +261,7 @@ static void check_close(struct Channel *
 
 /* EOF/close handling */
 static void check_close(struct Channel *channel) {
+	int close_allowed = 0;
 
 	TRACE(("check_close: writefd %d, readfd %d, errfd %d, sent_close %d, recv_close %d",
 				channel->writefd, channel->readfd,
@@ -274,8 +275,17 @@ static void check_close(struct Channel *
 	{
 		channel->flushing = 1;
 	}
+	
+	// if a type-specific check_close is defined we will only exit
+	// once that has been triggered. this is only used for a server "session"
+	// channel, to ensure that the shell has exited (and the exit status 
+	// retrieved) before we close things up.	
+	if (!channel->type->check_close	
+			|| channel->type->check_close(channel)) {
+		close_allowed = 1;
+	}
 
-	if (channel->recv_close && !write_pending(channel)) {
+	if (channel->recv_close && !write_pending(channel) && close_allowed) {
 		if (!channel->sent_close) {
 			TRACE(("Sending MSG_CHANNEL_CLOSE in response to same."))
 			send_msg_channel_close(channel);
@@ -312,9 +322,10 @@ static void check_close(struct Channel *
 	}
 
 	/* And if we can't receive any more data from them either, close up */
-	if (!channel->sent_close
-			&& channel->readfd == FD_CLOSED
+	if (channel->readfd == FD_CLOSED
 			&& (ERRFD_IS_WRITE(channel) || channel->errfd == FD_CLOSED)
+			&& !channel->sent_close
+			&& close_allowed
 			&& !write_pending(channel)) {
 		TRACE(("sending close, readfd is closed"))
 		send_msg_channel_close(channel);

Reply via email to