Don't empty the method store when flushing the query cache

When evp_method_store_flush() flushed the query cache, it also freed
all methods in the EVP method store, through an unfortunate call of
ossl_method_store_flush_cache() with an argument saying that all
methods should indeed be dropped.

To undo some of the confusion, ossl_method_store_flush_cache() is
renamed to ossl_method_store_cache_flush_all(), and limited to do
only that.  Some if the items in the internal ALGORITHM structure are
also renamed and commented to clarify what they are for.

Fixes #18150

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18151)

3 files changed, 26 insertions, 31 deletions

diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c
index 25ecd1b912..5fe9c7e930 100644
--- a/crypto/evp/evp_fetch.c
+++ b/crypto/evp/evp_fetch.c
@@ -409,12 +409,12 @@ void *evp_generic_fetch_from_prov(OSSL_PROVIDER *prov, int operation_id,
     return method;
 }
 
-int evp_method_store_flush(OSSL_LIB_CTX *libctx)
+int evp_method_store_cache_flush(OSSL_LIB_CTX *libctx)
 {
     OSSL_METHOD_STORE *store = get_evp_method_store(libctx);
 
     if (store != NULL)
-        return ossl_method_store_flush_cache(store, 1);
+        return ossl_method_store_cache_flush_all(store);
     return 1;
 }
 
@@ -461,7 +461,7 @@ static int evp_set_parsed_default_properties(OSSL_LIB_CTX *libctx,
         ossl_property_free(*plp);
         *plp = def_prop;
         if (store != NULL)
-            return ossl_method_store_flush_cache(store, 0);
+            return ossl_method_store_cache_flush_all(store);
     }
     ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR);
     return 0;
diff --git a/crypto/property/property.c b/crypto/property/property.c
index d0415a6c55..b2c71db481 100644
--- a/crypto/property/property.c
+++ b/crypto/property/property.c
@@ -62,10 +62,16 @@ typedef struct {
 
 struct ossl_method_store_st {
     OSSL_LIB_CTX *ctx;
-    size_t nelem;
     SPARSE_ARRAY_OF(ALGORITHM) *algs;
-    int need_flush;
     CRYPTO_RWLOCK *lock;
+
+    /* query cache specific values */
+
+    /* Count of the query cache entries for all algs */
+    size_t cache_nelem;
+
+    /* Flag: 1 if query cache entries for all algs need flushing */
+    int cache_need_flush;
 };
 
 typedef struct {
@@ -478,19 +484,10 @@ fin:
     return ret;
 }
 
-static void impl_cache_flush_alg(ossl_uintmax_t idx, ALGORITHM *alg, void *arg)
+static void impl_cache_flush_alg(ossl_uintmax_t idx, ALGORITHM *alg)
 {
-    SPARSE_ARRAY_OF(ALGORITHM) *algs = arg;
-
     lh_QUERY_doall(alg->cache, &impl_cache_free);
-    if (algs != NULL) {
-        sk_IMPLEMENTATION_pop_free(alg->impls, &impl_free);
-        lh_QUERY_free(alg->cache);
-        OPENSSL_free(alg);
-        ossl_sa_ALGORITHM_set(algs, idx, NULL);
-    } else {
-        lh_QUERY_flush(alg->cache);
-    }
+    lh_QUERY_flush(alg->cache);
 }
 
 static void ossl_method_cache_flush(OSSL_METHOD_STORE *store, int nid)
@@ -498,19 +495,17 @@ static void ossl_method_cache_flush(OSSL_METHOD_STORE *store, int nid)
     ALGORITHM *alg = ossl_method_store_retrieve(store, nid);
 
     if (alg != NULL) {
-        store->nelem -= lh_QUERY_num_items(alg->cache);
-        impl_cache_flush_alg(0, alg, NULL);
+        store->cache_nelem -= lh_QUERY_num_items(alg->cache);
+        impl_cache_flush_alg(0, alg);
     }
 }
 
-int ossl_method_store_flush_cache(OSSL_METHOD_STORE *store, int all)
+int ossl_method_store_cache_flush_all(OSSL_METHOD_STORE *store)
 {
-    void *arg = (all != 0 ? store->algs : NULL);
-
     if (!ossl_property_write_lock(store))
         return 0;
-    ossl_sa_ALGORITHM_doall_arg(store->algs, &impl_cache_flush_alg, arg);
-    store->nelem = 0;
+    ossl_sa_ALGORITHM_doall(store->algs, &impl_cache_flush_alg);
+    store->cache_nelem = 0;
     ossl_property_unlock(store);
     return 1;
 }
@@ -573,9 +568,9 @@ static void ossl_method_cache_flush_some(OSSL_METHOD_STORE *store)
     state.nelem = 0;
     if ((state.seed = OPENSSL_rdtsc()) == 0)
         state.seed = 1;
-    store->need_flush = 0;
+    store->cache_need_flush = 0;
     ossl_sa_ALGORITHM_doall_arg(store->algs, &impl_cache_flush_one_alg, &state);
-    store->nelem = state.nelem;
+    store->cache_nelem = state.nelem;
 }
 
 int ossl_method_store_cache_get(OSSL_METHOD_STORE *store, OSSL_PROVIDER *prov,
@@ -626,7 +621,7 @@ int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, OSSL_PROVIDER *prov,
 
     if (!ossl_property_write_lock(store))
         return 0;
-    if (store->need_flush)
+    if (store->cache_need_flush)
         ossl_method_cache_flush_some(store);
     alg = ossl_method_store_retrieve(store, nid);
     if (alg == NULL)
@@ -637,7 +632,7 @@ int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, OSSL_PROVIDER *prov,
         elem.provider = prov;
         if ((old = lh_QUERY_delete(alg->cache, &elem)) != NULL) {
             impl_cache_free(old);
-            store->nelem--;
+            store->cache_nelem--;
         }
         goto end;
     }
@@ -656,8 +651,8 @@ int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, OSSL_PROVIDER *prov,
             goto end;
         }
         if (!lh_QUERY_error(alg->cache)) {
-            if (++store->nelem >= IMPL_CACHE_FLUSH_THRESHOLD)
-                store->need_flush = 1;
+            if (++store->cache_nelem >= IMPL_CACHE_FLUSH_THRESHOLD)
+                store->cache_need_flush = 1;
             goto end;
         }
         ossl_method_free(&p->method);
diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index c24cb65f51..ef0c156a34 100644
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -15,7 +15,7 @@
 #include <openssl/params.h>
 #include <openssl/opensslv.h>
 #include "crypto/cryptlib.h"
-#include "crypto/evp.h" /* evp_method_store_flush */
+#include "crypto/evp.h" /* evp_method_store_cache_flush */
 #include "crypto/rand.h"
 #include "internal/nelem.h"
 #include "internal/thread_once.h"
@@ -1152,7 +1152,7 @@ static int provider_flush_store_cache(const OSSL_PROVIDER *prov)
     CRYPTO_THREAD_unlock(store->lock);
 
     if (!freeing)
-        return evp_method_store_flush(prov->libctx);
+        return evp_method_store_cache_flush(prov->libctx);
     return 1;
 }