# HG changeset patch # User Matt Johnston # Date 1429015470 -28800 # Node ID 26c1f9b8f82e1c8ff680d7fc71954341d7c5c3ac # Parent 2109829358870e6b4cbd02b58ddea2e4efe43699# Parent d92597ef089e2fabc56e2e21a47dbc2f18a9c0af merge main diff -r 210982935887 -r 26c1f9b8f82e CHANGES --- a/CHANGES Mon Mar 02 21:17:41 2015 +0800 +++ b/CHANGES Tue Apr 14 20:44:30 2015 +0800 @@ -1,3 +1,19 @@ +- Improve efficiency of writing data to local program/pipes, measured 30% for + connections to localhost + +- Use TCP Fast Open on Linux if available. saves a round trip at connection + to hosts that have previously been connected. + Needs a recent Linux kernel and possibly "sysctl -w net.ipv4.tcp_fastopen=3" + +- Forwarded TCP ports connect asynchronously and retry with other available + addresses (IPv4 or IPv6) + +- Free memory before exiting, patch from Thorsten Horstmann. Useful for + Dropbear ports to embedded systems and for checking memory leaks + with valgrind. Only partially implemented for client side. + +- Fix small ECC memory leaks + 2015.67 - Wednesday 28 January 2015 - Call fsync() after generating private keys to ensure they aren't lost if a diff -r 210982935887 -r 26c1f9b8f82e channel.h --- a/channel.h Mon Mar 02 21:17:41 2015 +0800 +++ b/channel.h Tue Apr 14 20:44:30 2015 +0800 @@ -106,7 +106,7 @@ void chaninitialise(const struct ChanType *chantypes[]); void chancleanup(); -void setchannelfds(fd_set *readfd, fd_set *writefd); +void setchannelfds(fd_set *readfds, fd_set *writefds, int allow_reads); void channelio(fd_set *readfd, fd_set *writefd); struct Channel* getchannel(); /* Returns an arbitrary channel that is in a ready state - not diff -r 210982935887 -r 26c1f9b8f82e cli-main.c --- a/cli-main.c Mon Mar 02 21:17:41 2015 +0800 +++ b/cli-main.c Tue Apr 14 20:44:30 2015 +0800 @@ -87,6 +87,7 @@ static void cli_dropbear_exit(int exitcode, const char* format, va_list param) { char fmtbuf[300]; + char exitmsg[500]; if (!sessinitdone) { snprintf(fmtbuf, sizeof(fmtbuf), "Exited: %s", @@ -98,12 +99,15 @@ cli_opts.remoteport, format); } + /* Arguments to the exit printout may be unsafe to use after session_cleanup() */ + vsnprintf(exitmsg, sizeof(exitmsg), fmtbuf, param); + /* Do the cleanup first, since then the terminal will be reset */ session_cleanup(); /* Avoid printing onwards from terminal cruft */ fprintf(stderr, "\n"); - _dropbear_log(LOG_INFO, fmtbuf, param); + dropbear_log(LOG_INFO, "%s", exitmsg);; exit(exitcode); } diff -r 210982935887 -r 26c1f9b8f82e common-channel.c --- a/common-channel.c Mon Mar 02 21:17:41 2015 +0800 +++ b/common-channel.c Tue Apr 14 20:44:30 2015 +0800 @@ -473,6 +473,14 @@ io_count++; } + if (io_count == 0) { + /* writechannel may sometimes be called twice in a main loop iteration. + 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; + } + if (morelen) { /* Default return value, none consumed */ *morelen = 0; @@ -482,7 +490,7 @@ if (written < 0) { if (errno != EINTR && errno != EAGAIN) { - TRACE(("errno %d len %d", errno, len)) + TRACE(("channel IO write error fd %d %s", fd, strerror(errno))) close_chan_fd(channel, fd, SHUT_WR); } } else { @@ -531,7 +539,7 @@ /* Set the file descriptors for the main select in session.c * This avoid channels which don't have any window available, are closed, etc*/ -void setchannelfds(fd_set *readfds, fd_set *writefds) { +void setchannelfds(fd_set *readfds, fd_set *writefds, int allow_reads) { unsigned int i; struct Channel * channel; @@ -549,7 +557,7 @@ FD if there's the possibility of "~."" to kill an interactive session (the read_mangler) */ if (channel->transwindow > 0 - && (ses.dataallowed || channel->read_mangler)) { + && ((ses.dataallowed && allow_reads) || channel->read_mangler)) { if (channel->readfd >= 0) { FD_SET(channel->readfd, readfds); @@ -830,6 +838,7 @@ channel->recvwindow -= datalen; dropbear_assert(channel->recvwindow <= opts.recv_window); + /* 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); diff -r 210982935887 -r 26c1f9b8f82e common-session.c --- a/common-session.c Mon Mar 02 21:17:41 2015 +0800 +++ b/common-session.c Tue Apr 14 20:44:30 2015 +0800 @@ -64,6 +64,13 @@ ses.sock_out = sock_out; ses.maxfd = MAX(sock_in, sock_out); + if (sock_in >= 0) { + setnonblocking(sock_in); + } + if (sock_out >= 0) { + setnonblocking(sock_out); + } + ses.socket_prio = DROPBEAR_PRIO_DEFAULT; /* Sets it to lowdelay */ update_channel_prio(); @@ -145,6 +152,7 @@ /* main loop, select()s for all sockets in use */ for(;;) { + const int writequeue_has_space = (ses.writequeue_len <= 2*TRANS_MAX_PAYLOAD_LEN); timeout.tv_sec = select_timeout(); timeout.tv_usec = 0; @@ -155,8 +163,12 @@ /* We delay reading from the input socket during initial setup until after we have written out our initial KEXINIT packet (empty writequeue). This means our initial packet can be in-flight while we're doing a blocking - read for the remote ident */ - if (ses.sock_in != -1 && (ses.remoteident || isempty(&ses.writequeue))) { + read for the remote ident. + We also avoid reading from the socket if the writequeue is full, that avoids + replies backing up */ + if (ses.sock_in != -1 + && (ses.remoteident || isempty(&ses.writequeue)) + && writequeue_has_space) { FD_SET(ses.sock_in, &readfd); } if (ses.sock_out != -1 && !isempty(&ses.writequeue)) { @@ -168,7 +180,7 @@ FD_SET(ses.signal_pipe[0], &readfd); /* set up for channels which can be read/written */ - setchannelfds(&readfd, &writefd); + setchannelfds(&readfd, &writefd, writequeue_has_space); /* Pending connections to test */ set_connect_fds(&writefd); @@ -318,9 +330,7 @@ void send_session_identification() { buffer *writebuf = buf_new(strlen(LOCAL_IDENT "\r\n") + 1); buf_putbytes(writebuf, LOCAL_IDENT "\r\n", strlen(LOCAL_IDENT "\r\n")); - buf_putbyte(writebuf, 0x0); /* packet type */ - buf_setpos(writebuf, 0); - enqueue(&ses.writequeue, writebuf); + writebuf_enqueue(writebuf, 0); } static void read_session_identification() { diff -r 210982935887 -r 26c1f9b8f82e netio.c --- a/netio.c Mon Mar 02 21:17:41 2015 +0800 +++ b/netio.c Tue Apr 14 20:44:30 2015 +0800 @@ -76,7 +76,7 @@ for (r = c->res_iter; r; r = r->ai_next) { - assert(c->sock == -1); + dropbear_assert(c->sock == -1); c->sock = socket(c->res_iter->ai_family, c->res_iter->ai_socktype, c->res_iter->ai_protocol); if (c->sock < 0) { @@ -99,11 +99,16 @@ message.msg_namelen = r->ai_addrlen; if (c->writequeue) { - int iovlen; /* Linux msg_iovlen is a size_t */ - message.msg_iov = packet_queue_to_iovec(c->writequeue, &iovlen); + /* 6 is arbitrary, enough to hold initial packets */ + int iovlen = 6; /* Linux msg_iovlen is a size_t */ + struct iovec iov[6]; + packet_queue_to_iovec(c->writequeue, iov, &iovlen); + message.msg_iov = iov; message.msg_iovlen = iovlen; res = sendmsg(c->sock, &message, MSG_FASTOPEN); if (res < 0 && errno != EINPROGRESS) { + m_free(c->errstring); + c->errstring = m_strdup(strerror(errno)); /* Not entirely sure which kind of errors are normal - 2.6.32 seems to return EPIPE for any (nonblocking?) sendmsg(). just fall back */ TRACE(("sendmsg tcp_fastopen failed, falling back. %s", strerror(errno))); @@ -112,7 +117,6 @@ /* Set to NULL to avoid trying again */ c->writequeue = NULL; } - m_free(message.msg_iov); packet_queue_consume(c->writequeue, res); } #endif @@ -124,6 +128,8 @@ if (res < 0 && errno != EINPROGRESS) { /* failure */ + m_free(c->errstring); + c->errstring = m_strdup(strerror(errno)); close(c->sock); c->sock = -1; continue; @@ -237,7 +243,7 @@ c->sock = -1; m_free(c->errstring); - c->errstring = strerror(val); + c->errstring = m_strdup(strerror(val)); } else { /* New connection has been established */ c->cb(DROPBEAR_SUCCESS, c->sock, c->cb_data, NULL); @@ -254,8 +260,7 @@ c->writequeue = writequeue; } -struct iovec * packet_queue_to_iovec(struct Queue *queue, int *ret_iov_count) { - struct iovec *iov = NULL; +void packet_queue_to_iovec(struct Queue *queue, struct iovec *iov, unsigned int *iov_count) { struct Link *l; unsigned int i; int len; @@ -265,10 +270,9 @@ #define IOV_MAX UIO_MAXIOV #endif - *ret_iov_count = MIN(queue->count, IOV_MAX); + *iov_count = MIN(MIN(queue->count, IOV_MAX), *iov_count); - iov = m_malloc(sizeof(*iov) * *ret_iov_count); - for (l = queue->head, i = 0; l; l = l->link, i++) + for (l = queue->head, i = 0; i < *iov_count; l = l->link, i++) { writebuf = (buffer*)l->item; len = writebuf->len - 1 - writebuf->pos; @@ -278,8 +282,6 @@ iov[i].iov_base = buf_getptr(writebuf, len); iov[i].iov_len = len; } - - return iov; } void packet_queue_consume(struct Queue *queue, ssize_t written) { diff -r 210982935887 -r 26c1f9b8f82e netio.h --- a/netio.h Mon Mar 02 21:17:41 2015 +0800 +++ b/netio.h Tue Apr 14 20:44:30 2015 +0800 @@ -44,7 +44,8 @@ void connect_set_writequeue(struct dropbear_progress_connection *c, struct Queue *writequeue); /* TODO: writev #ifdef guard */ -struct iovec * packet_queue_to_iovec(struct Queue *queue, int *ret_iov_count); +/* Fills out iov which contains iov_count slots, returning the number filled in iov_count */ +void packet_queue_to_iovec(struct Queue *queue, struct iovec *iov, unsigned int *iov_count); void packet_queue_consume(struct Queue *queue, ssize_t written); #ifdef DROPBEAR_TCP_FAST_OPEN diff -r 210982935887 -r 26c1f9b8f82e packet.c --- a/packet.c Mon Mar 02 21:17:41 2015 +0800 +++ b/packet.c Tue Apr 14 20:44:30 2015 +0800 @@ -58,8 +58,9 @@ ssize_t written; #ifdef HAVE_WRITEV - struct iovec *iov = NULL; - int iov_count; + /* 50 is somewhat arbitrary */ + unsigned int iov_count = 50; + struct iovec iov[50]; #endif TRACE2(("enter write_packet")) @@ -67,7 +68,7 @@ #if defined(HAVE_WRITEV) && (defined(IOV_MAX) || defined(UIO_MAXIOV)) - iov = packet_queue_to_iovec(&ses.writequeue, &iov_count); + packet_queue_to_iovec(&ses.writequeue, iov, &iov_count); /* This may return EAGAIN. The main loop sometimes calls write_packet() without bothering to test with select() since it's likely to be necessary */ @@ -75,15 +76,14 @@ if (written < 0) { if (errno == EINTR || errno == EAGAIN) { TRACE2(("leave write_packet: EINTR")) - m_free(iov); return; } else { dropbear_exit("Error writing: %s", strerror(errno)); } } - m_free(iov); packet_queue_consume(&ses.writequeue, written); + ses.writequeue_len -= written; if (written == 0) { ses.remoteclosed(); @@ -114,6 +114,8 @@ ses.remoteclosed(); } + ses.writequeue_len -= written; + if (written == len) { /* We've finished with the packet, free it */ dequeue(&ses.writequeue); @@ -571,15 +573,12 @@ /* stick the MAC on it */ buf_putbytes(writebuf, mac_bytes, mac_size); - /* The last byte of the buffer stores the cleartext packet_type. It is not - * transmitted but is used for transmit timeout purposes */ - buf_putbyte(writebuf, packet_type); - /* enqueue the packet for sending. It will get freed after transmission. */ - buf_setpos(writebuf, 0); - enqueue(&ses.writequeue, (void*)writebuf); + /* Update counts */ + ses.kexstate.datatrans += writebuf->len; + + writebuf_enqueue(writebuf, packet_type); /* Update counts */ - ses.kexstate.datatrans += writebuf->len; ses.transseq++; now = monotonic_now(); @@ -597,6 +596,16 @@ TRACE2(("leave encrypt_packet()")) } +void writebuf_enqueue(buffer * writebuf, unsigned char packet_type) { + /* The last byte of the buffer stores the cleartext packet_type. It is not + * transmitted but is used for transmit timeout purposes */ + buf_putbyte(writebuf, packet_type); + /* enqueue the packet for sending. It will get freed after transmission. */ + buf_setpos(writebuf, 0); + enqueue(&ses.writequeue, (void*)writebuf); + ses.writequeue_len += writebuf->len-1; +} + /* Create the packet mac, and append H(seqno|clearbuf) to the output */ /* output_mac must have ses.keys->trans.algo_mac->hashsize bytes. */ diff -r 210982935887 -r 26c1f9b8f82e packet.h --- a/packet.h Mon Mar 02 21:17:41 2015 +0800 +++ b/packet.h Tue Apr 14 20:44:30 2015 +0800 @@ -28,12 +28,15 @@ #include "includes.h" #include "queue.h" +#include "buffer.h" void write_packet(); void read_packet(); void decrypt_packet(); void encrypt_packet(); +void writebuf_enqueue(buffer * writebuf, unsigned char packet_type); + void process_packet(); void maybe_flush_reply_queue(); diff -r 210982935887 -r 26c1f9b8f82e session.h --- a/session.h Mon Mar 02 21:17:41 2015 +0800 +++ b/session.h Tue Apr 14 20:44:30 2015 +0800 @@ -125,6 +125,7 @@ throughout the code, as handlers fill out this buffer with the packet to send. */ struct Queue writequeue; /* A queue of encrypted packets to send */ + unsigned int writequeue_len; /* Number of bytes pending to send in writequeue */ buffer *readbuf; /* From the wire, decrypted in-place */ buffer *payload; /* Post-decompression, the actual SSH packet. May have extra data at the beginning, will be