Browse Source

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)
Matt Caswell 7 months ago
parent
commit
a091bc6022
2 changed files with 16 additions and 12 deletions
  1. 15 4
      ssl/d1_lib.c
  2. 1 8
      ssl/statem/statem_dtls.c

+ 15 - 4
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;
 }

+ 1 - 8
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);