"Reserve" the method store when constructing methods

Introducing the concept of reserving the store where a number of
provided operation methods are to be stored.

This avoids racing when constructing provided methods, which is
especially pertinent when multiple threads are trying to fetch the
same method, or even any implementation for the same given operation
type.

This introduces a |biglock| in OSSL_METHOD_STORE, which is separate
from the |lock| which is used for more internal and finer grained
locking.

Fixes #18152

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18153)

(cherry picked from commit e1eafe8c87612a94552e9ad5df56c489cb6f0ff2)

1 files changed, 29 insertions, 18 deletions

diff --git a/crypto/core_fetch.c b/crypto/core_fetch.c
index 7a8ef0a5e8..38db36ee1f 100644
--- a/crypto/core_fetch.c
+++ b/crypto/core_fetch.c
@@ -31,6 +31,31 @@ static int is_temporary_method_store(int no_store, void *cbdata)
     return no_store && !data->force_store;
 }
 
+static int ossl_method_construct_reserve_store(int no_store, void *cbdata)
+{
+    struct construct_data_st *data = cbdata;
+
+    if (is_temporary_method_store(no_store, data) && data->store == NULL) {
+        /*
+         * If we have been told not to store the method "permanently", we
+         * ask for a temporary store, and store the method there.
+         * The owner of |data->mcm| is completely responsible for managing
+         * that temporary store.
+         */
+        if ((data->store = data->mcm->get_tmp_store(data->mcm_data)) == NULL)
+            return 0;
+    }
+
+    return data->mcm->lock_store(data->store, data->mcm_data);
+}
+
+static int ossl_method_construct_unreserve_store(void *cbdata)
+{
+    struct construct_data_st *data = cbdata;
+
+    return data->mcm->unlock_store(data->store, data->mcm_data);
+}
+
 static int ossl_method_construct_precondition(OSSL_PROVIDER *provider,
                                               int operation_id, int no_store,
                                               void *cbdata, int *result)
@@ -95,24 +120,8 @@ static void ossl_method_construct_this(OSSL_PROVIDER *provider,
      * It is *expected* that the put function increments the refcnt
      * of the passed method.
      */
-
-    if (!is_temporary_method_store(no_store, data)) {
-        /* If we haven't been told not to store, add to the global store */
-        data->mcm->put(NULL, method, provider, algo->algorithm_names,
-                       algo->property_definition, data->mcm_data);
-    } else {
-        /*
-         * If we have been told not to store the method "permanently", we
-         * ask for a temporary store, and store the method there.
-         * The owner of |data->mcm| is completely responsible for managing
-         * that temporary store.
-         */
-        if ((data->store = data->mcm->get_tmp_store(data->mcm_data)) == NULL)
-            return;
-
-        data->mcm->put(data->store, method, provider, algo->algorithm_names,
-                       algo->property_definition, data->mcm_data);
-    }
+    data->mcm->put(data->store, method, provider, algo->algorithm_names,
+                   algo->property_definition, data->mcm_data);
 
     /* refcnt-- because we're dropping the reference */
     data->mcm->destruct(method, data->mcm_data);
@@ -143,7 +152,9 @@ void *ossl_method_construct(OSSL_LIB_CTX *libctx, int operation_id,
     cbdata.mcm_data = mcm_data;
     ossl_algorithm_do_all(libctx, operation_id, provider,
                           ossl_method_construct_precondition,
+                          ossl_method_construct_reserve_store,
                           ossl_method_construct_this,
+                          ossl_method_construct_unreserve_store,
                           ossl_method_construct_postcondition,
                           &cbdata);