Don't call ossl_provider_free() without first setting refcnt

The function ossl_provider_free() decrements the refcnt of the
provider and frees it if it has reached 0. This only works if the
refcnt has already been initialised. We must only call
ossl_provider_free() after this initialisation - otherwise it will fail
to free the provider correctly.

Addresses the issue mentioned here:
https://github.com/openssl/openssl/pull/18355#issuecomment-1138741857

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18417)

diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index 53f04400a09aa43946d65e374c3c728841e2e44e..cacc2c4a6a345a25042389d4cf90c8e2657505d0 100644 (file)
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -450,7 +450,15 @@ static OSSL_PROVIDER *provider_new(const char *name,
 #ifndef HAVE_ATOMICS
         || (prov->refcnt_lock = CRYPTO_THREAD_lock_new()) == NULL
 #endif
-        || (prov->opbits_lock = CRYPTO_THREAD_lock_new()) == NULL
+       ) {
+        OPENSSL_free(prov);
+        ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
+        return NULL;
+    }
+
+    prov->refcnt = 1; /* 1 One reference to be returned */
+
+    if ((prov->opbits_lock = CRYPTO_THREAD_lock_new()) == NULL
         || (prov->flag_lock = CRYPTO_THREAD_lock_new()) == NULL
         || (prov->name = OPENSSL_strdup(name)) == NULL
         || (prov->parameters = sk_INFOPAIR_deep_copy(parameters,
@@ -461,7 +469,6 @@ static OSSL_PROVIDER *provider_new(const char *name,
         return NULL;
     }
 
-    prov->refcnt = 1; /* 1 One reference to be returned */
     prov->init_function = init_function;
 
     return prov;