Reduce the number of mallocs in dtls1_new() by allocating message queues together...

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26150)

diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c
index 681c50889a9d548c6728d91b36689d79eb6f0f8e..91419ae8908e08ff88d3b3df6188698962960197 100644 (file)
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -95,25 +95,13 @@ int dtls1_new(SSL *ssl)
         return 0;
     }
 
-    d1->rcvd_messages = pqueue_new();
-    d1->sent_messages = pqueue_new();
-
-    if (s->server) {
+    if (s->server)
         d1->cookie_len = sizeof(s->d1->cookie);
-    }
 
     d1->link_mtu = 0;
     d1->mtu = 0;
     d1->hello_verify_request = SSL_HVR_NONE;
 
-    if (d1->rcvd_messages == NULL || d1->sent_messages == NULL) {
-        pqueue_free(d1->rcvd_messages);
-        pqueue_free(d1->sent_messages);
-        OPENSSL_free(d1);
-        ssl3_free(ssl);
-        return 0;
-    }
-
     s->d1 = d1;
 
     if (!ssl->method->ssl_clear(ssl))
@@ -132,8 +120,9 @@ void dtls1_clear_received_buffer(SSL_CONNECTION *s)
 {
     pitem *item = NULL;
     hm_fragment *frag = NULL;
+    pqueue *rcvd_messages = &s->d1->rcvd_messages;
 
-    while ((item = pqueue_pop(s->d1->rcvd_messages)) != NULL) {
+    while ((item = pqueue_pop(rcvd_messages)) != NULL) {
         frag = (hm_fragment *)item->data;
         dtls1_hm_fragment_free(frag);
         pitem_free(item);
@@ -143,8 +132,9 @@ void dtls1_clear_received_buffer(SSL_CONNECTION *s)
 void dtls1_clear_sent_buffer(SSL_CONNECTION *s)
 {
     pitem *item = NULL;
+    pqueue *sent_messages = &s->d1->sent_messages;
 
-    while ((item = pqueue_pop(s->d1->sent_messages)) != NULL) {
+    while ((item = pqueue_pop(sent_messages)) != NULL) {
         dtls_sent_msg *sent_msg = (dtls_sent_msg *)item->data;
 
         if (sent_msg->record_type == SSL3_RT_CHANGE_CIPHER_SPEC
@@ -170,24 +160,17 @@ void dtls1_free(SSL *ssl)
     if (s == NULL)
         return;
 
-    if (s->d1 != NULL) {
+    if (s->d1 != NULL)
         dtls1_clear_queues(s);
-        pqueue_free(s->d1->rcvd_messages);
-        pqueue_free(s->d1->sent_messages);
-    }
 
     DTLS_RECORD_LAYER_free(&s->rlayer);
-
     ssl3_free(ssl);
-
     OPENSSL_free(s->d1);
     s->d1 = NULL;
 }
 
 int dtls1_clear(SSL *ssl)
 {
-    pqueue *rcvd_messages;
-    pqueue *sent_messages;
     size_t mtu;
     size_t link_mtu;
 
@@ -201,8 +184,6 @@ int dtls1_clear(SSL *ssl)
     if (s->d1) {
         DTLS_timer_cb timer_cb = s->d1->timer_cb;
 
-        rcvd_messages = s->d1->rcvd_messages;
-        sent_messages = s->d1->sent_messages;
         mtu = s->d1->mtu;
         link_mtu = s->d1->link_mtu;
 
@@ -221,9 +202,6 @@ int dtls1_clear(SSL *ssl)
             s->d1->mtu = mtu;
             s->d1->link_mtu = link_mtu;
         }
-
-        s->d1->rcvd_messages = rcvd_messages;
-        s->d1->sent_messages = sent_messages;
     }
 
     if (!ssl3_clear(ssl))

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index b4f89580deb29ac544f970d7c46cbeb238b6a5e1..37de3393af5365502c714cfa55fe769754a71727 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1461,7 +1461,7 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
      */
     if (!SSL_CONNECTION_IS_DTLS(s)
             || direction == OSSL_RECORD_DIRECTION_READ
-            || pqueue_peek(s->d1->sent_messages) == NULL) {
+            || pqueue_peek(&s->d1->sent_messages) == NULL) {
         if (*thismethod != NULL && !(*thismethod)->free(*thisrl)) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
             return 0;

diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index 3c20ab516aacd665a5f294b35793b5b956558463..e8de306537555c1e1925c085479cf7fbaf0bf6c3 100644 (file)
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -2017,9 +2017,9 @@ typedef struct dtls1_state_st {
     unsigned short next_handshake_write_seq;
     unsigned short handshake_read_seq;
     /* Buffered received handshake messages */
-    pqueue *rcvd_messages;
+    pqueue rcvd_messages;
     /* Buffered (sent) handshake records */
-    pqueue *sent_messages;
+    pqueue sent_messages;
     /* Flag to indicate current HelloVerifyRequest status */
     enum {SSL_HVR_NONE = 0, SSL_HVR_RECEIVED, SSL_HVR_SENT} hello_verify_request;
     DOWNGRADE downgrade_after_hvr; /* Only used by a stateful server */

diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c
index 819881174a1a0976aa0db72bbf25b355c6ce3d3f..84365c1b6001f444b00af3f108a1c3b609bfba20 100644 (file)
--- a/ssl/statem/statem_dtls.c
+++ b/ssl/statem/statem_dtls.c
@@ -492,8 +492,9 @@ static int dtls1_retrieve_buffered_fragment(SSL_CONNECTION *s, size_t *len)
     hm_fragment *frag;
     int ret;
     int chretran = 0;
+    pqueue *rcvd_messages = &s->d1->rcvd_messages;
 
-    iter = pqueue_iterator(s->d1->rcvd_messages);
+    iter = pqueue_iterator(rcvd_messages);
     do {
         item = pqueue_next(&iter);
         if (item == NULL)
@@ -514,7 +515,7 @@ static int dtls1_retrieve_buffered_fragment(SSL_CONNECTION *s, size_t *len)
                  * It is safe to pop this message from the queue even though
                  * we have an active iterator
                  */
-                pqueue_pop(s->d1->rcvd_messages);
+                pqueue_pop(rcvd_messages);
                 dtls1_hm_fragment_free(frag);
                 pitem_free(item);
                 item = NULL;
@@ -534,7 +535,7 @@ static int dtls1_retrieve_buffered_fragment(SSL_CONNECTION *s, size_t *len)
                         * We have fragments for both a ClientHello without
                         * cookie and one with. Ditch the one without.
                         */
-                        pqueue_pop(s->d1->rcvd_messages);
+                        pqueue_pop(rcvd_messages);
                         dtls1_hm_fragment_free(frag);
                         pitem_free(item);
                         item = next;
@@ -555,7 +556,8 @@ static int dtls1_retrieve_buffered_fragment(SSL_CONNECTION *s, size_t *len)
 
     if (s->d1->handshake_read_seq == frag->msg_header.seq || chretran) {
         size_t frag_len = frag->msg_header.frag_len;
-        pqueue_pop(s->d1->rcvd_messages);
+
+        pqueue_pop(rcvd_messages);
 
         /* Calls SSLfatal() as required */
         ret = dtls1_preprocess_fragment(s, &frag->msg_header);
@@ -616,7 +618,7 @@ static int dtls1_reassemble_fragment(SSL_CONNECTION *s,
     memset(seq64be, 0, sizeof(seq64be));
     seq64be[6] = (unsigned char)(msg_hdr->seq >> 8);
     seq64be[7] = (unsigned char)msg_hdr->seq;
-    item = pqueue_find(s->d1->rcvd_messages, seq64be);
+    item = pqueue_find(&s->d1->rcvd_messages, seq64be);
 
     if (item == NULL) {
         frag = dtls1_hm_fragment_new(msg_hdr->msg_len, 1);
@@ -678,7 +680,7 @@ static int dtls1_reassemble_fragment(SSL_CONNECTION *s,
         if (item == NULL)
             goto err;
 
-        item = pqueue_insert(s->d1->rcvd_messages, item);
+        item = pqueue_insert(&s->d1->rcvd_messages, item);
         /*
          * pqueue_insert fails iff a duplicate item is inserted. However,
          * |item| cannot be a duplicate. If it were, |pqueue_find|, above,
@@ -715,7 +717,7 @@ static int dtls1_process_out_of_seq_message(SSL_CONNECTION *s,
     memset(seq64be, 0, sizeof(seq64be));
     seq64be[6] = (unsigned char)(msg_hdr->seq >> 8);
     seq64be[7] = (unsigned char)msg_hdr->seq;
-    item = pqueue_find(s->d1->rcvd_messages, seq64be);
+    item = pqueue_find(&s->d1->rcvd_messages, seq64be);
 
     /*
      * If we already have an entry and this one is a fragment, don't discard
@@ -773,7 +775,7 @@ static int dtls1_process_out_of_seq_message(SSL_CONNECTION *s,
         if (item == NULL)
             goto err;
 
-        item = pqueue_insert(s->d1->rcvd_messages, item);
+        item = pqueue_insert(&s->d1->rcvd_messages, item);
         /*
          * pqueue_insert fails iff a duplicate item is inserted. However,
          * |item| cannot be a duplicate. If it were, |pqueue_find|, above,
@@ -1110,7 +1112,7 @@ int dtls1_get_queue_priority(unsigned short seq, int record_type)
 
 int dtls1_retransmit_sent_messages(SSL_CONNECTION *s)
 {
-    piterator iter = pqueue_iterator(s->d1->sent_messages);
+    piterator iter = pqueue_iterator(&s->d1->sent_messages);
     pitem *item;
     int found = 0;
 
@@ -1179,7 +1181,7 @@ int dtls1_buffer_sent_message(SSL_CONNECTION *s, int record_type)
         return 0;
     }
 
-    pqueue_insert(s->d1->sent_messages, item);
+    pqueue_insert(&s->d1->sent_messages, item);
     return 1;
 }
 
@@ -1198,7 +1200,7 @@ int dtls1_retransmit_message(SSL_CONNECTION *s, unsigned short seq, int *found)
     seq64be[6] = (unsigned char)(seq >> 8);
     seq64be[7] = (unsigned char)seq;
 
-    item = pqueue_find(s->d1->sent_messages, seq64be);
+    item = pqueue_find(&s->d1->sent_messages, seq64be);
     if (item == NULL) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         *found = 0;