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