[openssl] master update
Matt Caswell
matt at openssl.org
Fri Nov 12 17:25:47 UTC 2021
The branch master has been updated
via 293e251e6f0367a9aa0d3d46037b19d1a6c91b20 (commit)
via addbd7c9d784e1cb630d43487b0572e867bfc86d (commit)
via 4aced11785f2e54875ad56f30c05bdee02b6e4e2 (commit)
via 1e8ed3e596162d7490b26fb12e58af5208f52402 (commit)
via cad22202a32a94059e351d9819e6c9ed5c66605a (commit)
via e39bd6215123f375ddcfe92fa2b2550294da0b73 (commit)
via dc6d9ede6241e6858f8fa78435d6c8eb9cf85aa1 (commit)
via 464c2b988ea149badabaf958a96fdc480df89dc7 (commit)
via 3b9de0c9aa791bd9e6f0534ec091accbdf15292f (commit)
via c59fc87b338880893286934f02c446854f5baabf (commit)
via 6de9214a5062e9d015c84cbbab681184e16fccaa (commit)
from 3641f04fb06e9679a67da113bab65e5f1bb5e9ba (commit)
- Log -----------------------------------------------------------------
commit 293e251e6f0367a9aa0d3d46037b19d1a6c91b20
Author: Matt Caswell <matt at openssl.org>
Date: Tue Nov 9 18:31:24 2021 +0000
Extend the test_multi_load() test
Run more threads and load the legacy provider (which uses a child lib ctx)
in order to hit more possible thread failures.
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
commit addbd7c9d784e1cb630d43487b0572e867bfc86d
Author: Matt Caswell <matt at openssl.org>
Date: Tue Nov 9 16:23:34 2021 +0000
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 <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
commit 4aced11785f2e54875ad56f30c05bdee02b6e4e2
Author: Matt Caswell <matt at openssl.org>
Date: Tue Nov 9 14:32:14 2021 +0000
Use a write lock during ossl_provider_find()
A "find" operation on a stack can end up sorting the underlying stack. In
this case it is necessary to use a "write" lock to synchronise access to
the stack across multiple threads.
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
commit 1e8ed3e596162d7490b26fb12e58af5208f52402
Author: Matt Caswell <matt at openssl.org>
Date: Tue Nov 9 14:20:31 2021 +0000
Correctly activate the provider in OSSL_PROVIDER_try_load
If during OSSL_PROVIDER_try_load() we attempt to load a provider, but
adding to the store gives back a different provider, then we need to
ensure this different provider has its activation count increased.
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
commit cad22202a32a94059e351d9819e6c9ed5c66605a
Author: Matt Caswell <matt at openssl.org>
Date: Tue Nov 9 13:48:31 2021 +0000
Stop receiving child callbacks in a child libctx when appropriate
We should stop receiving child callbacks if we're about to free up
the child libctx. Otherwise we can get callbacks when the libctx is half
freed up.
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
commit e39bd6215123f375ddcfe92fa2b2550294da0b73
Author: Matt Caswell <matt at openssl.org>
Date: Tue Nov 9 11:53:27 2021 +0000
Don't bail out during provider deactivation if we don't have store
A provider may have been activated, but failed when being added to
the store. At this point we still need to deactivate it.
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
commit dc6d9ede6241e6858f8fa78435d6c8eb9cf85aa1
Author: Matt Caswell <matt at openssl.org>
Date: Mon Nov 8 16:47:38 2021 +0000
Don't try and do ossl_provider_find in ossl_provider_new
We leave it to the caller to confirm that the provider does not exist
in the store. If it does exist then later adding it to the store will
fail.
It is possible that the provider could be added to the store in
between the caller checking, and the caller calling ossl_provider_new.
We leave it to the caller to properly handle the failure when it
attempts to add the provider to the store. This is simpler than
having ossl_provider_new try to handle it.
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
commit 464c2b988ea149badabaf958a96fdc480df89dc7
Author: Matt Caswell <matt at openssl.org>
Date: Mon Nov 8 16:30:43 2021 +0000
Remove the isinited variable from child_prov_globals
This variable might have made sense at some point but it not longer does
so. It was being used to check whether we are still initing or not. If we
are still initing then the assumption was that we already hold the lock.
That assumption was untrue. We need to always take the lock.
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
commit 3b9de0c9aa791bd9e6f0534ec091accbdf15292f
Author: Matt Caswell <matt at openssl.org>
Date: Fri Nov 5 14:43:01 2021 +0000
Avoid a race in init_thread_stop()
init_thread_stop() is called when a thread is stopping. It calls all
the callbacks that need to know about the demise of this thread. However,
the list of callbacks is also available globally and may be updated by
other threads so we need to make sure we use the right lock.
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
commit c59fc87b338880893286934f02c446854f5baabf
Author: Matt Caswell <matt at openssl.org>
Date: Fri Nov 5 13:42:40 2021 +0000
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 at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
commit 6de9214a5062e9d015c84cbbab681184e16fccaa
Author: Matt Caswell <matt at openssl.org>
Date: Fri Nov 5 13:29:41 2021 +0000
Don't write to the globals ossl_property_true and ossl_property_false
These global variables were previously overwritten with the same value
every time we created a new OSSL_LIB_CTX. Instead we preinitialise them
with the correct values, and then confirm that settings for each
OSSL_LIB_CTX agree with the preinitialised values.
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16980)
-----------------------------------------------------------------------
Summary of changes:
crypto/context.c | 4 ++
crypto/initthread.c | 15 ++++++
crypto/property/property_local.h | 3 +-
crypto/property/property_parse.c | 20 ++++----
crypto/property/property_query.c | 4 +-
crypto/provider.c | 10 +++-
crypto/provider_child.c | 28 ++++++-----
crypto/provider_conf.c | 17 +++++--
crypto/provider_core.c | 88 +++++++++++++++++++--------------
doc/internal/man3/ossl_provider_new.pod | 14 ++++--
doc/man3/DEFINE_STACK_OF.pod | 5 +-
include/internal/provider.h | 3 +-
test/provider_internal_test.c | 2 +-
test/threadstest.c | 22 +++++++--
14 files changed, 157 insertions(+), 78 deletions(-)
diff --git a/crypto/context.c b/crypto/context.c
index 1e0dfa8e01..bba8e4208b 100644
--- a/crypto/context.c
+++ b/crypto/context.c
@@ -240,6 +240,10 @@ void OSSL_LIB_CTX_free(OSSL_LIB_CTX *ctx)
if (ossl_lib_ctx_is_default(ctx))
return;
+#ifndef FIPS_MODULE
+ if (ctx->ischild)
+ ossl_provider_deinit_child(ctx);
+#endif
context_deinit(ctx);
OPENSSL_free(ctx);
}
diff --git a/crypto/initthread.c b/crypto/initthread.c
index d86e280fc1..1bdaeda9fc 100644
--- a/crypto/initthread.c
+++ b/crypto/initthread.c
@@ -309,11 +309,23 @@ void ossl_ctx_thread_stop(OSSL_LIB_CTX *ctx)
static void init_thread_stop(void *arg, THREAD_EVENT_HANDLER **hands)
{
THREAD_EVENT_HANDLER *curr, *prev = NULL, *tmp;
+#ifndef FIPS_MODULE
+ GLOBAL_TEVENT_REGISTER *gtr;
+#endif
/* Can't do much about this */
if (hands == NULL)
return;
+#ifndef FIPS_MODULE
+ gtr = get_global_tevent_register();
+ if (gtr == NULL)
+ return;
+
+ if (!CRYPTO_THREAD_write_lock(gtr->lock))
+ return;
+#endif
+
curr = *hands;
while (curr != NULL) {
if (arg != NULL && curr->arg != arg) {
@@ -332,6 +344,9 @@ static void init_thread_stop(void *arg, THREAD_EVENT_HANDLER **hands)
OPENSSL_free(tmp);
}
+#ifndef FIPS_MODULE
+ CRYPTO_THREAD_unlock(gtr->lock);
+#endif
}
int ossl_init_thread_start(const void *index, void *arg,
diff --git a/crypto/property/property_local.h b/crypto/property/property_local.h
index 46c5dbe3cc..6b85ce1586 100644
--- a/crypto/property/property_local.h
+++ b/crypto/property/property_local.h
@@ -34,7 +34,8 @@ struct ossl_property_list_st {
OSSL_PROPERTY_DEFINITION properties[1];
};
-extern OSSL_PROPERTY_IDX ossl_property_true, ossl_property_false;
+#define OSSL_PROPERTY_TRUE 1
+#define OSSL_PROPERTY_FALSE 2
/* Property string functions */
OSSL_PROPERTY_IDX ossl_property_name(OSSL_LIB_CTX *ctx, const char *s,
diff --git a/crypto/property/property_parse.c b/crypto/property/property_parse.c
index 3c0a6ff793..dc35646be2 100644
--- a/crypto/property/property_parse.c
+++ b/crypto/property/property_parse.c
@@ -19,8 +19,6 @@
#include "property_local.h"
#include "e_os.h"
-OSSL_PROPERTY_IDX ossl_property_true, ossl_property_false;
-
DEFINE_STACK_OF(OSSL_PROPERTY_DEFINITION)
static const char *skip_space(const char *s)
@@ -352,7 +350,7 @@ OSSL_PROPERTY_LIST *ossl_parse_property(OSSL_LIB_CTX *ctx, const char *defn)
} else {
/* A name alone means a true Boolean */
prop->type = OSSL_PROPERTY_TYPE_STRING;
- prop->v.str_val = ossl_property_true;
+ prop->v.str_val = OSSL_PROPERTY_TRUE;
}
if (!sk_OSSL_PROPERTY_DEFINITION_push(sk, prop))
@@ -411,7 +409,7 @@ OSSL_PROPERTY_LIST *ossl_parse_query(OSSL_LIB_CTX *ctx, const char *s,
/* A name alone is a Boolean comparison for true */
prop->oper = OSSL_PROPERTY_OPER_EQ;
prop->type = OSSL_PROPERTY_TYPE_STRING;
- prop->v.str_val = ossl_property_true;
+ prop->v.str_val = OSSL_PROPERTY_TRUE;
goto skip_value;
}
if (!parse_value(ctx, &s, prop, create_values))
@@ -485,9 +483,9 @@ int ossl_property_match_count(const OSSL_PROPERTY_LIST *query,
return -1;
} else if (q[i].type != OSSL_PROPERTY_TYPE_STRING
|| (oper == OSSL_PROPERTY_OPER_EQ
- && q[i].v.str_val != ossl_property_false)
+ && q[i].v.str_val != OSSL_PROPERTY_FALSE)
|| (oper == OSSL_PROPERTY_OPER_NE
- && q[i].v.str_val == ossl_property_false)) {
+ && q[i].v.str_val == OSSL_PROPERTY_FALSE)) {
if (!q[i].optional)
return -1;
} else {
@@ -560,9 +558,13 @@ int ossl_property_parse_init(OSSL_LIB_CTX *ctx)
if (ossl_property_name(ctx, predefined_names[i], 1) == 0)
goto err;
- /* Pre-populate the two Boolean values */
- if ((ossl_property_true = ossl_property_value(ctx, "yes", 1)) == 0
- || (ossl_property_false = ossl_property_value(ctx, "no", 1)) == 0)
+ /*
+ * Pre-populate the two Boolean values. We must do them before any other
+ * values and in this order so that we get the same index as the global
+ * OSSL_PROPERTY_TRUE and OSSL_PROPERTY_FALSE values
+ */
+ if ((ossl_property_value(ctx, "yes", 1) != OSSL_PROPERTY_TRUE)
+ || (ossl_property_value(ctx, "no", 1) != OSSL_PROPERTY_FALSE))
goto err;
return 1;
diff --git a/crypto/property/property_query.c b/crypto/property/property_query.c
index 1352bc009e..28cc704840 100644
--- a/crypto/property/property_query.c
+++ b/crypto/property/property_query.c
@@ -75,8 +75,8 @@ int ossl_property_is_enabled(OSSL_LIB_CTX *ctx, const char *property_name,
return 0;
return (prop->type == OSSL_PROPERTY_TYPE_STRING
&& ((prop->oper == OSSL_PROPERTY_OPER_EQ
- && prop->v.str_val == ossl_property_true)
+ && prop->v.str_val == OSSL_PROPERTY_TRUE)
|| (prop->oper == OSSL_PROPERTY_OPER_NE
- && prop->v.str_val != ossl_property_true)));
+ && prop->v.str_val != OSSL_PROPERTY_TRUE)));
}
diff --git a/crypto/provider.c b/crypto/provider.c
index 82d980a8ae..114b426929 100644
--- a/crypto/provider.c
+++ b/crypto/provider.c
@@ -35,10 +35,16 @@ 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;
}
+ if (actual != prov) {
+ if (!ossl_provider_activate(actual, 1, 0)) {
+ ossl_provider_free(actual);
+ return NULL;
+ }
+ }
return actual;
}
@@ -53,7 +59,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;
diff --git a/crypto/provider_child.c b/crypto/provider_child.c
index 272d67a52d..977ea4db3b 100644
--- a/crypto/provider_child.c
+++ b/crypto/provider_child.c
@@ -22,7 +22,6 @@ DEFINE_STACK_OF(OSSL_PROVIDER)
struct child_prov_globals {
const OSSL_CORE_HANDLE *handle;
const OSSL_CORE_HANDLE *curr_prov;
- unsigned int isinited:1;
CRYPTO_RWLOCK *lock;
OSSL_FUNC_core_get_libctx_fn *c_get_libctx;
OSSL_FUNC_provider_register_child_cb_fn *c_provider_register_child_cb;
@@ -43,7 +42,6 @@ static void child_prov_ossl_ctx_free(void *vgbl)
{
struct child_prov_globals *gbl = vgbl;
- gbl->c_provider_deregister_child_cb(gbl->handle);
CRYPTO_THREAD_lock_free(gbl->lock);
OPENSSL_free(gbl);
}
@@ -110,11 +108,7 @@ static int provider_create_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
if (gbl == NULL)
return 0;
- /*
- * If !gbl->isinited, then we are still initing and we already hold the
- * lock - so don't take it again.
- */
- if (gbl->isinited && !CRYPTO_THREAD_write_lock(gbl->lock))
+ if (!CRYPTO_THREAD_write_lock(gbl->lock))
return 0;
provname = gbl->c_prov_name(prov);
@@ -153,7 +147,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;
}
@@ -161,8 +155,7 @@ static int provider_create_child_cb(const OSSL_CORE_HANDLE *prov, void *cbdata)
ret = 1;
err:
- if (gbl->isinited)
- CRYPTO_THREAD_unlock(gbl->lock);
+ CRYPTO_THREAD_unlock(gbl->lock);
return ret;
}
@@ -188,7 +181,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;
@@ -272,11 +265,20 @@ int ossl_provider_init_as_child(OSSL_LIB_CTX *ctx,
ctx))
return 0;
- gbl->isinited = 1;
-
return 1;
}
+void ossl_provider_deinit_child(OSSL_LIB_CTX *ctx)
+{
+ struct child_prov_globals *gbl
+ = ossl_lib_ctx_get_data(ctx, OSSL_LIB_CTX_CHILD_PROVIDER_INDEX,
+ &child_prov_ossl_ctx_method);
+ if (gbl == NULL)
+ return;
+
+ gbl->c_provider_deregister_child_cb(gbl->handle);
+}
+
int ossl_provider_up_ref_parent(OSSL_PROVIDER *prov, int activate)
{
struct child_prov_globals *gbl;
diff --git a/crypto/provider_conf.c b/crypto/provider_conf.c
index 054261771a..c13c887c3d 100644
--- a/crypto/provider_conf.c
+++ b/crypto/provider_conf.c
@@ -222,13 +222,24 @@ 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 (actual != prov
+ && !ossl_provider_activate(actual, 1, 0)) {
+ ossl_provider_free(actual);
ok = 0;
} else {
if (pcgbl->activated_providers == NULL)
pcgbl->activated_providers = sk_OSSL_PROVIDER_new_null();
- sk_OSSL_PROVIDER_push(pcgbl->activated_providers, actual);
- ok = 1;
+ if (pcgbl->activated_providers == NULL
+ || !sk_OSSL_PROVIDER_push(pcgbl->activated_providers,
+ actual)) {
+ ossl_provider_deactivate(actual, 1);
+ ossl_provider_free(actual);
+ ok = 0;
+ } else {
+ ok = 1;
+ }
}
}
if (!ok)
diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index 3b8d3fbb6d..1d5787a648 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,
@@ -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);
}
@@ -423,7 +423,11 @@ OSSL_PROVIDER *ossl_provider_find(OSSL_LIB_CTX *libctx, const char *name,
#endif
tmpl.name = (char *)name;
- if (!CRYPTO_THREAD_read_lock(store->lock))
+ /*
+ * A "find" operation can sort the stack, and therefore a write lock is
+ * required.
+ */
+ if (!CRYPTO_THREAD_write_lock(store->lock))
return NULL;
if ((i = sk_OSSL_PROVIDER_find(store->providers, &tmpl)) != -1)
prov = sk_OSSL_PROVIDER_value(store->providers, i);
@@ -498,13 +502,18 @@ 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;
}
#endif
+/*
+ * We assume that the requested provider does not already exist in the store.
+ * The caller should check. If it does exist then adding it to the store later
+ * will fail.
+ */
OSSL_PROVIDER *ossl_provider_new(OSSL_LIB_CTX *libctx, const char *name,
OSSL_provider_init_fn *init_function,
int noconfig)
@@ -516,14 +525,6 @@ OSSL_PROVIDER *ossl_provider_new(OSSL_LIB_CTX *libctx, const char *name,
if ((store = get_provider_store(libctx)) == NULL)
return NULL;
- if ((prov = ossl_provider_find(libctx, name,
- noconfig)) != NULL) { /* refcount +1 */
- ossl_provider_free(prov); /* refcount -1 */
- ERR_raise_data(ERR_LIB_CRYPTO, CRYPTO_R_PROVIDER_ALREADY_EXISTS,
- "name=%s", name);
- return NULL;
- }
-
memset(&template, 0, sizeof(template));
if (init_function == NULL) {
const OSSL_PROVIDER_INFO *p;
@@ -644,8 +645,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,27 +1006,35 @@ 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
+ int lock = 1;
if (!ossl_assert(prov != NULL))
return -1;
+ /*
+ * No need to lock if we've got no store because we've not been shared with
+ * other threads.
+ */
store = get_provider_store(prov->libctx);
if (store == NULL)
- return -1;
+ lock = 0;
- if (!CRYPTO_THREAD_read_lock(store->lock))
+ if (lock && !CRYPTO_THREAD_read_lock(store->lock))
return -1;
- if (!CRYPTO_THREAD_write_lock(prov->flag_lock)) {
+ if (lock && !CRYPTO_THREAD_write_lock(prov->flag_lock)) {
CRYPTO_THREAD_unlock(store->lock);
return -1;
}
@@ -1039,17 +1051,15 @@ 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);
#ifndef FIPS_MODULE
- if (removechildren) {
+ if (removechildren && store != NULL) {
int i, max = sk_OSSL_PROVIDER_CHILD_CB_num(store->child_cbs);
OSSL_PROVIDER_CHILD_CB *child_cb;
@@ -1059,7 +1069,10 @@ static int provider_deactivate(OSSL_PROVIDER *prov, int upcalls)
}
}
#endif
- CRYPTO_THREAD_unlock(store->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);
@@ -1077,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;
/*
@@ -1115,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;
@@ -1169,11 +1180,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 +1367,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
diff --git a/doc/internal/man3/ossl_provider_new.pod b/doc/internal/man3/ossl_provider_new.pod
index f6bdaecde2..1dba1860f7 100644
--- a/doc/internal/man3/ossl_provider_new.pod
+++ b/doc/internal/man3/ossl_provider_new.pod
@@ -8,7 +8,7 @@ ossl_provider_set_module_path,
ossl_provider_add_parameter, ossl_provider_set_child, ossl_provider_get_parent,
ossl_provider_up_ref_parent, ossl_provider_free_parent,
ossl_provider_default_props_update, ossl_provider_get0_dispatch,
-ossl_provider_init_as_child,
+ossl_provider_init_as_child, ossl_provider_deinit_child,
ossl_provider_activate, ossl_provider_deactivate, ossl_provider_add_to_store,
ossl_provider_ctx,
ossl_provider_doall_activated,
@@ -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);
@@ -98,7 +98,7 @@ ossl_provider_get_capabilities
int ossl_provider_init_as_child(OSSL_LIB_CTX *ctx,
const OSSL_CORE_HANDLE *handle,
const OSSL_DISPATCH *in);
-
+ void ossl_provider_deinit_child(OSSL_LIB_CTX *ctx);
=head1 DESCRIPTION
@@ -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
@@ -300,6 +302,10 @@ the necessary upcalls for managing child providers. The I<handle> and I<in>
parameters are the B<OSSL_CORE_HANDLE> and B<OSSL_DISPATCH> pointers that were
passed to the provider's B<OSSL_provider_init> function.
+ossl_provider_deinit_child() deregisters callbacks from the parent library
+context about provider creation or removal events for the child library context
+I<ctx>. Must only be called if I<ctx> is a child library context.
+
=head1 NOTES
Locating a provider module happens as follows:
diff --git a/doc/man3/DEFINE_STACK_OF.pod b/doc/man3/DEFINE_STACK_OF.pod
index d7152466f4..ec9eda81c6 100644
--- a/doc/man3/DEFINE_STACK_OF.pod
+++ b/doc/man3/DEFINE_STACK_OF.pod
@@ -178,7 +178,10 @@ where a comparison function has been specified, I<sk> is sorted and
B<sk_I<TYPE>_find>() returns the index of a matching element or B<-1> if there
is no match. Note that, in this case the comparison function will usually
compare the values pointed to rather than the pointers themselves and
-the order of elements in I<sk> can change.
+the order of elements in I<sk> can change. Note that because the stack may be
+sorted as the result of a B<sk_I<TYPE>_find>() call, if a lock is being used to
+synchronise access to the stack across multiple threads, then that lock must be
+a "write" lock.
B<sk_I<TYPE>_find_ex>() operates like B<sk_I<TYPE>_find>() except when a
comparison function has been specified and no matching element is found.
diff --git a/include/internal/provider.h b/include/internal/provider.h
index 9b9c62ebe8..b8ba7f6892 100644
--- a/include/internal/provider.h
+++ b/include/internal/provider.h
@@ -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);
@@ -107,6 +107,7 @@ void ossl_provider_add_conf_module(void);
int ossl_provider_init_as_child(OSSL_LIB_CTX *ctx,
const OSSL_CORE_HANDLE *handle,
const OSSL_DISPATCH *in);
+void ossl_provider_deinit_child(OSSL_LIB_CTX *ctx);
# ifdef __cplusplus
}
diff --git a/test/provider_internal_test.c b/test/provider_internal_test.c
index d9cc68d59d..cb7d5efcf5 100644
--- a/test/provider_internal_test.c
+++ b/test/provider_internal_test.c
@@ -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);
diff --git a/test/threadstest.c b/test/threadstest.c
index f689676c54..cfd5991770 100644
--- a/test/threadstest.c
+++ b/test/threadstest.c
@@ -30,7 +30,7 @@
#include "threadstest.h"
/* Limit the maximum number of threads */
-#define MAXIMUM_THREADS 3
+#define MAXIMUM_THREADS 10
/* Limit the maximum number of providers loaded into a library context */
#define MAXIMUM_PROVIDERS 4
@@ -558,6 +558,7 @@ static int test_multi_load_unload_provider(void)
return testresult;
}
+static char *multi_load_provider = "legacy";
/*
* This test attempts to load several providers at the same time, and if
* run with a thread sanitizer, should crash if the core provider code
@@ -567,7 +568,7 @@ static void test_multi_load_worker(void)
{
OSSL_PROVIDER *prov;
- if (!TEST_ptr(prov = OSSL_PROVIDER_load(multi_libctx, "default"))
+ if (!TEST_ptr(prov = OSSL_PROVIDER_load(multi_libctx, multi_load_provider))
|| !TEST_true(OSSL_PROVIDER_unload(prov)))
multi_success = 0;
}
@@ -588,6 +589,7 @@ static int test_multi_default(void)
static int test_multi_load(void)
{
int res = 1;
+ OSSL_PROVIDER *prov;
/* The multidefault test must run prior to this test */
if (!multidefault_run) {
@@ -595,7 +597,21 @@ static int test_multi_load(void)
res = test_multi_default();
}
- return thread_run_test(NULL, 3, &test_multi_load_worker, 0, NULL) && res;
+ /*
+ * We use the legacy provider in test_multi_load_worker because it uses a
+ * child libctx that might hit more codepaths that might be sensitive to
+ * threading issues. But in a no-legacy build that won't be loadable so
+ * we use the default provider instead.
+ */
+ prov = OSSL_PROVIDER_load(NULL, "legacy");
+ if (prov == NULL) {
+ TEST_info("Cannot load legacy provider - assuming this is a no-legacy build");
+ multi_load_provider = "default";
+ }
+ OSSL_PROVIDER_unload(prov);
+
+ return thread_run_test(NULL, MAXIMUM_THREADS, &test_multi_load_worker, 0,
+ NULL) && res;
}
static void test_obj_create_one(void)
More information about the openssl-commits
mailing list