Dropbear 0.50 server returnsexit code 255 to ssh when app returns 0

[Steven Hein]

Matt--

I tested 0.50 with the common-channel.c patch below,
and it worked as expected (always returned the
remote command's exit code correctly).

As you said, it does cause the "sleep 10 & hello"
command to return immediately after echoing
"hello", leaving the "sleep 10" running on the remote
host.

Thanks for the quick turn-around!

Steve


Matt Johnston wrote:
> 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);
>   


-- 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Steve Hein (ssh at sgi.com)              Engineering Diagnostics/Software
Silicon Graphics, Inc.                          
1168 Industrial Blvd.                 Phone: (715) 726-8410
Chippewa Falls, WI 54729              Fax:   (715) 726-6715
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~