From addbd7c9d784e1cb630d43487b0572e867bfc86d Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 9 Nov 2021 16:23:34 +0000 Subject: [PATCH] Hold the flag_lock when calling child callbacks Not holding the flag lock when creating/removing child providers can confuse the activation counts if the parent provider is loaded/unloaded at the same time. Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/16980) --- crypto/provider_core.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/crypto/provider_core.c b/crypto/provider_core.c index a46a96cc93e..1d5787a6482 100644 --- a/crypto/provider_core.c +++ b/crypto/provider_core.c @@ -107,8 +107,8 @@ * some other function while holding a lock make sure you know whether it * will make any upcalls or not. For example ossl_provider_up_ref() can call * ossl_provider_up_ref_parent() which can call the c_prov_up_ref() upcall. - * - It is permissible to hold the store lock when calling child provider - * callbacks. No other locks may be held during such callbacks. + * - It is permissible to hold the store and flag locks when calling child + * provider callbacks. No other locks may be held during such callbacks. */ static OSSL_PROVIDER *provider_new(const char *name, @@ -1058,9 +1058,6 @@ static int provider_deactivate(OSSL_PROVIDER *prov, int upcalls, removechildren = 0; #endif - if (lock) - CRYPTO_THREAD_unlock(prov->flag_lock); - #ifndef FIPS_MODULE if (removechildren && store != NULL) { int i, max = sk_OSSL_PROVIDER_CHILD_CB_num(store->child_cbs); @@ -1072,8 +1069,10 @@ static int provider_deactivate(OSSL_PROVIDER *prov, int upcalls, } } #endif - if (lock) + if (lock) { + CRYPTO_THREAD_unlock(prov->flag_lock); CRYPTO_THREAD_unlock(store->lock); + } #ifndef FIPS_MODULE if (freeparent) ossl_provider_free_parent(prov, 1); @@ -1091,7 +1090,7 @@ static int provider_activate(OSSL_PROVIDER *prov, int lock, int upcalls) { int count = -1; struct provider_store_st *store; - int ret = 1, createchildren = 0; + int ret = 1; store = prov->store; /* @@ -1129,15 +1128,13 @@ static int provider_activate(OSSL_PROVIDER *prov, int lock, int upcalls) count = ++prov->activatecnt; prov->flag_activated = 1; - if (prov->activatecnt == 1 && store != NULL) - createchildren = 1; - - if (lock) - CRYPTO_THREAD_unlock(prov->flag_lock); - if (createchildren) + if (prov->activatecnt == 1 && store != NULL) { ret = create_provider_children(prov); - if (lock) + } + if (lock) { + CRYPTO_THREAD_unlock(prov->flag_lock); CRYPTO_THREAD_unlock(store->lock); + } if (!ret) return -1; -- 2.47.2