crypto-api: always allocate memory when serializing iovec_t

The AEAD iov interface falls back to serializing the input buffers if
the low-level cipher doesn't support scatter/gather encryption.
However, there was a bug in the functions used for the serialization,
which causes memory leaks under a certain condition (i.e. the number
of input buffers is 1).

This patch makes the logic of the functions simpler, by removing a
micro-optimization that tries to minimize the number of calls to
malloc/free.

The original problem was reported by Marius Steffen in:
https://bugzilla.samba.org/show_bug.cgi?id=14399
and the cause was investigated by Alexander Haase in:
https://gitlab.com/gnutls/gnutls/-/merge_requests/1277

Signed-off-by: Daiki Ueno <ueno@gnu.org>

diff --git a/lib/crypto-api.c b/lib/crypto-api.c
index 45be64ed1f3d0d236155006a28a3c433b18bacbe..8524f5ed4f13ff17d535fdf3099e96c12a9e7f11 100644 (file)
--- a/lib/crypto-api.c
+++ b/lib/crypto-api.c
@@ -891,32 +891,23 @@ gnutls_aead_cipher_encrypt(gnutls_aead_cipher_hd_t handle,
 struct iov_store_st {
        void *data;
        size_t size;
-       unsigned allocated;
 };
 
 static void iov_store_free(struct iov_store_st *s)
 {
-       if (s->allocated) {
-               gnutls_free(s->data);
-               s->allocated = 0;
-       }
+       gnutls_free(s->data);
 }
 
 static int iov_store_grow(struct iov_store_st *s, size_t length)
 {
-       if (s->allocated || s->data == NULL) {
-               s->size += length;
-               s->data = gnutls_realloc(s->data, s->size);
-               if (s->data == NULL)
-                       return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
-               s->allocated = 1;
-       } else {
-               void *data = s->data;
-               size_t size = s->size + length;
-               s->data = gnutls_malloc(size);
-               memcpy(s->data, data, s->size);
-               s->size += length;
-       }
+       void *data;
+
+       s->size += length;
+       data = gnutls_realloc(s->data, s->size);
+       if (data == NULL)
+               return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
+
+       s->data = data;
        return 0;
 }
 
@@ -926,11 +917,6 @@ copy_from_iov(struct iov_store_st *dst, const giovec_t *iov, int iovcnt)
        memset(dst, 0, sizeof(*dst));
        if (iovcnt == 0) {
                return 0;
-       } else if (iovcnt == 1) {
-               dst->data = iov[0].iov_base;
-               dst->size = iov[0].iov_len;
-               /* implies: dst->allocated = 0; */
-               return 0;
        } else {
                int i;
                uint8_t *p;
@@ -944,11 +930,11 @@ copy_from_iov(struct iov_store_st *dst, const giovec_t *iov, int iovcnt)
 
                p = dst->data;
                for (i=0;i<iovcnt;i++) {
-                       memcpy(p, iov[i].iov_base, iov[i].iov_len);
+                       if (iov[i].iov_len > 0)
+                               memcpy(p, iov[i].iov_base, iov[i].iov_len);
                        p += iov[i].iov_len;
                }
 
-               dst->allocated = 1;
                return 0;
        }
 }

diff --git a/tests/aead-cipher-vec.c b/tests/aead-cipher-vec.c
index fba9010d9ec12fa6ee6686b5d5fbbef451008fdc..6a30a35f7b855737e357495b25f1542cbfbde2bf 100644 (file)
--- a/tests/aead-cipher-vec.c
+++ b/tests/aead-cipher-vec.c
@@ -49,6 +49,7 @@ static void start(const char *name, int algo)
        giovec_t auth_iov[2];
        uint8_t tag[64];
        size_t tag_size = 0;
+       size_t i;
 
        key.data = key16;
        key.size = gnutls_cipher_get_key_size(algo);
@@ -82,21 +83,23 @@ static void start(const char *name, int algo)
        if (ret < 0)
                fail("gnutls_cipher_init: %s\n", gnutls_strerror(ret));
 
-       ret = gnutls_aead_cipher_encryptv2(ch,
-                                          iv.data, iv.size,
-                                          auth_iov, 2,
-                                          iov, 3,
-                                          tag, &tag_size);
-       if (ret < 0)
-               fail("could not encrypt data: %s\n", gnutls_strerror(ret));
-
-       ret = gnutls_aead_cipher_decryptv2(ch,
-                                          iv.data, iv.size,
-                                          auth_iov, 2,
-                                          iov, 3,
-                                          tag, tag_size);
-       if (ret < 0)
-               fail("could not decrypt data: %s\n", gnutls_strerror(ret));
+       for (i = 0; i < 2; i++) {
+               ret = gnutls_aead_cipher_encryptv2(ch,
+                                                  iv.data, iv.size,
+                                                  auth_iov, 2,
+                                                  iov, i + 1,
+                                                  tag, &tag_size);
+               if (ret < 0)
+                       fail("could not encrypt data: %s\n", gnutls_strerror(ret));
+
+               ret = gnutls_aead_cipher_decryptv2(ch,
+                                                  iv.data, iv.size,
+                                                  auth_iov, 2,
+                                                  iov, i + 1,
+                                                  tag, tag_size);
+               if (ret < 0)
+                       fail("could not decrypt data: %s\n", gnutls_strerror(ret));
+       }
 
        gnutls_aead_cipher_deinit(ch);
 }