From 97beb77f319f119957235233396627bb22283da0 Mon Sep 17 00:00:00 2001 From: Pauli Date: Sun, 2 Jul 2023 17:19:17 +1000 Subject: [PATCH] fix memory allocation and reference counting issues Reviewed-by: Tomas Mraz Reviewed-by: Kurt Roeckx (Merged from https://github.com/openssl/openssl/pull/21341) --- crypto/bio/bio_lib.c | 1 - crypto/dh/dh_lib.c | 7 +++++-- crypto/dsa/dsa_lib.c | 7 +++++-- crypto/ec/ec_kmeth.c | 9 +++++---- crypto/evp/evp_rand.c | 5 ++++- crypto/provider_core.c | 2 +- crypto/rsa/rsa_lib.c | 7 +++++-- include/internal/refcount.h | 3 ++- ssl/ssl_lib.c | 9 ++++++--- ssl/ssl_sess.c | 11 ++++++----- 10 files changed, 39 insertions(+), 22 deletions(-) diff --git a/crypto/bio/bio_lib.c b/crypto/bio/bio_lib.c index 209b74730e..c0dfc6cc44 100644 --- a/crypto/bio/bio_lib.c +++ b/crypto/bio/bio_lib.c @@ -98,7 +98,6 @@ BIO *BIO_new_ex(OSSL_LIB_CTX *libctx, const BIO_METHOD *method) if (method->create != NULL && !method->create(bio)) { ERR_raise(ERR_LIB_BIO, ERR_R_INIT_FAIL); CRYPTO_free_ex_data(CRYPTO_EX_INDEX_BIO, bio, &bio->ex_data); - CRYPTO_FREE_REF(&bio->references); goto err; } if (method->create == NULL) diff --git a/crypto/dh/dh_lib.c b/crypto/dh/dh_lib.c index f774c04383..d67511f15c 100644 --- a/crypto/dh/dh_lib.c +++ b/crypto/dh/dh_lib.c @@ -85,8 +85,11 @@ static DH *dh_new_intern(ENGINE *engine, OSSL_LIB_CTX *libctx) return NULL; } - if (!CRYPTO_NEW_REF(&ret->references, 1)) - goto err; + if (!CRYPTO_NEW_REF(&ret->references, 1)) { + CRYPTO_THREAD_lock_free(ret->lock); + OPENSSL_free(ret); + return NULL; + } ret->libctx = libctx; ret->meth = DH_get_default_method(); diff --git a/crypto/dsa/dsa_lib.c b/crypto/dsa/dsa_lib.c index 03c4cb0c06..c2ae3bf158 100644 --- a/crypto/dsa/dsa_lib.c +++ b/crypto/dsa/dsa_lib.c @@ -144,8 +144,11 @@ static DSA *dsa_new_intern(ENGINE *engine, OSSL_LIB_CTX *libctx) return NULL; } - if (!CRYPTO_NEW_REF(&ret->references, 1)) - goto err; + if (!CRYPTO_NEW_REF(&ret->references, 1)) { + CRYPTO_THREAD_lock_free(ret->lock); + OPENSSL_free(ret); + return NULL; + } ret->libctx = libctx; ret->meth = DSA_get_default_method(); diff --git a/crypto/ec/ec_kmeth.c b/crypto/ec/ec_kmeth.c index ec68ab154e..3471a82d7c 100644 --- a/crypto/ec/ec_kmeth.c +++ b/crypto/ec/ec_kmeth.c @@ -86,6 +86,11 @@ EC_KEY *ossl_ec_key_new_method_int(OSSL_LIB_CTX *libctx, const char *propq, if (ret == NULL) return NULL; + if (!CRYPTO_NEW_REF(&ret->references, 1)) { + OPENSSL_free(ret); + return NULL; + } + ret->libctx = libctx; if (propq != NULL) { ret->propq = OPENSSL_strdup(propq); @@ -93,9 +98,6 @@ EC_KEY *ossl_ec_key_new_method_int(OSSL_LIB_CTX *libctx, const char *propq, goto err; } - if (!CRYPTO_NEW_REF(&ret->references, 1)) - goto err; - ret->meth = EC_KEY_get_default_method(); #if !defined(OPENSSL_NO_ENGINE) && !defined(FIPS_MODULE) if (engine != NULL) { @@ -133,7 +135,6 @@ EC_KEY *ossl_ec_key_new_method_int(OSSL_LIB_CTX *libctx, const char *propq, return ret; err: - CRYPTO_FREE_REF(&ret->references); EC_KEY_free(ret); return NULL; } diff --git a/crypto/evp/evp_rand.c b/crypto/evp/evp_rand.c index e8dfd32ff7..03458a090f 100644 --- a/crypto/evp/evp_rand.c +++ b/crypto/evp/evp_rand.c @@ -78,7 +78,10 @@ static void *evp_rand_new(void) { EVP_RAND *rand = OPENSSL_zalloc(sizeof(*rand)); - if (rand == NULL || !CRYPTO_NEW_REF(&rand->refcnt, 1)) { + if (rand == NULL) + return NULL; + + if (!CRYPTO_NEW_REF(&rand->refcnt, 1)) { OPENSSL_free(rand); return NULL; } diff --git a/crypto/provider_core.c b/crypto/provider_core.c index 5a7f603037..49a0eb8c46 100644 --- a/crypto/provider_core.c +++ b/crypto/provider_core.c @@ -443,7 +443,7 @@ static OSSL_PROVIDER *provider_new(const char *name, if ((prov = OPENSSL_zalloc(sizeof(*prov))) == NULL) return NULL; if (!CRYPTO_NEW_REF(&prov->refcnt, 1)) { - ossl_provider_free(prov); + OPENSSL_free(prov); return NULL; } #ifndef HAVE_ATOMICS diff --git a/crypto/rsa/rsa_lib.c b/crypto/rsa/rsa_lib.c index c591b8941d..1601e92ddb 100644 --- a/crypto/rsa/rsa_lib.c +++ b/crypto/rsa/rsa_lib.c @@ -86,8 +86,11 @@ static RSA *rsa_new_intern(ENGINE *engine, OSSL_LIB_CTX *libctx) return NULL; } - if (!CRYPTO_NEW_REF(&ret->references, 1)) - goto err; + if (!CRYPTO_NEW_REF(&ret->references, 1)) { + CRYPTO_THREAD_lock_free(ret->lock); + OPENSSL_free(ret); + return NULL; + } ret->libctx = libctx; ret->meth = RSA_get_default_method(); diff --git a/include/internal/refcount.h b/include/internal/refcount.h index 9740fb10ce..fbab72e430 100644 --- a/include/internal/refcount.h +++ b/include/internal/refcount.h @@ -230,7 +230,8 @@ static ossl_unused ossl_inline int CRYPTO_NEW_REF(CRYPTO_REF_COUNT *refcnt, int static ossl_unused ossl_inline void CRYPTO_FREE_REF(CRYPTO_REF_COUNT *refcnt) \ { - CRYPTO_THREAD_lock_free(refcnt->lock); + if (refcnt != NULL) + CRYPTO_THREAD_lock_free(refcnt->lock); } # else /* OPENSSL_THREADS */ diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 6caa1f5fe0..e14eeffd1b 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -3781,6 +3781,7 @@ SSL_CTX *SSL_CTX_new_ex(OSSL_LIB_CTX *libctx, const char *propq, if (!OPENSSL_init_ssl(OPENSSL_INIT_LOAD_SSL_STRINGS, NULL)) return NULL; + /* Doing this for the run once effect */ if (SSL_get_ex_data_X509_STORE_CTX_idx() < 0) { ERR_raise(ERR_LIB_SSL, SSL_R_X509_VERIFICATION_SETUP_PROBLEMS); goto err; @@ -3788,11 +3789,13 @@ SSL_CTX *SSL_CTX_new_ex(OSSL_LIB_CTX *libctx, const char *propq, ret = OPENSSL_zalloc(sizeof(*ret)); if (ret == NULL) - goto err; + return NULL; /* Init the reference counting before any call to SSL_CTX_free */ - if (!CRYPTO_NEW_REF(&ret->references, 1)) - goto err; + if (!CRYPTO_NEW_REF(&ret->references, 1)) { + OPENSSL_free(ret); + return NULL; + } ret->lock = CRYPTO_THREAD_lock_new(); if (ret->lock == NULL) { diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 486d938c94..3dcc4d81e5 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -141,9 +141,8 @@ SSL_SESSION *ssl_session_dup(const SSL_SESSION *src, int ticket) SSL_SESSION *dest; dest = OPENSSL_malloc(sizeof(*dest)); - if (dest == NULL) { - goto err; - } + if (dest == NULL) + return NULL; memcpy(dest, src, sizeof(*dest)); /* @@ -171,8 +170,10 @@ SSL_SESSION *ssl_session_dup(const SSL_SESSION *src, int ticket) dest->next = NULL; dest->owner = NULL; - if (!CRYPTO_NEW_REF(&dest->references, 1)) - goto err; + if (!CRYPTO_NEW_REF(&dest->references, 1)) { + OPENSSL_free(dest); + return NULL; + } if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_SSL_SESSION, dest, &dest->ex_data)) { ERR_raise(ERR_LIB_SSL, ERR_R_CRYPTO_LIB); -- 2.34.1