From f2239a4e4b26690b110df082d3616165c94839fc Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Sat, 28 Feb 2015 08:11:33 -0800 Subject: [PATCH] libdmsg - Fix buffer indexing bug in crypted path * Fix a buffering index bug in the crypted path which causes a buffer overrun and/or implodes the connection on a protocol error. --- lib/libdmsg/crypto.c | 9 +++++++-- lib/libdmsg/msg.c | 19 ++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/lib/libdmsg/crypto.c b/lib/libdmsg/crypto.c index 55eca1e3c5..e96ef18e37 100644 --- a/lib/libdmsg/crypto.c +++ b/lib/libdmsg/crypto.c @@ -751,8 +751,13 @@ dmsg_crypto_encrypt(dmsg_iocom_t *iocom __unused, dmsg_ioq_t *ioq, used += DMSG_CRYPTO_CHUNK_SIZE; p_len -= DMSG_CRYPTO_CHUNK_SIZE; - ioq->fifo_cdx += (size_t)ct_used; /* crypted count */ - ioq->fifo_cdn += (size_t)ct_used; /* crypted count */ + /* + * NOTE: crypted count will eventually differ from + * nmax, but for now we have not yet introduced + * random armor. + */ + ioq->fifo_cdx += (size_t)ct_used; + ioq->fifo_cdn += (size_t)ct_used; ioq->fifo_end += (size_t)ct_used; nmax -= (size_t)ct_used; #ifdef CRYPTO_DEBUG diff --git a/lib/libdmsg/msg.c b/lib/libdmsg/msg.c index 1bcdef6ad1..8e74c391c2 100644 --- a/lib/libdmsg/msg.c +++ b/lib/libdmsg/msg.c @@ -1359,8 +1359,13 @@ dmsg_iocom_flush2(dmsg_iocom_t *iocom) * be greater depending on the crypto mechanic. iov[] is adjusted * to point at the FIFO if necessary. * - * NOTE: The return value from the writev() is the post-encrypted - * byte count, not the plaintext count. + * NOTE: nact is the number of bytes eaten from the message. For + * encrypted data this is the number of bytes processed for + * encryption and not necessarily the number of bytes writable. + * The return value from the writev() is the post-encrypted + * byte count which might be larger. + * + * NOTE: For direct writes, nact is the return value from the writev(). */ if (iocom->flags & DMSG_IOCOMF_CRYPTED) { /* @@ -1382,12 +1387,16 @@ dmsg_iocom_flush2(dmsg_iocom_t *iocom) ioq->fifo_beg = 0; } + /* + * beg .... cdx ............ cdn ............. end + * [WRITABLE] [PARTIALENCRYPT] [NOTYETENCRYPTED] + * + * Advance beg on a successful write. + */ iovcnt = dmsg_crypto_encrypt(iocom, ioq, iov, iovcnt, &nact); n = writev(iocom->sock_fd, iov, iovcnt); if (n > 0) { ioq->fifo_beg += n; - ioq->fifo_cdn += n; - ioq->fifo_cdx += n; if (ioq->fifo_beg == ioq->fifo_end) { ioq->fifo_beg = 0; ioq->fifo_cdn = 0; @@ -1405,7 +1414,7 @@ dmsg_iocom_flush2(dmsg_iocom_t *iocom) /* * In this situation we are not staging the messages to the * FIFO but instead writing them directly from the msg - * structure(s), so (nact) is basically (n). + * structure(s) unencrypted, so (nact) is basically (n). */ n = writev(iocom->sock_fd, iov, iovcnt); if (n > 0) -- 2.41.0