fix memory allocation and reference counting issues

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/21341)

diff --git a/crypto/bio/bio_lib.c b/crypto/bio/bio_lib.c
index 209b74730eb4322c39a19054084c987efa559eb6..c0dfc6cc44c1ad6f447d2ff47b44917b2665ad12 100644 (file)
--- 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 f774c043835f13950b2a19b73dcaed73d1ee0a71..d67511f15c4e46d6914d5270d00b2ceba418434c 100644 (file)
--- 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 03c4cb0c06cdf6308c03941b0d91cde9ed4562d7..c2ae3bf1584a29915633dcc671b9a1381d1851c5 100644 (file)
--- 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 ec68ab154eed59f25a5778b98f1d5232c6c4f154..3471a82d7c8dee52a9c5783785a9d2b07a86265d 100644 (file)
--- 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 e8dfd32ff71ce32358bf82001181a9180e88cb54..03458a090f3621567536a60633990f716497eec2 100644 (file)
--- 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 5a7f603037b8b4906a7d93760a26be7edef6178b..49a0eb8c46690088ccfd39d1e23c9880f95ec049 100644 (file)
--- 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 c591b8941daf59ea7b6434c2958f11518830f03a..1601e92ddb006a9dc02a5def4671b28bb94e8c25 100644 (file)
--- 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 9740fb10ce4d55e9852a92f586f585628e13e0e2..fbab72e430ab9d94c7c302b2654cf2f58df436c8 100644 (file)
--- 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 6caa1f5fe0dadc1daf08d4145e82afd1a6980950..e14eeffd1beeea5496a82cf902197935f2e4ba2c 100644 (file)
--- 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 486d938c94757abed6da5e98d5135fc24bc3cf17..3dcc4d81e5bc6376b4508a09bde6b1608e97a013 100644 (file)
--- 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);