Fix an issue in provider_activate_fallbacks()

The above function was running while holding the store lock with a read
lock. Unfortunately it actually modifies the store, so a write lock is
required instead.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13660)

diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index 79c548e52dfee4bebc9b672e57edb06f1df8f2aa..91cfcaa9eca1e8120710c78d135e696af11feef9 100644 (file)
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -729,34 +729,50 @@ static int provider_forall_loaded(struct provider_store_st *store,
  */
 static void provider_activate_fallbacks(struct provider_store_st *store)
 {
-    if (store->use_fallbacks) {
-        int num_provs = sk_OSSL_PROVIDER_num(store->providers);
-        int activated_fallback_count = 0;
-        int i;
+    int use_fallbacks;
+    int num_provs;
+    int activated_fallback_count = 0;
+    int i;
+
+    CRYPTO_THREAD_read_lock(store->lock);
+    use_fallbacks = store->use_fallbacks;
+    CRYPTO_THREAD_unlock(store->lock);
+    if (!use_fallbacks)
+        return;
+
+    CRYPTO_THREAD_write_lock(store->lock);
+    /* Check again, just in case another thread changed it */
+    use_fallbacks = store->use_fallbacks;
+    if (!use_fallbacks) {
+        CRYPTO_THREAD_unlock(store->lock);
+        return;
+    }
 
-        for (i = 0; i < num_provs; i++) {
-            OSSL_PROVIDER *prov = sk_OSSL_PROVIDER_value(store->providers, i);
+    num_provs = sk_OSSL_PROVIDER_num(store->providers);
+    for (i = 0; i < num_provs; i++) {
+        OSSL_PROVIDER *prov = sk_OSSL_PROVIDER_value(store->providers, i);
 
-            if (ossl_provider_up_ref(prov)) {
-                if (prov->flag_fallback) {
-                    if (provider_activate(prov)) {
-                        prov->flag_activated_as_fallback = 1;
-                        activated_fallback_count++;
-                    }
+        if (ossl_provider_up_ref(prov)) {
+            if (prov->flag_fallback) {
+                if (provider_activate(prov)) {
+                    prov->flag_activated_as_fallback = 1;
+                    activated_fallback_count++;
                 }
-                ossl_provider_free(prov);
             }
+            ossl_provider_free(prov);
         }
-
-        /*
-         * We assume that all fallbacks have been added to the store before
-         * any fallback is activated.
-         * TODO: We may have to reconsider this, IF we find ourselves adding
-         * fallbacks after any previous fallback has been activated.
-         */
-        if (activated_fallback_count > 0)
-            store->use_fallbacks = 0;
     }
+
+    /*
+     * We assume that all fallbacks have been added to the store before
+     * any fallback is activated.
+     * TODO: We may have to reconsider this, IF we find ourselves adding
+     * fallbacks after any previous fallback has been activated.
+     */
+    if (activated_fallback_count > 0)
+        store->use_fallbacks = 0;
+
+    CRYPTO_THREAD_unlock(store->lock);
 }
 
 int ossl_provider_forall_loaded(OSSL_LIB_CTX *ctx,
@@ -776,10 +792,9 @@ int ossl_provider_forall_loaded(OSSL_LIB_CTX *ctx,
 #endif
 
     if (store != NULL) {
-        CRYPTO_THREAD_read_lock(store->lock);
-
         provider_activate_fallbacks(store);
 
+        CRYPTO_THREAD_read_lock(store->lock);
         /*
          * Now, we sweep through all providers
          */
@@ -794,9 +809,7 @@ int ossl_provider_forall_loaded(OSSL_LIB_CTX *ctx,
 int ossl_provider_available(OSSL_PROVIDER *prov)
 {
     if (prov != NULL) {
-        CRYPTO_THREAD_read_lock(prov->store->lock);
         provider_activate_fallbacks(prov->store);
-        CRYPTO_THREAD_unlock(prov->store->lock);
 
         return prov->flag_activated;
     }