libdmsg - Fix buffer indexing bug in crypted path
authorMatthew Dillon <dillon@apollo.backplane.com>
Sat, 28 Feb 2015 16:11:33 +0000 (08:11 -0800)
committerMatthew Dillon <dillon@apollo.backplane.com>
Sat, 28 Feb 2015 16:11:33 +0000 (08:11 -0800)
* 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
lib/libdmsg/msg.c

index 55eca1e..e96ef18 100644 (file)
@@ -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
index 1bcdef6..8e74c39 100644 (file)
@@ -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)