Skip to content

Commit

Permalink
Fix memory leak when freeing the DTLS record layer
Browse files Browse the repository at this point in the history
We need to check whether the sent_messages has actually buffered any
messages in it. If not we won't free the old record layer later when we
clear out the old buffered messages and a memory leak will result.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #19586)
  • Loading branch information
mattcaswell authored and hlandau committed Nov 14, 2022
1 parent 22094d1 commit 20c7feb
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
2 changes: 1 addition & 1 deletion ssl/record/methods/dtls_meth.c
Expand Up @@ -684,7 +684,7 @@ dtls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,

err:
if (ret != OSSL_RECORD_RETURN_SUCCESS) {
OPENSSL_free(*retrl);
dtls_free(*retrl);
*retrl = NULL;
}
return ret;
Expand Down
11 changes: 7 additions & 4 deletions ssl/record/rec_layer_s3.c
Expand Up @@ -1356,11 +1356,14 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,

/*
* Free the old record layer if we have one except in the case of DTLS when
* writing. In that case the record layer is still referenced by buffered
* messages for potential retransmit. Only when those buffered messages get
* freed do we free the record layer object (see dtls1_hm_fragment_free)
* writing and there are still buffered sent messages in our queue. In that
* case the record layer is still referenced by those buffered messages for
* potential retransmit. Only when those buffered messages get freed do we
* free the record layer object (see dtls1_hm_fragment_free)
*/
if (!SSL_CONNECTION_IS_DTLS(s) || direction == OSSL_RECORD_DIRECTION_READ) {
if (!SSL_CONNECTION_IS_DTLS(s)
|| direction == OSSL_RECORD_DIRECTION_READ
|| 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;
Expand Down

0 comments on commit 20c7feb

Please sign in to comment.