"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)

1 files changed, 21 insertions, 5 deletions

diff --git a/crypto/core_algorithm.c b/crypto/core_algorithm.c
index 6d1192f098..b685183ab8 100644
--- a/crypto/core_algorithm.c
+++ b/crypto/core_algorithm.c
@@ -18,8 +18,10 @@ struct algorithm_data_st {
     int operation_id;            /* May be zero for finding them all */
     int (*pre)(OSSL_PROVIDER *, int operation_id, int no_store, void *data,
                int *result);
+    int (*reserve_store)(int no_store, void *data);
     void (*fn)(OSSL_PROVIDER *, const OSSL_ALGORITHM *, int no_store,
                void *data);
+    int (*unreserve_store)(void *data);
     int (*post)(OSSL_PROVIDER *, int operation_id, int no_store, void *data,
                 int *result);
     void *data;
@@ -43,6 +45,10 @@ static int algorithm_do_map(OSSL_PROVIDER *provider, const OSSL_ALGORITHM *map,
     struct algorithm_data_st *data = cbdata;
     int ret = 0;
 
+    if (!data->reserve_store(no_store, data->data))
+        /* Error, bail out! */
+        return -1;
+
     /* Do we fulfill pre-conditions? */
     if (data->pre == NULL) {
         /* If there is no pre-condition function, assume "yes" */
@@ -50,7 +56,8 @@ static int algorithm_do_map(OSSL_PROVIDER *provider, const OSSL_ALGORITHM *map,
     } else if (!data->pre(provider, cur_operation, no_store, data->data,
                           &ret)) {
         /* Error, bail out! */
-        return -1;
+        ret = -1;
+        goto end;
     }
 
     /*
@@ -58,8 +65,10 @@ static int algorithm_do_map(OSSL_PROVIDER *provider, const OSSL_ALGORITHM *map,
      * but do continue with the next.  This simply means that another thread
      * got to it first.
      */
-    if (ret == 0)
-        return 1;
+    if (ret == 0) {
+        ret = 1;
+        goto end;
+    }
 
     if (map != NULL) {
         const OSSL_ALGORITHM *thismap;
@@ -75,9 +84,12 @@ static int algorithm_do_map(OSSL_PROVIDER *provider, const OSSL_ALGORITHM *map,
     } else if (!data->post(provider, cur_operation, no_store, data->data,
                            &ret)) {
         /* Error, bail out! */
-        return -1;
+        ret = -1;
     }
 
+ end:
+    data->unreserve_store(data->data);
+
     return ret;
 }
 
@@ -103,7 +115,7 @@ static int algorithm_do_this(OSSL_PROVIDER *provider, void *cbdata)
          cur_operation++) {
         int no_store = 0;        /* Assume caching is ok */
         const OSSL_ALGORITHM *map = NULL;
-        int ret;
+        int ret = 0;
 
         map = ossl_provider_query_operation(provider, cur_operation,
                                             &no_store);
@@ -126,9 +138,11 @@ void ossl_algorithm_do_all(OSSL_LIB_CTX *libctx, int operation_id,
                            OSSL_PROVIDER *provider,
                            int (*pre)(OSSL_PROVIDER *, int operation_id,
                                       int no_store, void *data, int *result),
+                           int (*reserve_store)(int no_store, void *data),
                            void (*fn)(OSSL_PROVIDER *provider,
                                       const OSSL_ALGORITHM *algo,
                                       int no_store, void *data),
+                           int (*unreserve_store)(void *data),
                            int (*post)(OSSL_PROVIDER *, int operation_id,
                                        int no_store, void *data, int *result),
                            void *data)
@@ -138,7 +152,9 @@ void ossl_algorithm_do_all(OSSL_LIB_CTX *libctx, int operation_id,
     cbdata.libctx = libctx;
     cbdata.operation_id = operation_id;
     cbdata.pre = pre;
+    cbdata.reserve_store = reserve_store;
     cbdata.fn = fn;
+    cbdata.unreserve_store = unreserve_store;
     cbdata.post = post;
     cbdata.data = data;