CORE: Separate OSSL_PROVIDER activation from OSSL_PROVIDER reference

This introduces a separate activation counter, and the function
ossl_provider_deactivate() for provider deactivation.

Something to be noted is that if the reference count goes down to
zero, we don't care if the activation count is non-zero (i.e. someone
forgot to call ossl_provider_deactivate()).  Since there are no more
references to the provider, it doesn't matter.
The important thing is that deactivation doesn't remove the provider
as long as there are references to it, for example because there are
live methods associated with that provider, but still makes the
provider unavailable to create new methods from.

Fixes #13503
Fixes #12157

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

5 files changed, 109 insertions, 68 deletions

diff --git a/crypto/provider.c b/crypto/provider.c
index 0441fb2f2a..bd8f75a2c1 100644
--- a/crypto/provider.c
+++ b/crypto/provider.c
@@ -40,6 +40,8 @@ OSSL_PROVIDER *OSSL_PROVIDER_load(OSSL_LIB_CTX *libctx, const char *name)
 
 int OSSL_PROVIDER_unload(OSSL_PROVIDER *prov)
 {
+    if (!ossl_provider_deactivate(prov))
+        return 0;
     ossl_provider_free(prov);
     return 1;
 }
diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index 954befd4a2..f0d6fb20f8 100644
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -44,12 +44,15 @@ struct provider_store_st;        /* Forward declaration */
 struct ossl_provider_st {
     /* Flag bits */
     unsigned int flag_initialized:1;
+    unsigned int flag_activated:1;
     unsigned int flag_fallback:1; /* Can be used as fallback */
     unsigned int flag_activated_as_fallback:1;
 
     /* OpenSSL library side data */
     CRYPTO_REF_COUNT refcnt;
     CRYPTO_RWLOCK *refcnt_lock;  /* For the ref counter */
+    CRYPTO_REF_COUNT activatecnt;
+    CRYPTO_RWLOCK *activatecnt_lock; /* For the activate counter */
     char *name;
     char *path;
     DSO *module;
@@ -110,20 +113,15 @@ struct provider_store_st {
 };
 
 /*
- * provider_deactivate_free() is a wrapper around ossl_provider_free()
- * that also makes sure that activated fallback providers are deactivated.
- * This is simply done by freeing them an extra time, to compensate for the
- * refcount that provider_activate_fallbacks() gives them.
+ * provider_deactivate_free() is a wrapper around ossl_provider_deactivate()
+ * and ossl_provider_free(), called as needed.
  * Since this is only called when the provider store is being emptied, we
  * don't need to care about any lock.
  */
 static void provider_deactivate_free(OSSL_PROVIDER *prov)
 {
-    int extra_free = (prov->flag_initialized
-                      && prov->flag_activated_as_fallback);
-
-    if (extra_free)
-        ossl_provider_free(prov);
+    if (prov->flag_activated)
+        ossl_provider_deactivate(prov);
     ossl_provider_free(prov);
 }
 
@@ -251,6 +249,7 @@ static OSSL_PROVIDER *provider_new(const char *name,
     if ((prov = OPENSSL_zalloc(sizeof(*prov))) == NULL
 #ifndef HAVE_ATOMICS
         || (prov->refcnt_lock = CRYPTO_THREAD_lock_new()) == NULL
+        || (prov->activatecnt_lock = CRYPTO_THREAD_lock_new()) == NULL
 #endif
         || !ossl_provider_up_ref(prov) /* +1 One reference to be returned */
         || (prov->name = OPENSSL_strdup(name)) == NULL) {
@@ -337,38 +336,35 @@ void ossl_provider_free(OSSL_PROVIDER *prov)
         CRYPTO_DOWN_REF(&prov->refcnt, &ref, prov->refcnt_lock);
 
         /*
-         * When the refcount drops below two, the store is the only
-         * possible reference, or it has already been taken away from
-         * the store (this may happen if a provider was activated
-         * because it's a fallback, but isn't currently used)
-         * When that happens, the provider is inactivated.
+         * When the refcount drops to zero, we clean up the provider.
+         * Note that this also does teardown, which may seem late,
+         * considering that init happens on first activation.  However,
+         * there may be other structures hanging on to the provider after
+         * the last deactivation and may therefore need full access to the
+         * provider's services.  Therefore, we deinit late.
          */
-        if (ref < 2 && prov->flag_initialized) {
+        if (ref == 0) {
+            if (prov->flag_initialized) {
 #ifndef FIPS_MODULE
-            ossl_init_thread_deregister(prov);
+                ossl_init_thread_deregister(prov);
 #endif
-            if (prov->teardown != NULL)
-                prov->teardown(prov->provctx);
+                if (prov->teardown != NULL)
+                    prov->teardown(prov->provctx);
 #ifndef OPENSSL_NO_ERR
 # ifndef FIPS_MODULE
-            if (prov->error_strings != NULL) {
-                ERR_unload_strings(prov->error_lib, prov->error_strings);
-                OPENSSL_free(prov->error_strings);
-                prov->error_strings = NULL;
-            }
+                if (prov->error_strings != NULL) {
+                    ERR_unload_strings(prov->error_lib, prov->error_strings);
+                    OPENSSL_free(prov->error_strings);
+                    prov->error_strings = NULL;
+                }
 # endif
 #endif
-            OPENSSL_free(prov->operation_bits);
-            prov->operation_bits = NULL;
-            prov->operation_bits_sz = 0;
-            prov->flag_initialized = 0;
-        }
+                OPENSSL_free(prov->operation_bits);
+                prov->operation_bits = NULL;
+                prov->operation_bits_sz = 0;
+                prov->flag_initialized = 0;
+            }
 
-        /*
-         * When the refcount drops to zero, it has been taken out of
-         * the store.  All we have to do here is clean it out.
-         */
-        if (ref == 0) {
 #ifndef FIPS_MODULE
             DSO_free(prov->module);
 #endif
@@ -377,6 +373,7 @@ void ossl_provider_free(OSSL_PROVIDER *prov)
             sk_INFOPAIR_pop_free(prov->parameters, free_infopair);
 #ifndef HAVE_ATOMICS
             CRYPTO_THREAD_lock_free(prov->refcnt_lock);
+            CRYPTO_THREAD_lock_free(prov->activatecnt_lock);
 #endif
             OPENSSL_free(prov);
         }
@@ -460,7 +457,7 @@ int OSSL_PROVIDER_set_default_search_path(OSSL_LIB_CTX *libctx,
  * locking.  Direct callers must remember to set the store flags when
  * appropriate.
  */
-static int provider_activate(OSSL_PROVIDER *prov)
+static int provider_init(OSSL_PROVIDER *prov)
 {
     const OSSL_DISPATCH *provider_dispatch = NULL;
     void *tmp_provctx = NULL;    /* safety measure */
@@ -633,18 +630,60 @@ static int provider_activate(OSSL_PROVIDER *prov)
     return 1;
 }
 
+static int provider_deactivate(OSSL_PROVIDER *prov)
+{
+    int ref = 0;
+
+    if (!ossl_assert(prov != NULL))
+        return 0;
+
+    if (CRYPTO_DOWN_REF(&prov->activatecnt, &ref, prov->activatecnt_lock) <= 0)
+        return 0;
+
+    if (ref < 1)
+        prov->flag_activated = 0;
+
+    /* We don't deinit here, that's done in ossl_provider_free() */
+    return 1;
+}
+
+static int provider_activate(OSSL_PROVIDER *prov)
+{
+    int ref = 0;
+
+    if (CRYPTO_UP_REF(&prov->activatecnt, &ref, prov->activatecnt_lock) <= 0)
+        return 0;
+
+    if (provider_init(prov)) {
+        prov->flag_activated = 1;
+
+        return 1;
+    }
+
+    provider_deactivate(prov);
+    return 0;
+}
+
 int ossl_provider_activate(OSSL_PROVIDER *prov)
 {
+    if (prov == NULL)
+        return 0;
     if (provider_activate(prov)) {
         CRYPTO_THREAD_write_lock(prov->store->lock);
         prov->store->use_fallbacks = 0;
         CRYPTO_THREAD_unlock(prov->store->lock);
         return 1;
     }
-
     return 0;
 }
 
+int ossl_provider_deactivate(OSSL_PROVIDER *prov)
+{
+    if (prov == NULL)
+        return 0;
+    return provider_deactivate(prov);
+}
+
 void *ossl_provider_ctx(const OSSL_PROVIDER *prov)
 {
     return prov->provctx;
@@ -669,7 +708,7 @@ static int provider_forall_loaded(struct provider_store_st *store,
         OSSL_PROVIDER *prov =
             sk_OSSL_PROVIDER_value(store->providers, i);
 
-        if (prov->flag_initialized) {
+        if (prov->flag_activated) {
             if (found_activated != NULL)
                 *found_activated = 1;
             if (!(ret = cb(prov, cbdata)))
@@ -695,23 +734,14 @@ static void provider_activate_fallbacks(struct provider_store_st *store)
         for (i = 0; i < num_provs; i++) {
             OSSL_PROVIDER *prov = sk_OSSL_PROVIDER_value(store->providers, i);
 
-            /*
-             * Activated fallback providers get an extra refcount, to
-             * simulate a regular load.
-             * Note that we don't care if the activation succeeds or not,
-             * other than to maintain a correct refcount.  If the activation
-             * doesn't succeed, then any future attempt to use the fallback
-             * provider will fail anyway.
-             */
-            if (prov->flag_fallback) {
-                if (ossl_provider_up_ref(prov)) {
-                    if (!provider_activate(prov)) {
-                        ossl_provider_free(prov);
-                    } else {
+            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);
             }
         }
 
@@ -765,7 +795,7 @@ int ossl_provider_available(OSSL_PROVIDER *prov)
         provider_activate_fallbacks(prov->store);
         CRYPTO_THREAD_unlock(prov->store->lock);
 
-        return prov->flag_initialized;
+        return prov->flag_activated;
     }
     return 0;
 }
diff --git a/doc/internal/man3/ossl_provider_new.pod b/doc/internal/man3/ossl_provider_new.pod
index dc7717062c..d01673e767 100644
--- a/doc/internal/man3/ossl_provider_new.pod
+++ b/doc/internal/man3/ossl_provider_new.pod
@@ -6,7 +6,7 @@ ossl_provider_find, ossl_provider_new, ossl_provider_up_ref,
 ossl_provider_free,
 ossl_provider_set_fallback, ossl_provider_set_module_path,
 ossl_provider_add_parameter,
-ossl_provider_activate, ossl_provider_available,
+ossl_provider_activate, ossl_provider_deactivate, ossl_provider_available,
 ossl_provider_ctx,
 ossl_provider_forall_loaded,
 ossl_provider_name, ossl_provider_dso,
@@ -36,9 +36,13 @@ ossl_provider_get_capabilities
  int ossl_provider_add_parameter(OSSL_PROVIDER *prov, const char *name,
                                  const char *value);
 
- /* Load and initialize the Provider */
+ /*
+  * Activate the Provider
+  * If the Provider is a module, the module will be loaded
+  */
  int ossl_provider_activate(OSSL_PROVIDER *prov);
- /* Check if provider is available */
+ int ossl_provider_deactivate(OSSL_PROVIDER *prov);
+ /* Check if provider is available (activated) */
  int ossl_provider_available(OSSL_PROVIDER *prov);
 
  /* Return pointer to the provider's context */
@@ -89,8 +93,8 @@ Provider objects are reference counted.
 Provider objects are initially inactive, i.e. they are only recorded
 in the store, but are not used.
 They are activated with the first call to ossl_provider_activate(),
-and are inactivated when ossl_provider_free() has been called as many
-times as ossl_provider_activate() has.
+and are deactivated with the last call to ossl_provider_deactivate().
+Activation affects a separate counter.
 
 =head2 Functions
 
@@ -127,11 +131,10 @@ ossl_provider_up_ref() increments the provider object I<prov>'s
 reference count.
 
 ossl_provider_free() decrements the provider object I<prov>'s
-reference count; if it drops below 2, the provider object is assumed
-to have fallen out of use and will be deactivated (its I<teardown>
-function is called); if it drops down to zero, I<prov> is assumed to
-have been taken out of the store, and the associated module will be
-unloaded if one was loaded, and I<prov> itself will be freed.
+reference count; when it drops to zero, the provider object is assumed
+to have fallen out of use and will be deinitialized (its I<teardown>
+function is called), and the associated module will be unloaded if one
+was loaded, and I<prov> itself will be freed.
 
 ossl_provider_set_fallback() marks an available provider I<prov> as
 fallback.
@@ -155,9 +158,9 @@ Only text parameters can be given, and it's up to the provider to
 interpret them.
 
 ossl_provider_activate() "activates" the provider for the given
-provider object I<prov>.
-What "activates" means depends on what type of provider object it
-is:
+provider object I<prov> by incrementing its activation count, flagging
+it as activated, and initializing it if it isn't already initialized.
+Initializing means one of the following:
 
 =over 4
 
@@ -175,6 +178,10 @@ be located in that module, and called.
 
 =back
 
+ossl_provider_deactivate() "deactivates" the provider for the given
+provider object I<prov> by decrementing its activation count.  When
+that count reaches zero, the activation flag is cleared.
+
 ossl_provider_available() activates all fallbacks if no provider is
 activated yet, then checks if given provider object I<prov> is
 activated.
@@ -269,8 +276,9 @@ it has been incremented.
 
 ossl_provider_free() doesn't return any value.
 
-ossl_provider_set_module_path(), ossl_provider_set_fallback() and
-ossl_provider_activate() return 1 on success, or 0 on error.
+ossl_provider_set_module_path(), ossl_provider_set_fallback(),
+ossl_provider_activate() and ossl_provider_deactivate() return 1 on
+success, or 0 on error.
 
 ossl_provider_available() return 1 if the provider is available,
 otherwise 0.
diff --git a/include/internal/provider.h b/include/internal/provider.h
index ab36c93b32..7a0fc84875 100644
--- a/include/internal/provider.h
+++ b/include/internal/provider.h
@@ -47,10 +47,10 @@ int ossl_provider_disable_fallback_loading(OSSL_LIB_CTX *libctx);
 /*
  * Activate the Provider
  * If the Provider is a module, the module will be loaded
- * Inactivation is done by freeing the Provider
  */
 int ossl_provider_activate(OSSL_PROVIDER *prov);
-/* Check if the provider is available */
+int ossl_provider_deactivate(OSSL_PROVIDER *prov);
+/* Check if the provider is available (activated) */
 int ossl_provider_available(OSSL_PROVIDER *prov);
 
 /* Return pointer to the provider's context */
diff --git a/test/provider_internal_test.c b/test/provider_internal_test.c
index 478f28182d..4b2b6d5349 100644
--- a/test/provider_internal_test.c
+++ b/test/provider_internal_test.c
@@ -30,7 +30,8 @@ static int test_provider(OSSL_PROVIDER *prov, const char *expected_greeting)
         && 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)
-        && TEST_str_eq(greeting, expected_greeting);
+        && TEST_str_eq(greeting, expected_greeting)
+        && TEST_true(ossl_provider_deactivate(prov));
 
     TEST_info("Got this greeting: %s\n", greeting);
     ossl_provider_free(prov);