Move freeing of an old record layer to dtls1_clear_sent_buffer

When we are clearing the sent messages queue we should ensure we free any
old write record layers that are no longer in use. Previously this logic
was in dtls1_hm_fragment_free() - but this can end up freeing the current
record layer under certain error conditions.

Fixes #22664

Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22679)

diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c
index be4ff02a897cd02e8c5e109a40e0488738a6410b..1ac0975d0a8faf6e4c47b8ae2015900734a39373 100644 (file)
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -130,6 +130,17 @@ void dtls1_clear_sent_buffer(SSL_CONNECTION *s)
 
     while ((item = pqueue_pop(s->d1->sent_messages)) != NULL) {
         frag = (hm_fragment *)item->data;
+
+        if (frag->msg_header.is_ccs
+                && frag->msg_header.saved_retransmit_state.wrlmethod != NULL
+                && s->rlayer.wrl != frag->msg_header.saved_retransmit_state.wrl) {
+            /*
+             * If we're freeing the CCS then we're done with the old wrl and it
+             * can bee freed
+             */
+            frag->msg_header.saved_retransmit_state.wrlmethod->free(frag->msg_header.saved_retransmit_state.wrl);
+        }
+
         dtls1_hm_fragment_free(frag);
         pitem_free(item);
     }
@@ -143,16 +154,16 @@ void dtls1_free(SSL *ssl)
     if (s == NULL)
         return;
 
-    DTLS_RECORD_LAYER_free(&s->rlayer);
-
-    ssl3_free(ssl);
-
     if (s->d1 != NULL) {
         dtls1_clear_queues(s);
         pqueue_free(s->d1->buffered_messages);
         pqueue_free(s->d1->sent_messages);
     }
 
+    DTLS_RECORD_LAYER_free(&s->rlayer);
+
+    ssl3_free(ssl);
+
     OPENSSL_free(s->d1);
     s->d1 = NULL;
 }

diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c
index 97d9f4591c131d658d8c7d100246d9d0b1b64cb9..c674ddfb54df534cd2e178e9fc175c646528a999 100644 (file)
--- a/ssl/statem/statem_dtls.c
+++ b/ssl/statem/statem_dtls.c
@@ -94,14 +94,7 @@ void dtls1_hm_fragment_free(hm_fragment *frag)
 {
     if (!frag)
         return;
-    if (frag->msg_header.is_ccs) {
-        /*
-         * If we're freeing the CCS then we're done with the old wrl and it
-         * can bee freed
-         */
-        if (frag->msg_header.saved_retransmit_state.wrlmethod != NULL)
-            frag->msg_header.saved_retransmit_state.wrlmethod->free(frag->msg_header.saved_retransmit_state.wrl);
-    }
+
     OPENSSL_free(frag->fragment);
     OPENSSL_free(frag->reassembly);
     OPENSSL_free(frag);