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 <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19424)

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 5b71d6dc0a2e891def06e46fe87c46a52dd808a2..b2a68c9f348dcb8dd022837b4c6191ca39886c22 100644 (file)
--- 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)