Hi Catalin,<br>
<br>
Thanks for looking at that - the last patch looks sensible, I'll give it a good test. There are a lot of subtle scenarios in channel closing (and variations between OSes). <br>
<br>
Cheers,<br>
Matt<br>
<br>
Catalin Patulea <cat@vv.carleton.ca> wrote:<br>
>Hm, that broke channel-close-by-child-exit. One more try, where we<br>
>check for the child exiting and close writefd as a result. If writefd<br>
>is the last remaining open pipe to the child, then we also close the<br>
>channel as a whole.<br>
><br>
>On Wed, Jul 17, 2013 at 3:25 PM, Catalin Patulea <cat@vv.carleton.ca><br>
>wrote:<br>
>> Attached patch should fix both, and use hard tabs so should apply<br>
>easily.<br>
>><br>
>> Rather than replacing readfd with writefd, *both* are checked for<br>
>> FD_CLOSED before closing the entire channel. Then each direction can<br>
>> be initially closed independently.<br>
>><br>
>> On Wed, Jul 17, 2013 at 7:57 PM, Catalin Patulea <cat@vv.carleton.ca><br>
>wrote:<br>
>>> dbclient handling of remote EOF/local not at EOF also appears<br>
>broken:<br>
>>><br>
>>> openssh:<br>
>>> $ ssh -v root@1.2.3.4 'exec cat >/dev/null 2>/dev/null'<br>
>>> [...]<br>
>>> debug1: Entering interactive session.<br>
>>> debug1: Sending command: exec cat >/dev/null 2>/dev/null<br>
>>> # remote has sent EOF by virtue of &>/dev/null, but local->remote<br>
>>> # direction still active<br>
>>> foo # local sends<br>
>>> bar<br>
>>> ^D # local EOF, channel tears down<br>
>>> debug1: client_input_channel_req: channel 1 rtype exit-status reply<br>
>0<br>
>>> debug1: channel 1: free: client-session, nchannels 2<br>
>>> <exit><br>
>>><br>
>>> Note 'exec' is required to replace shell and prevent it from holding<br>
>a<br>
>>> ref to the stdout pipe.<br>
>>><br>
>>> dropbear:<br>
>>> $ ./dbclient -v root@lun.nanobit.org 'exec cat >/dev/null<br>
>2>/dev/null'<br>
>>> [...]<br>
>>> TRACE (31787) 1374079900.330457: process_packet: packet type = 96, <br>
>len 5<br>
>>> TRACE (31787) 1374079900.330499: enter recv_msg_channel_eof<br>
>>> TRACE (31787) 1374079900.330513: CLOSE some fd 1<br>
>>> TRACE (31787) 1374079900.330526: sending close, readfd is closed<br>
>>> # remote EOF should *not* cause entire channel teardown;<br>
>>> # local may still have something to write ("foo bar" from openssh<br>
>>> # example).<br>
>>> TRACE (31787) 1374079900.330537: enter send_msg_channel_close<br>
>0x1f78660<br>
>>> TRACE (31787) 1374079900.330549: enter cli_tty_cleanup<br>
>>> TRACE (31787) 1374079900.330560: leave cli_tty_cleanup: not in raw<br>
>mode<br>
>>> TRACE (31787) 1374079900.330606: CLOSE some fd 0<br>
>>> TRACE (31787) 1374079900.330618: CLOSE some fd 2<br>
>>> <exit><br>
>>><br>
>>><br>
>>> On Sat, Jul 13, 2013 at 12:51 PM, Catalin Patulea<br>
><cat@vv.carleton.ca> wrote:<br>
>>>> Maybe the check on common-channel.c:338 should be against writefd<br>
>>>> instead of readfd? This would get set by<br>
>>>> close_chan_fd(channel->writefd) once recv_eof happens.<br>
>>>><br>
>>>> This patch indeed causes 'foo' to surface after input EOF:<br>
>>>><br>
>>>> diff -r 69cb561cc4c4 common-channel.c<br>
>>>> --- a/common-channel.c Sat Jul 13 11:53:24 2013 +0300<br>
>>>> +++ b/common-channel.c Sat Jul 13 12:50:41 2013 +0300<br>
>>>> @@ -335,7 +335,7 @@<br>
>>>> }<br>
>>>><br>
>>>> /* And if we can't receive any more data from them either, close<br>
>up */<br>
>>>> - if (channel->readfd == FD_CLOSED<br>
>>>> + if (channel->writefd == FD_CLOSED<br>
>>>> && (ERRFD_IS_WRITE(channel) || channel->errfd == FD_CLOSED)<br>
>>>> && !channel->sent_close<br>
>>>> && close_allowed<br>
>>>><br>
>>>> On Sat, Jul 13, 2013 at 12:31 PM, Catalin Patulea<br>
><cat@vv.carleton.ca> wrote:<br>
>>>>> Hi,<br>
>>>>><br>
>>>>> I'm seeing a difference in how dbclient handles EOF on input<br>
>compared<br>
>>>>> to openssh client. openssh client propagates input EOF to the<br>
>remote<br>
>>>>> command, but continues pumping command stdout. dbclient seems to<br>
>abort<br>
>>>>> before flushing the stdout buffer.<br>
>>>>><br>
>>>>> In the following examples, <a href="http://1.2.3.4">1.2.3.4</a> is an openwrt router running<br>
>>>>> dropbear server. The remote command is designed to wait for EOF,<br>
>then<br>
>>>>> output something to stdout.<br>
>>>>><br>
>>>>> openssh client:<br>
>>>>> $ ssh root@1.2.3.4 'cat; echo foo'<br>
>>>>> ^D<br>
>>>>> foo<br>
>>>>><br>
>>>>> dbclient:<br>
>>>>> $ ./dbclient root@1.2.3.4 'cat; echo foo'<br>
>>>>> ^D<br>
>>>>> <no output><br>
>>>>><br>
>>>>> I build dropbear with DEBUG_TRACE and these are the last few<br>
>lines:<br>
>>>>> TRACE (...): empty queue dequeing<br>
>>>>> ^D<br>
>>>>> TRACE (...): send normal readfd<br>
>>>>> TRACE (...): enter send_msg_channel_data<br>
>>>>> TRACE (...): enter send_msg_channel_data isextended 0 fd 0<br>
>>>>> TRACE (...): maxlen 16375<br>
>>>>> TRACE (...): CLOSE some fd 0<br>
>>>>> TRACE (...): leave send_msg_channel_data: len 0 read err 17 or EOF<br>
>for fd 0<br>
>>>>> TRACE (...): enter send_msg_channel_eof<br>
>>>>> TRACE (...): leave send_msg_channel_eof<br>
>>>>> TRACE (...): sending close, readfd is closed<br>
>>>>> TRACE (...): enter send_msg_channel_close 0xcbfdc0<br>
>>>>> TRACE (...): enter cli_tty_cleanup<br>
>>>>> TRACE (...): leave cli_tty_cleanup: not in raw mode<br>
>>>>> TRACE (...): CLOSE some fd -1<br>
>>>>> TRACE (...): CLOSE some fd 2<br>
>>>>><br>
>>>>> I tried reading through the relevant sections of channel-common.c<br>
>but<br>
>>>>> I could use some guidance/background. Is this behaviour<br>
>intentional?<br>
>>>>><br>
>>>>> Catalin<br>