Commit c6048af2 authored by Corey Minyard's avatar Corey Minyard Committed by Pauli
Browse files

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: default avatarCorey Minyard <minyard@acm.org>

Reviewed-by: default avatarBernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: default avatarPaul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8051)
parent 37842dfa

crypto/bio/bss_mem.c

+14 −10
Original line number Diff line number Diff line
@@ -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) {

test/bio_memleak_test.c

0 → 100644
+54 −0
Original line number Diff line number Diff line 
/*

 * 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;

}

test/build.info

+5 −1
Original line number Diff line number Diff line
@@ -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

test/recipes/90-test_bio_memleak.t

0 → 100644
+12 −0
Original line number Diff line number Diff line 
#! /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");