Set use_fallbacks to zero when we add a provider to the store

Update use_fallbacks to zero when we add a provider to the store rather
than when we activate it. Its only at the point that we add it to the store
that it is actually usable and visible to other threads.

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

7 files changed, 20 insertions, 33 deletions

diff --git a/crypto/provider.c b/crypto/provider.c
index 12336acc57..f5dbc4f94a 100644
--- a/crypto/provider.c
+++ b/crypto/provider.c
@@ -26,12 +26,12 @@ OSSL_PROVIDER *OSSL_PROVIDER_try_load(OSSL_LIB_CTX *libctx, const char *name,
         isnew = 1;
     }
 
-    if (!ossl_provider_activate(prov, retain_fallbacks, 1)) {
+    if (!ossl_provider_activate(prov, 1)) {
         ossl_provider_free(prov);
         return NULL;
     }
 
-    if (isnew && !ossl_provider_add_to_store(prov)) {
+    if (isnew && !ossl_provider_add_to_store(prov, retain_fallbacks)) {
         ossl_provider_deactivate(prov);
         ossl_provider_free(prov);
         return NULL;
diff --git a/crypto/provider_child.c b/crypto/provider_child.c
index cabf3ba19d..b077e95ffc 100644
--- a/crypto/provider_child.c
+++ b/crypto/provider_child.c
@@ -148,11 +148,11 @@ static int provider_create_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
                                        1)) == NULL)
             goto err;
 
-        if (!ossl_provider_activate(cprov, 0, 0))
+        if (!ossl_provider_activate(cprov, 0))
             goto err;
 
         if (!ossl_provider_set_child(cprov, prov)
-            || !ossl_provider_add_to_store(cprov)) {
+            || !ossl_provider_add_to_store(cprov, 0)) {
             ossl_provider_deactivate(cprov);
             ossl_provider_free(cprov);
             goto err;
diff --git a/crypto/provider_conf.c b/crypto/provider_conf.c
index 8e83264dc6..1e59e959e3 100644
--- a/crypto/provider_conf.c
+++ b/crypto/provider_conf.c
@@ -171,9 +171,9 @@ static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name,
         ok = provider_conf_params(prov, NULL, NULL, value, cnf);
 
         if (ok) {
-            if (!ossl_provider_activate(prov, 0, 1)) {
+            if (!ossl_provider_activate(prov, 1)) {
                 ok = 0;
-            } else if (!ossl_provider_add_to_store(prov)) {
+            } else if (!ossl_provider_add_to_store(prov, 0)) {
                 ossl_provider_deactivate(prov);
                 ok = 0;
             } else {
diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index 62fab92028..83c6bf28f0 100644
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -431,7 +431,7 @@ int ossl_provider_up_ref(OSSL_PROVIDER *prov)
 static int provider_up_ref_intern(OSSL_PROVIDER *prov, int activate)
 {
     if (activate)
-        return ossl_provider_activate(prov, 0, 1);
+        return ossl_provider_activate(prov, 1);
 
     return ossl_provider_up_ref(prov);
 }
@@ -512,7 +512,7 @@ OSSL_PROVIDER *ossl_provider_new(OSSL_LIB_CTX *libctx, const char *name,
     return prov;
 }
 
-int ossl_provider_add_to_store(OSSL_PROVIDER *prov)
+int ossl_provider_add_to_store(OSSL_PROVIDER *prov, int retain_fallbacks)
 {
     struct provider_store_st *store = NULL;
     int ret = 1;
@@ -530,6 +530,8 @@ int ossl_provider_add_to_store(OSSL_PROVIDER *prov)
         ossl_provider_free(prov);
         ret = 0;
     }
+    if (!retain_fallbacks)
+        store->use_fallbacks = 0;
     CRYPTO_THREAD_unlock(store->lock);
 
     return ret;
@@ -1025,24 +1027,15 @@ static int provider_flush_store_cache(const OSSL_PROVIDER *prov)
     return 1;
 }
 
-int ossl_provider_activate(OSSL_PROVIDER *prov, int retain_fallbacks,
-                           int upcalls)
+int ossl_provider_activate(OSSL_PROVIDER *prov, int upcalls)
 {
     int count;
 
     if (prov == NULL)
         return 0;
-    if ((count = provider_activate(prov, 1, upcalls)) > 0) {
-        if (!retain_fallbacks) {
-            if (!CRYPTO_THREAD_write_lock(prov->store->lock)) {
-                provider_deactivate(prov);
-                return 0;
-            }
-            prov->store->use_fallbacks = 0;
-            CRYPTO_THREAD_unlock(prov->store->lock);
-        }
+    if ((count = provider_activate(prov, 1, upcalls)) > 0)
         return count == 1 ? provider_flush_store_cache(prov) : 1;
-    }
+
     return 0;
 }
 
@@ -1485,10 +1478,8 @@ int ossl_provider_activate_child(OSSL_PROVIDER *prov,
      * The provider could be in one of two states: (1) Already a child,
      * (2) Not a child (not eligible to be one).
      */
-    if (prov->ischild && provider_activate(prov, 0, 0)) {
+    if (prov->ischild && provider_activate(prov, 0, 0))
         flush = 1;
-        prov->store->use_fallbacks = 0;
-    }
 
     CRYPTO_THREAD_unlock(prov->flag_lock);
     CRYPTO_THREAD_unlock(prov->store->lock);
diff --git a/doc/internal/man3/ossl_provider_new.pod b/doc/internal/man3/ossl_provider_new.pod
index d563d41e72..0a76135281 100644
--- a/doc/internal/man3/ossl_provider_new.pod
+++ b/doc/internal/man3/ossl_provider_new.pod
@@ -53,8 +53,7 @@ ossl_provider_get_capabilities
   * Activate the Provider
   * If the Provider is a module, the module will be loaded
   */
- int ossl_provider_activate(OSSL_PROVIDER *prov, int retain_fallbacks,
-                            int upcalls);
+ int ossl_provider_activate(OSSL_PROVIDER *prov, int upcalls);
  int ossl_provider_deactivate(OSSL_PROVIDER *prov);
 
  /* Return pointer to the provider's context */
@@ -218,10 +217,8 @@ be located in that module, and called.
 
 =back
 
-If I<retain_fallbacks> is zero, fallbacks are disabled.  If it is nonzero,
-fallbacks are left unchanged. If I<upcalls> is nonzero then, if this is a child
-provider, upcalls to the parent libctx will be made to inform it of an
-up-ref.
+If I<upcalls> is nonzero then, if this is a child provider, upcalls to the
+parent libctx will be made to inform it of an up-ref.
 
 ossl_provider_deactivate() "deactivates" the provider for the given
 provider object I<prov> by decrementing its activation count.  When
diff --git a/include/internal/provider.h b/include/internal/provider.h
index 45ad1a5b00..b6e413f7a4 100644
--- a/include/internal/provider.h
+++ b/include/internal/provider.h
@@ -59,10 +59,9 @@ int ossl_provider_disable_fallback_loading(OSSL_LIB_CTX *libctx);
  * Activate the Provider
  * If the Provider is a module, the module will be loaded
  */
-int ossl_provider_activate(OSSL_PROVIDER *prov, int retain_fallbacks,
-                           int upcalls);
+int ossl_provider_activate(OSSL_PROVIDER *prov, int upcalls);
 int ossl_provider_deactivate(OSSL_PROVIDER *prov);
-int ossl_provider_add_to_store(OSSL_PROVIDER *prov);
+int ossl_provider_add_to_store(OSSL_PROVIDER *prov, int retain_fallbacks);
 
 /* Return pointer to the provider's context */
 void *ossl_provider_ctx(const OSSL_PROVIDER *prov);
diff --git a/test/provider_internal_test.c b/test/provider_internal_test.c
index 7a37ef8c24..87906c1bdc 100644
--- a/test/provider_internal_test.c
+++ b/test/provider_internal_test.c
@@ -26,7 +26,7 @@ static int test_provider(OSSL_PROVIDER *prov, const char *expected_greeting)
     int ret = 0;
 
     ret =
-        TEST_true(ossl_provider_activate(prov, 0, 1))
+        TEST_true(ossl_provider_activate(prov, 1))
         && TEST_true(ossl_provider_get_params(prov, greeting_request))
         && TEST_ptr(greeting = greeting_request[0].data)
         && TEST_size_t_gt(greeting_request[0].data_size, 0)