diff options
author | Matt Caswell <matt@openssl.org> | 2021-01-11 17:02:01 +0000 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2021-01-14 17:30:46 +0000 |
commit | 7dd2cb569358591bb832af66fdabd6a6c580c1d4 (patch) | |
tree | 47703a558c054c0997bd6d1059e0b96a55d9784a /crypto/provider_core.c | |
parent | b457c8f514130d3b92de574620d38c1058eb7b35 (diff) |
Fix an issue in provider_activate_fallbacks()
The above function was running while holding the store lock with a read
lock. Unfortunately it actually modifies the store, so a write lock is
required instead.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13660)
Diffstat (limited to 'crypto/provider_core.c')
-rw-r--r-- | crypto/provider_core.c | 65 |
1 files changed, 39 insertions, 26 deletions
diff --git a/crypto/provider_core.c b/crypto/provider_core.c index 79c548e52d..91cfcaa9ec 100644 --- a/crypto/provider_core.c +++ b/crypto/provider_core.c @@ -729,34 +729,50 @@ static int provider_forall_loaded(struct provider_store_st *store, */ static void provider_activate_fallbacks(struct provider_store_st *store) { - if (store->use_fallbacks) { - int num_provs = sk_OSSL_PROVIDER_num(store->providers); - int activated_fallback_count = 0; - int i; + int use_fallbacks; + int num_provs; + int activated_fallback_count = 0; + int i; + + CRYPTO_THREAD_read_lock(store->lock); + use_fallbacks = store->use_fallbacks; + CRYPTO_THREAD_unlock(store->lock); + if (!use_fallbacks) + return; + + CRYPTO_THREAD_write_lock(store->lock); + /* Check again, just in case another thread changed it */ + use_fallbacks = store->use_fallbacks; + if (!use_fallbacks) { + CRYPTO_THREAD_unlock(store->lock); + return; + } - for (i = 0; i < num_provs; i++) { - OSSL_PROVIDER *prov = sk_OSSL_PROVIDER_value(store->providers, i); + num_provs = sk_OSSL_PROVIDER_num(store->providers); + for (i = 0; i < num_provs; i++) { + OSSL_PROVIDER *prov = sk_OSSL_PROVIDER_value(store->providers, i); - if (ossl_provider_up_ref(prov)) { - if (prov->flag_fallback) { - if (provider_activate(prov)) { - prov->flag_activated_as_fallback = 1; - activated_fallback_count++; - } + if (ossl_provider_up_ref(prov)) { + if (prov->flag_fallback) { + if (provider_activate(prov)) { + prov->flag_activated_as_fallback = 1; + activated_fallback_count++; } - ossl_provider_free(prov); } + ossl_provider_free(prov); } - - /* - * We assume that all fallbacks have been added to the store before - * any fallback is activated. - * TODO: We may have to reconsider this, IF we find ourselves adding - * fallbacks after any previous fallback has been activated. - */ - if (activated_fallback_count > 0) - store->use_fallbacks = 0; } + + /* + * We assume that all fallbacks have been added to the store before + * any fallback is activated. + * TODO: We may have to reconsider this, IF we find ourselves adding + * fallbacks after any previous fallback has been activated. + */ + if (activated_fallback_count > 0) + store->use_fallbacks = 0; + + CRYPTO_THREAD_unlock(store->lock); } int ossl_provider_forall_loaded(OSSL_LIB_CTX *ctx, @@ -776,10 +792,9 @@ int ossl_provider_forall_loaded(OSSL_LIB_CTX *ctx, #endif if (store != NULL) { - CRYPTO_THREAD_read_lock(store->lock); - provider_activate_fallbacks(store); + CRYPTO_THREAD_read_lock(store->lock); /* * Now, we sweep through all providers */ @@ -794,9 +809,7 @@ int ossl_provider_forall_loaded(OSSL_LIB_CTX *ctx, int ossl_provider_available(OSSL_PROVIDER *prov) { if (prov != NULL) { - CRYPTO_THREAD_read_lock(prov->store->lock); provider_activate_fallbacks(prov->store); - CRYPTO_THREAD_unlock(prov->store->lock); return prov->flag_activated; } |