mirror of
https://github.com/openssl/openssl.git
synced 2025-01-18 13:44:20 +08:00
Don't attempt to deactive child providers if we don't need to
If a provider doesn't have any child providers then there is no need to attempt to remove them - so we should not do so. This removes some potentialy thread races. Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/16980)
This commit is contained in:
parent
6de9214a50
commit
c59fc87b33
@ -35,7 +35,7 @@ OSSL_PROVIDER *OSSL_PROVIDER_try_load(OSSL_LIB_CTX *libctx, const char *name,
|
||||
|
||||
actual = prov;
|
||||
if (isnew && !ossl_provider_add_to_store(prov, &actual, retain_fallbacks)) {
|
||||
ossl_provider_deactivate(prov);
|
||||
ossl_provider_deactivate(prov, 1);
|
||||
ossl_provider_free(prov);
|
||||
return NULL;
|
||||
}
|
||||
@ -53,7 +53,7 @@ OSSL_PROVIDER *OSSL_PROVIDER_load(OSSL_LIB_CTX *libctx, const char *name)
|
||||
|
||||
int OSSL_PROVIDER_unload(OSSL_PROVIDER *prov)
|
||||
{
|
||||
if (!ossl_provider_deactivate(prov))
|
||||
if (!ossl_provider_deactivate(prov, 1))
|
||||
return 0;
|
||||
ossl_provider_free(prov);
|
||||
return 1;
|
||||
|
@ -153,7 +153,7 @@ static int provider_create_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
|
||||
|
||||
if (!ossl_provider_set_child(cprov, prov)
|
||||
|| !ossl_provider_add_to_store(cprov, NULL, 0)) {
|
||||
ossl_provider_deactivate(cprov);
|
||||
ossl_provider_deactivate(cprov, 0);
|
||||
ossl_provider_free(cprov);
|
||||
goto err;
|
||||
}
|
||||
@ -188,7 +188,7 @@ static int provider_remove_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
|
||||
*/
|
||||
ossl_provider_free(cprov);
|
||||
if (ossl_provider_is_child(cprov)
|
||||
&& !ossl_provider_deactivate(cprov))
|
||||
&& !ossl_provider_deactivate(cprov, 1))
|
||||
return 0;
|
||||
|
||||
return 1;
|
||||
|
@ -222,7 +222,7 @@ static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name,
|
||||
if (!ossl_provider_activate(prov, 1, 0)) {
|
||||
ok = 0;
|
||||
} else if (!ossl_provider_add_to_store(prov, &actual, 0)) {
|
||||
ossl_provider_deactivate(prov);
|
||||
ossl_provider_deactivate(prov, 1);
|
||||
ok = 0;
|
||||
} else {
|
||||
if (pcgbl->activated_providers == NULL)
|
||||
|
@ -229,7 +229,7 @@ struct provider_store_st {
|
||||
static void provider_deactivate_free(OSSL_PROVIDER *prov)
|
||||
{
|
||||
if (prov->flag_activated)
|
||||
ossl_provider_deactivate(prov);
|
||||
ossl_provider_deactivate(prov, 1);
|
||||
ossl_provider_free(prov);
|
||||
}
|
||||
|
||||
@ -498,7 +498,7 @@ static int provider_up_ref_intern(OSSL_PROVIDER *prov, int activate)
|
||||
static int provider_free_intern(OSSL_PROVIDER *prov, int deactivate)
|
||||
{
|
||||
if (deactivate)
|
||||
return ossl_provider_deactivate(prov);
|
||||
return ossl_provider_deactivate(prov, 1);
|
||||
|
||||
ossl_provider_free(prov);
|
||||
return 1;
|
||||
@ -644,8 +644,11 @@ int ossl_provider_add_to_store(OSSL_PROVIDER *prov, OSSL_PROVIDER **actualprov,
|
||||
* name and raced to put them in the store. This thread lost. We
|
||||
* deactivate the one we just created and use the one that already
|
||||
* exists instead.
|
||||
* If we get here then we know we did not create provider children
|
||||
* above, so we inform ossl_provider_deactivate not to attempt to remove
|
||||
* any.
|
||||
*/
|
||||
ossl_provider_deactivate(prov);
|
||||
ossl_provider_deactivate(prov, 0);
|
||||
ossl_provider_free(prov);
|
||||
}
|
||||
|
||||
@ -1002,15 +1005,18 @@ static int provider_init(OSSL_PROVIDER *prov)
|
||||
}
|
||||
|
||||
/*
|
||||
* Deactivate a provider.
|
||||
* Deactivate a provider. If upcalls is 0 then we suppress any upcalls to a
|
||||
* parent provider. If removechildren is 0 then we suppress any calls to remove
|
||||
* child providers.
|
||||
* Return -1 on failure and the activation count on success
|
||||
*/
|
||||
static int provider_deactivate(OSSL_PROVIDER *prov, int upcalls)
|
||||
static int provider_deactivate(OSSL_PROVIDER *prov, int upcalls,
|
||||
int removechildren)
|
||||
{
|
||||
int count;
|
||||
struct provider_store_st *store;
|
||||
#ifndef FIPS_MODULE
|
||||
int freeparent = 0, removechildren = 0;
|
||||
int freeparent = 0;
|
||||
#endif
|
||||
|
||||
if (!ossl_assert(prov != NULL))
|
||||
@ -1039,12 +1045,12 @@ static int provider_deactivate(OSSL_PROVIDER *prov, int upcalls)
|
||||
}
|
||||
#endif
|
||||
|
||||
if ((count = --prov->activatecnt) < 1) {
|
||||
if ((count = --prov->activatecnt) < 1)
|
||||
prov->flag_activated = 0;
|
||||
#ifndef FIPS_MODULE
|
||||
removechildren = 1;
|
||||
else
|
||||
removechildren = 0;
|
||||
#endif
|
||||
}
|
||||
|
||||
CRYPTO_THREAD_unlock(prov->flag_lock);
|
||||
|
||||
@ -1169,11 +1175,12 @@ int ossl_provider_activate(OSSL_PROVIDER *prov, int upcalls, int aschild)
|
||||
return 0;
|
||||
}
|
||||
|
||||
int ossl_provider_deactivate(OSSL_PROVIDER *prov)
|
||||
int ossl_provider_deactivate(OSSL_PROVIDER *prov, int removechildren)
|
||||
{
|
||||
int count;
|
||||
|
||||
if (prov == NULL || (count = provider_deactivate(prov, 1)) < 0)
|
||||
if (prov == NULL
|
||||
|| (count = provider_deactivate(prov, 1, removechildren)) < 0)
|
||||
return 0;
|
||||
return count == 0 ? provider_flush_store_cache(prov) : 1;
|
||||
}
|
||||
@ -1355,7 +1362,7 @@ int ossl_provider_doall_activated(OSSL_LIB_CTX *ctx,
|
||||
for (curr++; curr < max; curr++) {
|
||||
OSSL_PROVIDER *prov = sk_OSSL_PROVIDER_value(provs, curr);
|
||||
|
||||
provider_deactivate(prov, 0);
|
||||
provider_deactivate(prov, 0, 1);
|
||||
/*
|
||||
* As above where we did the up-ref, we don't call ossl_provider_free
|
||||
* to avoid making upcalls. There should always be at least one ref
|
||||
|
@ -53,7 +53,7 @@ ossl_provider_get_capabilities
|
||||
* If the Provider is a module, the module will be loaded
|
||||
*/
|
||||
int ossl_provider_activate(OSSL_PROVIDER *prov, int upcalls, int aschild);
|
||||
int ossl_provider_deactivate(OSSL_PROVIDER *prov);
|
||||
int ossl_provider_deactivate(OSSL_PROVIDER *prov, int removechildren);
|
||||
int ossl_provider_add_to_store(OSSL_PROVIDER *prov, OSSL_PROVIDER **actualprov,
|
||||
int retain_fallbacks);
|
||||
|
||||
@ -220,7 +220,9 @@ no action is taken and ossl_provider_activate() returns success.
|
||||
|
||||
ossl_provider_deactivate() "deactivates" the provider for the given
|
||||
provider object I<prov> by decrementing its activation count. When
|
||||
that count reaches zero, the activation flag is cleared.
|
||||
that count reaches zero, the activation flag is cleared. If the
|
||||
I<removechildren> parameter is 0 then no attempt is made to remove any
|
||||
associated child providers.
|
||||
|
||||
ossl_provider_add_to_store() adds the provider I<prov> to the provider store and
|
||||
makes it available to other threads. This will prevent future automatic loading
|
||||
|
@ -56,7 +56,7 @@ int ossl_provider_disable_fallback_loading(OSSL_LIB_CTX *libctx);
|
||||
* If the Provider is a module, the module will be loaded
|
||||
*/
|
||||
int ossl_provider_activate(OSSL_PROVIDER *prov, int upcalls, int aschild);
|
||||
int ossl_provider_deactivate(OSSL_PROVIDER *prov);
|
||||
int ossl_provider_deactivate(OSSL_PROVIDER *prov, int removechildren);
|
||||
int ossl_provider_add_to_store(OSSL_PROVIDER *prov, OSSL_PROVIDER **actualprov,
|
||||
int retain_fallbacks);
|
||||
|
||||
|
@ -31,7 +31,7 @@ static int test_provider(OSSL_PROVIDER *prov, const char *expected_greeting)
|
||||
&& TEST_ptr(greeting = greeting_request[0].data)
|
||||
&& TEST_size_t_gt(greeting_request[0].data_size, 0)
|
||||
&& TEST_str_eq(greeting, expected_greeting)
|
||||
&& TEST_true(ossl_provider_deactivate(prov));
|
||||
&& TEST_true(ossl_provider_deactivate(prov, 1));
|
||||
|
||||
TEST_info("Got this greeting: %s\n", greeting);
|
||||
ossl_provider_free(prov);
|
||||
|
Loading…
Reference in New Issue
Block a user