mirror/openssl
2
0
Fork 0
mirror of https://github.com/openssl/openssl.git synced 2025-03-25 20:00:44 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix a race in ossl_provider_add_to_store()

Browse Source 
If two threads both attempt to load the same provider at the same time,
they will first both check to see if the provider already exists. If it
doesn't then they will both then create new provider objects and call the
init function. However only one of the threads will be successful in adding
the provider to the store. For the "losing" thread we should still return
"success", but we should deinitialise and free the no longer required
provider object, and return the object that exists in the store.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15854)
This commit is contained in:
Matt Caswell 2021-06-22 15:39:40 +01:00
parent d382c46525
commit 59a783d05a
6 changed files with 75 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
crypto/provider.c
View File

@ -18,7 +18,7 @@
OSSL_PROVIDER *OSSL_PROVIDER_try_load(OSSL_LIB_CTX *libctx, const char *name,
int retain_fallbacks)
{
OSSL_PROVIDER *prov = NULL;
OSSL_PROVIDER *prov = NULL, *actual;
int isnew = 0;
/* Find it or create it */
@ -33,13 +33,14 @@ OSSL_PROVIDER *OSSL_PROVIDER_try_load(OSSL_LIB_CTX *libctx, const char *name,
return NULL;
}
if (isnew && !ossl_provider_add_to_store(prov, retain_fallbacks)) {
actual = prov;
if (isnew && !ossl_provider_add_to_store(prov, &actual, retain_fallbacks)) {
ossl_provider_deactivate(prov);
ossl_provider_free(prov);
return NULL;
}
return prov;
return actual;
}
OSSL_PROVIDER *OSSL_PROVIDER_load(OSSL_LIB_CTX *libctx, const char *name)

14
crypto/provider_child.c
View File

@ -127,9 +127,9 @@ static int provider_create_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
if ((cprov = ossl_provider_find(ctx, provname, 1)) != NULL) {
/*
* We free the newly created ref. We rely on the provider sticking around
* in the provider store.
*/
* We free the newly created ref. We rely on the provider sticking around
* in the provider store.
*/
ossl_provider_free(cprov);
/*
@ -152,17 +152,11 @@ static int provider_create_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
goto err;
if (!ossl_provider_set_child(cprov, prov)
|| !ossl_provider_add_to_store(cprov, 0)) {
|| !ossl_provider_add_to_store(cprov, NULL, 0)) {
ossl_provider_deactivate(cprov);
ossl_provider_free(cprov);
goto err;
}
/*
* We free the newly created ref. We rely on the provider sticking around
* in the provider store.
*/
ossl_provider_free(cprov);
}
ret = 1;

6
crypto/provider_conf.c
View File

@ -113,7 +113,7 @@ static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name,
int i;
STACK_OF(CONF_VALUE) *ecmds;
int soft = 0;
OSSL_PROVIDER *prov = NULL;
OSSL_PROVIDER *prov = NULL, *actual = NULL;
const char *path = NULL;
long activate = 0;
int ok = 0;
@ -173,13 +173,13 @@ static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name,
if (ok) {
if (!ossl_provider_activate(prov, 1, 0)) {
ok = 0;
} else if (!ossl_provider_add_to_store(prov, 0)) {
} else if (!ossl_provider_add_to_store(prov, &actual, 0)) {
ossl_provider_deactivate(prov);
ok = 0;
} else {
if (pcgbl->activated_providers == NULL)
pcgbl->activated_providers = sk_OSSL_PROVIDER_new_null();
sk_OSSL_PROVIDER_push(pcgbl->activated_providers, prov);
sk_OSSL_PROVIDER_push(pcgbl->activated_providers, actual);
ok = 1;
}
}

70
crypto/provider_core.c
View File

@ -511,33 +511,69 @@ static int create_provider_children(OSSL_PROVIDER *prov)
return ret;
}
int ossl_provider_add_to_store(OSSL_PROVIDER *prov, int retain_fallbacks)
int ossl_provider_add_to_store(OSSL_PROVIDER *prov, OSSL_PROVIDER **actualprov,
int retain_fallbacks)
{
struct provider_store_st *store = NULL;
int ret = 1;
struct provider_store_st *store;
int idx;
OSSL_PROVIDER tmpl = { 0, };
OSSL_PROVIDER *actualtmp = NULL;
if ((store = get_provider_store(prov->libctx)) == NULL)
return 0;
if (!ossl_provider_up_ref(prov)) {
ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
if (!CRYPTO_THREAD_write_lock(store->lock))
return 0;
tmpl.name = (char *)prov->name;
idx = sk_OSSL_PROVIDER_find(store->providers, &tmpl);
if (idx == -1)
actualtmp = prov;
else
actualtmp = sk_OSSL_PROVIDER_value(store->providers, idx);
if (actualprov != NULL) {
if (!ossl_provider_up_ref(actualtmp)) {
ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
actualtmp = NULL;
goto err;
}
*actualprov = actualtmp;
}
if (!CRYPTO_THREAD_write_lock(store->lock)
|| sk_OSSL_PROVIDER_push(store->providers, prov) == 0) {
ossl_provider_free(prov);
ret = 0;
}
prov->store = store;
if (!retain_fallbacks)
store->use_fallbacks = 0;
if (!create_provider_children(prov)) {
ret = 0;
if (idx == -1) {
if (sk_OSSL_PROVIDER_push(store->providers, prov) == 0)
goto err;
prov->store = store;
if (!create_provider_children(prov)) {
sk_OSSL_PROVIDER_delete_ptr(store->providers, prov);
goto err;
}
if (!retain_fallbacks)
store->use_fallbacks = 0;
}
CRYPTO_THREAD_unlock(store->lock);
return ret;
if (actualtmp != prov) {
/*
* The provider is already in the store. Probably two threads
* independently initialised their own provider objects with the same
* 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.
*/
ossl_provider_deactivate(prov);
ossl_provider_free(prov);
}
return 1;
err:
CRYPTO_THREAD_unlock(store->lock);
if (actualprov != NULL)
ossl_provider_free(actualtmp);
return 0;
}
void ossl_provider_free(OSSL_PROVIDER *prov)

11
doc/internal/man3/ossl_provider_new.pod
View File

@ -55,7 +55,8 @@ ossl_provider_get_capabilities
*/
int ossl_provider_activate(OSSL_PROVIDER *prov, int upcalls, int aschild);
int ossl_provider_deactivate(OSSL_PROVIDER *prov);
int ossl_provider_add_to_store(OSSL_PROVIDER *prov, int retain_fallbacks);
int ossl_provider_add_to_store(OSSL_PROVIDER *prov, OSSL_PROVIDER **actualprov,
int retain_fallbacks);
/* Return pointer to the provider's context */
void *ossl_provider_ctx(const OSSL_PROVIDER *prov);
@ -229,7 +230,13 @@ that count reaches zero, the activation flag is cleared.
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
of fallback providers, unless I<retain_fallbacks> is true.
of fallback providers, unless I<retain_fallbacks> is true. If a provider of the
same name already exists in the store then it is not added but this function
still returns success. On success the I<actualprov> value is populated with a
pointer to the provider of the given name that is now in the store. The
reference passed in the I<prov> argument is consumed by this function. A
reference to the provider that should be used is passed back in the
I<actualprov> argument.
ossl_provider_ctx() returns a context created by the provider.
Outside of the provider, it's completely opaque, but it needs to be

3
include/internal/provider.h
View File

@ -58,7 +58,8 @@ int ossl_provider_disable_fallback_loading(OSSL_LIB_CTX *libctx);
*/
int ossl_provider_activate(OSSL_PROVIDER *prov, int upcalls, int aschild);
int ossl_provider_deactivate(OSSL_PROVIDER *prov);
int ossl_provider_add_to_store(OSSL_PROVIDER *prov, int retain_fallbacks);
int ossl_provider_add_to_store(OSSL_PROVIDER *prov, OSSL_PROVIDER **actualprov,
int retain_fallbacks);
/* Return pointer to the provider's context */
void *ossl_provider_ctx(const OSSL_PROVIDER *prov);