Fix a race in ossl_provider_add_to_store()

If two threads both attempt to load the same provider at the same time,
they will first both check to see if the provider already exists. If it
doesn't then they will both then create new provider objects and call the
init function. However only one of the threads will be successful in adding
the provider to the store. For the "losing" thread we should still return
"success", but we should deinitialise and free the no longer required
provider object, and return the object that exists in the store.

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

1 files changed, 4 insertions, 10 deletions

diff --git a/crypto/provider_child.c b/crypto/provider_child.c
index 3cad1c564f..272d67a52d 100644
--- a/crypto/provider_child.c
+++ b/crypto/provider_child.c
@@ -127,9 +127,9 @@ static int provider_create_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
 
     if ((cprov = ossl_provider_find(ctx, provname, 1)) != NULL) {
         /*
-        * We free the newly created ref. We rely on the provider sticking around
-        * in the provider store.
-        */
+         * We free the newly created ref. We rely on the provider sticking around
+         * in the provider store.
+         */
         ossl_provider_free(cprov);
 
         /*
@@ -152,17 +152,11 @@ static int provider_create_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
             goto err;
 
         if (!ossl_provider_set_child(cprov, prov)
-            || !ossl_provider_add_to_store(cprov, 0)) {
+            || !ossl_provider_add_to_store(cprov, NULL, 0)) {
             ossl_provider_deactivate(cprov);
             ossl_provider_free(cprov);
             goto err;
         }
-
-        /*
-        * We free the newly created ref. We rely on the provider sticking around
-        * in the provider store.
-        */
-        ossl_provider_free(cprov);
     }
 
     ret = 1;