# HG changeset patch # User Matt Johnston # Date 1447948372 -28800 # Node ID 509cf5df51c61199a6e2b9e26756768eaf67993c # Parent 1397a677cb5c84fe1c2eda9c1baa297f3bcdbb0d Avoid queueing into circbuffer when the channel is about to close diff -r 1397a677cb5c -r 509cf5df51c6 common-channel.c --- a/common-channel.c Thu Nov 19 23:52:11 2015 +0800 +++ b/common-channel.c Thu Nov 19 23:52:52 2015 +0800 @@ -42,7 +42,7 @@ static void send_msg_channel_open_confirmation(struct Channel* channel, unsigned int recvwindow, unsigned int recvmaxpacket); -static void writechannel(struct Channel* channel, int fd, circbuffer *cbuf, +static int writechannel(struct Channel* channel, int fd, circbuffer *cbuf, const unsigned char *moredata, unsigned int *morelen); static void send_msg_channel_window_adjust(struct Channel *channel, unsigned int incr); @@ -100,15 +100,6 @@ TRACE(("leave chancleanup")) } -static void -chan_initwritebuf(struct Channel *channel) -{ - dropbear_assert(channel->writebuf->size == 0 && channel->recvwindow == 0); - cbuf_free(channel->writebuf); - channel->writebuf = cbuf_new(opts.recv_window); - channel->recvwindow = opts.recv_window; -} - /* Create a new channel entry, send a reply confirm or failure */ /* If remotechan, transwindow and transmaxpacket are not know (for a new * outgoing connection, with them to be filled on confirmation), they should @@ -167,8 +158,8 @@ newchan->await_open = 0; newchan->flushing = 0; - newchan->writebuf = cbuf_new(0); /* resized later by chan_initwritebuf */ - newchan->recvwindow = 0; + newchan->writebuf = cbuf_new(opts.recv_window); + newchan->recvwindow = opts.recv_window; newchan->extrabuf = NULL; /* The user code can set it up */ newchan->recvdonelen = 0; @@ -379,7 +370,6 @@ { channel->readfd = channel->writefd = sock; channel->conn_pending = NULL; - chan_initwritebuf(channel); send_msg_channel_open_confirmation(channel, channel->recvwindow, channel->recvmaxpacket); TRACE(("leave channel_connect_done: success")) @@ -436,7 +426,7 @@ } #ifndef HAVE_WRITEV -static void writechannel_fallback(struct Channel* channel, int fd, circbuffer *cbuf, +static int writechannel_fallback(struct Channel* channel, int fd, circbuffer *cbuf, const unsigned char *UNUSED(moredata), unsigned int *morelen) { unsigned char *circ_p1, *circ_p2; @@ -455,23 +445,24 @@ if (errno != EINTR && errno != EAGAIN) { TRACE(("channel IO write error fd %d %s", fd, strerror(errno))) close_chan_fd(channel, fd, SHUT_WR); + return DROPBEAR_FAILURE; } - } else { - cbuf_incrread(cbuf, written); - channel->recvdonelen += written; } + cbuf_incrread(cbuf, written); + channel->recvdonelen += written; + return DROPBEAR_SUCCESS; } #endif /* !HAVE_WRITEV */ #ifdef HAVE_WRITEV -static void writechannel_writev(struct Channel* channel, int fd, circbuffer *cbuf, +static int writechannel_writev(struct Channel* channel, int fd, circbuffer *cbuf, const unsigned char *moredata, unsigned int *morelen) { struct iovec iov[3]; unsigned char *circ_p1, *circ_p2; unsigned int circ_len1, circ_len2; int io_count = 0; - + int cbuf_written; ssize_t written; cbuf_readptrs(cbuf, &circ_p1, &circ_len1, &circ_p2, &circ_len2); @@ -503,7 +494,7 @@ From common_recv_msg_channel_data() then channelio(). The second call may not have any data to write, so we just return. */ TRACE(("leave writechannel, no data")) - return; + return DROPBEAR_SUCCESS; } if (morelen) { @@ -517,29 +508,32 @@ if (errno != EINTR && errno != EAGAIN) { TRACE(("channel IO write error fd %d %s", fd, strerror(errno))) close_chan_fd(channel, fd, SHUT_WR); + return DROPBEAR_FAILURE; } - } else { - int cbuf_written = MIN(circ_len1+circ_len2, (unsigned int)written); - cbuf_incrread(cbuf, cbuf_written); - if (morelen) { - *morelen = written - cbuf_written; - } - channel->recvdonelen += written; + } + + cbuf_written = MIN(circ_len1+circ_len2, (unsigned int)written); + cbuf_incrread(cbuf, cbuf_written); + if (morelen) { + *morelen = written - cbuf_written; } - + channel->recvdonelen += written; + return DROPBEAR_SUCCESS; } #endif /* HAVE_WRITEV */ /* Called to write data out to the local side of the channel. Writes the circular buffer contents and also the "moredata" buffer - if not null. Will ignore EAGAIN */ -static void writechannel(struct Channel* channel, int fd, circbuffer *cbuf, + if not null. Will ignore EAGAIN. + Returns DROPBEAR_FAILURE if writing to fd had an error and the channel is being closed, DROPBEAR_SUCCESS otherwise */ +static int writechannel(struct Channel* channel, int fd, circbuffer *cbuf, const unsigned char *moredata, unsigned int *morelen) { + int ret = DROPBEAR_SUCCESS; TRACE(("enter writechannel fd %d", fd)) #ifdef HAVE_WRITEV - writechannel_writev(channel, fd, cbuf, moredata, morelen); + ret = writechannel_writev(channel, fd, cbuf, moredata, morelen); #else - writechannel_fallback(channel, fd, cbuf, moredata, morelen); + ret = writechannel_fallback(channel, fd, cbuf, moredata, morelen); #endif /* Window adjust handling */ @@ -555,6 +549,7 @@ channel->recvwindow <= cbuf_getavail(channel->extrabuf)); TRACE(("leave writechannel")) + return ret; } @@ -829,6 +824,7 @@ unsigned int buflen; unsigned int len; unsigned int consumed; + int res; TRACE(("enter recv_msg_channel_data")) @@ -861,7 +857,7 @@ /* Attempt to write the data immediately without having to put it in the circular buffer */ consumed = datalen; - writechannel(channel, fd, cbuf, buf_getptr(ses.payload, datalen), &consumed); + res = writechannel(channel, fd, cbuf, buf_getptr(ses.payload, datalen), &consumed); datalen -= consumed; buf_incrpos(ses.payload, consumed); @@ -869,17 +865,20 @@ /* We may have to run throught twice, if the buffer wraps around. Can't * just "leave it for next time" like with writechannel, since this - * is payload data */ - len = datalen; - while (len > 0) { - buflen = cbuf_writelen(cbuf); - buflen = MIN(buflen, len); + * is payload data. + * If the writechannel() failed then remaining data is discarded */ + if (res == DROPBEAR_SUCCESS) { + len = datalen; + while (len > 0) { + buflen = cbuf_writelen(cbuf); + buflen = MIN(buflen, len); - memcpy(cbuf_writeptr(cbuf, buflen), - buf_getptr(ses.payload, buflen), buflen); - cbuf_incrwrite(cbuf, buflen); - buf_incrpos(ses.payload, buflen); - len -= buflen; + memcpy(cbuf_writeptr(cbuf, buflen), + buf_getptr(ses.payload, buflen), buflen); + cbuf_incrwrite(cbuf, buflen); + buf_incrpos(ses.payload, buflen); + len -= buflen; + } } TRACE(("leave recv_msg_channel_data")) @@ -993,8 +992,6 @@ channel->prio = DROPBEAR_CHANNEL_PRIO_BULK; } - chan_initwritebuf(channel); - /* success */ send_msg_channel_open_confirmation(channel, channel->recvwindow, channel->recvmaxpacket); @@ -1137,7 +1134,6 @@ /* Outbound opened channels don't make use of in-progress connections, * we can set it up straight away */ - chan_initwritebuf(chan); /* set fd non-blocking */ setnonblocking(fd);