"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, 30 insertions, 9 deletions

diff --git a/crypto/property/property.c b/crypto/property/property.c
index 496940c378..672cc35607 100644
--- a/crypto/property/property.c
+++ b/crypto/property/property.c
@@ -63,7 +63,19 @@ typedef struct {
 struct ossl_method_store_st {
     OSSL_LIB_CTX *ctx;
     SPARSE_ARRAY_OF(ALGORITHM) *algs;
+    /*
+     * Lock to protect the |algs| array from concurrent writing, when
+     * individual implementations or queries are inserted.  This is used
+     * by the appropriate functions here.
+     */
     CRYPTO_RWLOCK *lock;
+    /*
+     * Lock to reserve the whole store.  This is used when fetching a set
+     * of algorithms, via these functions, found in crypto/core_fetch.c:
+     * ossl_method_construct_reserve_store()
+     * ossl_method_construct_unreserve_store()
+     */
+    CRYPTO_RWLOCK *biglock;
 
     /* query cache specific values */
 
@@ -230,13 +242,10 @@ OSSL_METHOD_STORE *ossl_method_store_new(OSSL_LIB_CTX *ctx)
     res = OPENSSL_zalloc(sizeof(*res));
     if (res != NULL) {
         res->ctx = ctx;
-        if ((res->algs = ossl_sa_ALGORITHM_new()) == NULL) {
-            OPENSSL_free(res);
-            return NULL;
-        }
-        if ((res->lock = CRYPTO_THREAD_lock_new()) == NULL) {
-            ossl_sa_ALGORITHM_free(res->algs);
-            OPENSSL_free(res);
+        if ((res->algs = ossl_sa_ALGORITHM_new()) == NULL
+            || (res->lock = CRYPTO_THREAD_lock_new()) == NULL
+            || (res->biglock = CRYPTO_THREAD_lock_new()) == NULL) {
+            ossl_method_store_free(res);
             return NULL;
         }
     }
@@ -246,13 +255,25 @@ OSSL_METHOD_STORE *ossl_method_store_new(OSSL_LIB_CTX *ctx)
 void ossl_method_store_free(OSSL_METHOD_STORE *store)
 {
     if (store != NULL) {
-        ossl_sa_ALGORITHM_doall_arg(store->algs, &alg_cleanup, store);
+        if (store->algs != NULL)
+            ossl_sa_ALGORITHM_doall_arg(store->algs, &alg_cleanup, store);
         ossl_sa_ALGORITHM_free(store->algs);
         CRYPTO_THREAD_lock_free(store->lock);
+        CRYPTO_THREAD_lock_free(store->biglock);
         OPENSSL_free(store);
     }
 }
 
+int ossl_method_lock_store(OSSL_METHOD_STORE *store)
+{
+    return store != NULL ? CRYPTO_THREAD_write_lock(store->biglock) : 0;
+}
+
+int ossl_method_unlock_store(OSSL_METHOD_STORE *store)
+{
+    return store != NULL ? CRYPTO_THREAD_unlock(store->biglock) : 0;
+}
+
 static ALGORITHM *ossl_method_store_retrieve(OSSL_METHOD_STORE *store, int nid)
 {
     return ossl_sa_ALGORITHM_get(store->algs, nid);
@@ -260,7 +281,7 @@ static ALGORITHM *ossl_method_store_retrieve(OSSL_METHOD_STORE *store, int nid)
 
 static int ossl_method_store_insert(OSSL_METHOD_STORE *store, ALGORITHM *alg)
 {
-        return ossl_sa_ALGORITHM_set(store->algs, alg->nid, alg);
+    return ossl_sa_ALGORITHM_set(store->algs, alg->nid, alg);
 }
 
 int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,