Fix a memory leak in the mem bio

If you use a BIO and set up your own buffer that is not freed, the
memory bio will leak the BIO_BUF_MEM object it allocates.

The trouble is that the BIO_BUF_MEM is allocated and kept around,
but it is not freed if BIO_NOCLOSE is set.

The freeing of BIO_BUF_MEM was fairly confusing, simplify things
so mem_buf_free only frees the memory buffer and free the BIO_BUF_MEM
in mem_free(), where it should be done.

Alse add a test for a leak in the memory bio
Setting a memory buffer caused a leak.

Signed-off-by: Corey Minyard <minyard@acm.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8051)

diff --git a/crypto/bio/bss_mem.c b/crypto/bio/bss_mem.c
index ee9ea917a03e9defa37de08fa950f861f075a6d6..89c54b2d53df58b9fad0a2ddfcbaa52071153314 100644 (file)
--- a/crypto/bio/bss_mem.c
+++ b/crypto/bio/bss_mem.c
@@ -20,7 +20,7 @@ static long mem_ctrl(BIO *h, int cmd, long arg1, void *arg2);
 static int mem_new(BIO *h);
 static int secmem_new(BIO *h);
 static int mem_free(BIO *data);
-static int mem_buf_free(BIO *data, int free_all);
+static int mem_buf_free(BIO *data);
 static int mem_buf_sync(BIO *h);
 
 static const BIO_METHOD mem_method = {
@@ -140,10 +140,20 @@ static int secmem_new(BIO *bi)
 
 static int mem_free(BIO *a)
 {
-    return mem_buf_free(a, 1);
+    BIO_BUF_MEM *bb;
+
+    if (a == NULL)
+        return 0;
+
+    bb = (BIO_BUF_MEM *)a->ptr;
+    if (!mem_buf_free(a))
+        return 0;
+    OPENSSL_free(bb->readp);
+    OPENSSL_free(bb);
+    return 1;
 }
 
-static int mem_buf_free(BIO *a, int free_all)
+static int mem_buf_free(BIO *a)
 {
     if (a == NULL)
         return 0;
@@ -155,11 +165,6 @@ static int mem_buf_free(BIO *a, int free_all)
         if (a->flags & BIO_FLAGS_MEM_RDONLY)
             b->data = NULL;
         BUF_MEM_free(b);
-        if (free_all) {
-            OPENSSL_free(bb->readp);
-            OPENSSL_free(bb);
-        }
-        a->ptr = NULL;
     }
     return 1;
 }
@@ -266,11 +271,10 @@ static long mem_ctrl(BIO *b, int cmd, long num, void *ptr)
         }
         break;
     case BIO_C_SET_BUF_MEM:
-        mem_buf_free(b, 0);
+        mem_buf_free(b);
         b->shutdown = (int)num;
         bbm->buf = ptr;
         *bbm->readp = *bbm->buf;
-        b->ptr = bbm;
         break;
     case BIO_C_GET_BUF_MEM_PTR:
         if (ptr != NULL) {

diff --git a/test/bio_memleak_test.c b/test/bio_memleak_test.c
new file mode 100644 (file)
index 0000000..36680e3
--- /dev/null
+++ b/test/bio_memleak_test.c
@@ -0,0 +1,54 @@
+/*
+ * Copyright 2018 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the OpenSSL license (the "License").  You may not use
+ * this file except in compliance with the License.  You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+#include <stdio.h>
+#include <string.h>
+#include <openssl/buffer.h>
+#include <openssl/bio.h>
+
+#include "testutil.h"
+
+static int test_bio_memleak(void)
+{
+    int ok = 0;
+    BIO *bio;
+    BUF_MEM bufmem;
+    const char *str = "BIO test\n";
+    char buf[100];
+
+    bio = BIO_new(BIO_s_mem());
+    if (bio == NULL)
+        goto finish;
+    bufmem.length = strlen(str) + 1;
+    bufmem.data = (char *) str;
+    bufmem.max = bufmem.length;
+    BIO_set_mem_buf(bio, &bufmem, BIO_NOCLOSE);
+    BIO_set_flags(bio, BIO_FLAGS_MEM_RDONLY);
+
+    if (BIO_read(bio, buf, sizeof(buf)) <= 0)
+       goto finish;
+
+    ok = strcmp(buf, str) == 0;
+
+finish:
+    BIO_free(bio);
+    return ok;
+}
+
+int global_init(void)
+{
+    CRYPTO_set_mem_debug(1);
+    CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON);
+    return 1;
+}
+
+int setup_tests(void)
+{
+    ADD_TEST(test_bio_memleak);
+    return 1;
+}

diff --git a/test/build.info b/test/build.info
index 962af11eef3aa47b560eabfd8cd7558ef4155a57..2e17a5fb8c4f4b34ac55c1135a316a786829a091 100644 (file)
--- a/test/build.info
+++ b/test/build.info
@@ -42,7 +42,7 @@ INCLUDE_MAIN___test_libtestutil_OLB = /INCLUDE=main
           packettest asynctest secmemtest srptest memleaktest stack_test \
           dtlsv1listentest ct_test threadstest afalgtest d2i_test \
           ssl_test_ctx_test ssl_test x509aux cipherlist_test asynciotest \
-          bio_callback_test \
+          bio_callback_test bio_memleak_test \
           bioprinttest sslapitest dtlstest sslcorrupttest bio_enc_test \
           pkey_meth_test pkey_meth_kdf_test uitest cipherbytes_test \
           asn1_encode_test asn1_decode_test asn1_string_table_test \
@@ -300,6 +300,10 @@ INCLUDE_MAIN___test_libtestutil_OLB = /INCLUDE=main
   INCLUDE[bio_callback_test]=../include
   DEPEND[bio_callback_test]=../libcrypto libtestutil.a
 
+  SOURCE[bio_memleak_test]=bio_memleak_test.c
+  INCLUDE[bio_memleak_test]=../include
+  DEPEND[bio_memleak_test]=../libcrypto libtestutil.a
+
   SOURCE[bioprinttest]=bioprinttest.c
   INCLUDE[bioprinttest]=../include
   DEPEND[bioprinttest]=../libcrypto libtestutil.a

diff --git a/test/recipes/90-test_bio_memleak.t b/test/recipes/90-test_bio_memleak.t
new file mode 100644 (file)
index 0000000..93f7f92
--- /dev/null
+++ b/test/recipes/90-test_bio_memleak.t
@@ -0,0 +1,12 @@
+#! /usr/bin/env perl
+# Copyright 2018 The OpenSSL Project Authors. All Rights Reserved.
+#
+# Licensed under the OpenSSL license (the "License").  You may not use
+# this file except in compliance with the License.  You can obtain a copy
+# in the file LICENSE in the source distribution or at
+# https://www.openssl.org/source/license.html
+
+
+use OpenSSL::Test::Simple;
+
+simple_test("test_bio_memleak", "bio_memleak_test");