From: Matt Caswell Date: Tue, 18 Oct 2022 11:23:40 +0000 (+0100) Subject: Move freeing of BIOs as late as possible X-Git-Tag: openssl-3.2.0-alpha1~1873 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=cd6e89b6b6ebe204cc442da9b563213bd67eb27f;p=thirdparty%2Fopenssl.git Move freeing of BIOs as late as possible Calling SSL_free() will call BIO_free_all() on the rbio and wbio. We keep references to the rbio and wbio inside the record layer object. References to that object are held directly, as well as in fragment retransmission queues. We need to ensure all record layer objects are cleaned up before we call BIO_free_all() on rbio/wbio - otherwise the "top" BIO may not have its reference count drop to 0 when BIO_free_all() is called. This means that the rest of the BIOs in the chain don't get freed and a memory leak can occur. Reviewed-by: Richard Levitte Reviewed-by: Tomas Mraz Reviewed-by: Hugo Landau (Merged from https://github.com/openssl/openssl/pull/19424) --- diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 5b71d6dc0a2..b2a68c9f348 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -1355,11 +1355,6 @@ void ossl_ssl_connection_free(SSL *ssl) RECORD_LAYER_clear(&s->rlayer); - BIO_free_all(s->wbio); - s->wbio = NULL; - BIO_free_all(s->rbio); - s->rbio = NULL; - BUF_MEM_free(s->init_buf); /* add extra stuff */ @@ -1422,6 +1417,17 @@ void ossl_ssl_connection_free(SSL *ssl) #ifndef OPENSSL_NO_SRTP sk_SRTP_PROTECTION_PROFILE_free(s->srtp_profiles); #endif + + /* + * We do this late. We want to ensure that any other references we held to + * these BIOs are freed first *before* we call BIO_free_all(), because + * BIO_free_all() will only free each BIO in the chain if the number of + * references to the first BIO have dropped to 0 + */ + BIO_free_all(s->wbio); + s->wbio = NULL; + BIO_free_all(s->rbio); + s->rbio = NULL; } void SSL_set0_rbio(SSL *s, BIO *rbio)