]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Fix a deadlock while attempting to get the Primary EVP_RAND_CTX
authorMatt Caswell <matt@openssl.org>
Tue, 15 Apr 2025 14:03:36 +0000 (15:03 +0100)
committerTomas Mraz <tomas@openssl.org>
Wed, 30 Apr 2025 08:55:44 +0000 (10:55 +0200)
When only the FIPS provider was loaded we could get a deadlock when
calling RAND_get0_primary() due to attempting to obtain a recursive
lock.

We reduce the scope of the locks that we hold to avoid this.

Fixes #27391

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27408)

crypto/rand/rand_lib.c

index 9233322b5ff54b811ad9c7f0b2823af4b5100d67..365774b479297a57e95fe11105a6f0b0406481d7 100644 (file)
@@ -755,7 +755,7 @@ static EVP_RAND_CTX *rand_new_crngt(OSSL_LIB_CTX *libctx, EVP_RAND_CTX *parent)
  */
 static EVP_RAND_CTX *rand_get0_primary(OSSL_LIB_CTX *ctx, RAND_GLOBAL *dgbl)
 {
-    EVP_RAND_CTX *ret;
+    EVP_RAND_CTX *ret, *seed, *newseed = NULL, *primary;
 
     if (dgbl == NULL)
         return NULL;
@@ -764,34 +764,26 @@ static EVP_RAND_CTX *rand_get0_primary(OSSL_LIB_CTX *ctx, RAND_GLOBAL *dgbl)
         return NULL;
 
     ret = dgbl->primary;
+    seed = dgbl->seed;
     CRYPTO_THREAD_unlock(dgbl->lock);
 
     if (ret != NULL)
         return ret;
 
-    if (!CRYPTO_THREAD_write_lock(dgbl->lock))
-        return NULL;
-
-    ret = dgbl->primary;
-    if (ret != NULL) {
-        CRYPTO_THREAD_unlock(dgbl->lock);
-        return ret;
-    }
-
 #if !defined(FIPS_MODULE) || !defined(OPENSSL_NO_FIPS_JITTER)
     /* Create a seed source for libcrypto or jitter enabled FIPS provider */
-    if (dgbl->seed == NULL) {
+    if (seed == NULL) {
         ERR_set_mark();
-        dgbl->seed = rand_new_seed(ctx);
+        seed = newseed = rand_new_seed(ctx);
         ERR_pop_to_mark();
     }
 #endif  /* !FIPS_MODULE || !OPENSSL_NO_FIPS_JITTER */
 
 #if defined(FIPS_MODULE)
     /* The FIPS provider has entropy health tests instead of the primary */
-    ret = rand_new_crngt(ctx, dgbl->seed);
+    ret = rand_new_crngt(ctx, seed);
 #else   /* FIPS_MODULE */
-    ret = rand_new_drbg(ctx, dgbl->seed, PRIMARY_RESEED_INTERVAL,
+    ret = rand_new_drbg(ctx, seed, PRIMARY_RESEED_INTERVAL,
                         PRIMARY_RESEED_TIME_INTERVAL);
 #endif  /* FIPS_MODULE */
 
@@ -799,12 +791,30 @@ static EVP_RAND_CTX *rand_get0_primary(OSSL_LIB_CTX *ctx, RAND_GLOBAL *dgbl)
      * The primary DRBG may be shared between multiple threads so we must
      * enable locking.
      */
-    dgbl->primary = ret;
-    if (ret != NULL && !EVP_RAND_enable_locking(ret)) {
-        ERR_raise(ERR_LIB_EVP, EVP_R_UNABLE_TO_ENABLE_LOCKING);
+    if (ret == NULL || !EVP_RAND_enable_locking(ret)) {
+        if (ret != NULL) {
+            ERR_raise(ERR_LIB_EVP, EVP_R_UNABLE_TO_ENABLE_LOCKING);
+            EVP_RAND_CTX_free(ret);
+        }
+        if (newseed == NULL)
+            return NULL;
+        /* else carry on and store seed */
+        ret = NULL;
+    }
+
+    if (!CRYPTO_THREAD_write_lock(dgbl->lock))
+        return NULL;
+
+    primary = dgbl->primary;
+    if (primary != NULL) {
+        CRYPTO_THREAD_unlock(dgbl->lock);
         EVP_RAND_CTX_free(ret);
-        ret = dgbl->primary = NULL;
+        EVP_RAND_CTX_free(newseed);
+        return primary;
     }
+    if (newseed != NULL)
+        dgbl->seed = newseed;
+    dgbl->primary = ret;
     CRYPTO_THREAD_unlock(dgbl->lock);
 
     return ret;