From 6a233c31ded0badd0eb8ce75608d7ff193b7057c Mon Sep 17 00:00:00 2001 From: Frederik Wedel-Heinen Date: Wed, 11 Dec 2024 13:13:28 +0100 Subject: [PATCH] Reduce the number of mallocs in dtls1_new() by allocating message queues together with the d1 struct. Reviewed-by: Viktor Dukhovni Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/26150) --- ssl/d1_lib.c | 34 ++++++---------------------------- ssl/record/rec_layer_s3.c | 2 +- ssl/ssl_local.h | 4 ++-- ssl/statem/statem_dtls.c | 24 +++++++++++++----------- 4 files changed, 22 insertions(+), 42 deletions(-) diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c index 681c50889a9..91419ae8908 100644 --- 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 b4f89580deb..37de3393af5 100644 --- 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 3c20ab516aa..e8de3065375 100644 --- 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 819881174a1..84365c1b600 100644 --- 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; -- 2.47.2