[openssl] master update
Dr. Paul Dale
pauli at openssl.org
Thu Apr 8 07:47:17 UTC 2021
The branch master has been updated
via 4b1f34f11ff94bc2d0169d6669bfc47f99bd93b3 (commit)
via a135dea4e024ea6750be25859c1d613789a4d575 (commit)
via 860ecfd70022fa5700c7fb129845129b4c674ecd (commit)
from 9695f6de1579f5d46e75cfebbaf44bc99cb421ec (commit)
- Log -----------------------------------------------------------------
commit 4b1f34f11ff94bc2d0169d6669bfc47f99bd93b3
Author: Pauli <pauli at openssl.org>
Date: Wed Apr 7 11:32:59 2021 +1000
property: lock the lib ctx when updating the property definition cache
Although the store being used is adequately and properly locked, the library
context is not. Due to the mechanisms used for fetching, it is possible for
multiple stores to live within the same library context for short periods.
This fix prevents threading issues resulting from such coincidences.
Reviewed-by: Matt Caswell <matt at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14773)
commit a135dea4e024ea6750be25859c1d613789a4d575
Author: Pauli <pauli at openssl.org>
Date: Tue Mar 30 12:27:44 2021 +1000
test: fix problem with threads test using default library context.
Also add a new test that deliberately tests the default library context.
Fixes #14720
Reviewed-by: Matt Caswell <matt at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14773)
commit 860ecfd70022fa5700c7fb129845129b4c674ecd
Author: Pauli <pauli at openssl.org>
Date: Tue Mar 30 10:29:01 2021 +1000
property: check return values from the property locking calls.
A failure to obtain a lock would have resulted in much badness, now it results
in a failure return.
Reviewed-by: Matt Caswell <matt at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14773)
-----------------------------------------------------------------------
Summary of changes:
crypto/context.c | 16 +++++++++++++++
crypto/evp/evp_fetch.c | 7 ++++---
crypto/property/defn_cache.c | 18 ++++++++++++-----
crypto/property/property.c | 29 +++++++++++++++++---------
crypto/property/property_local.h | 6 ------
crypto/provider_core.c | 2 +-
doc/internal/man3/OSSL_METHOD_STORE.pod | 4 ++--
include/crypto/evp.h | 2 +-
include/internal/core.h | 4 ++++
include/internal/property.h | 2 +-
test/threadstest.c | 36 ++++++++++++++++++++++++++++++++-
11 files changed, 96 insertions(+), 30 deletions(-)
diff --git a/crypto/context.c b/crypto/context.c
index 852f9dd7ce..6c088e6628 100644
--- a/crypto/context.c
+++ b/crypto/context.c
@@ -11,6 +11,7 @@
#include <openssl/conf.h>
#include "internal/thread_once.h"
#include "internal/property.h"
+#include "internal/core.h"
struct ossl_lib_ctx_onfree_list_st {
ossl_lib_ctx_onfree_fn *fn;
@@ -39,6 +40,21 @@ struct ossl_lib_ctx_st {
struct ossl_lib_ctx_onfree_list_st *onfreelist;
};
+int ossl_lib_ctx_write_lock(OSSL_LIB_CTX *ctx)
+{
+ return CRYPTO_THREAD_write_lock(ossl_lib_ctx_get_concrete(ctx)->lock);
+}
+
+int ossl_lib_ctx_read_lock(OSSL_LIB_CTX *ctx)
+{
+ return CRYPTO_THREAD_read_lock(ossl_lib_ctx_get_concrete(ctx)->lock);
+}
+
+int ossl_lib_ctx_unlock(OSSL_LIB_CTX *ctx)
+{
+ return CRYPTO_THREAD_unlock(ossl_lib_ctx_get_concrete(ctx)->lock);
+}
+
static int context_init(OSSL_LIB_CTX *ctx)
{
size_t i;
diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c
index 4b81204046..3893220441 100644
--- a/crypto/evp/evp_fetch.c
+++ b/crypto/evp/evp_fetch.c
@@ -371,12 +371,13 @@ void *evp_generic_fetch_by_number(OSSL_LIB_CTX *libctx, int operation_id,
free_method);
}
-void evp_method_store_flush(OSSL_LIB_CTX *libctx)
+int evp_method_store_flush(OSSL_LIB_CTX *libctx)
{
OSSL_METHOD_STORE *store = get_evp_method_store(libctx);
if (store != NULL)
- ossl_method_store_flush_cache(store, 1);
+ return ossl_method_store_flush_cache(store, 1);
+ return 1;
}
static int evp_set_parsed_default_properties(OSSL_LIB_CTX *libctx,
@@ -390,7 +391,7 @@ static int evp_set_parsed_default_properties(OSSL_LIB_CTX *libctx,
ossl_property_free(*plp);
*plp = def_prop;
if (store != NULL)
- ossl_method_store_flush_cache(store, 0);
+ return ossl_method_store_flush_cache(store, 0);
return 1;
}
ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR);
diff --git a/crypto/property/defn_cache.c b/crypto/property/defn_cache.c
index b3aefe8f8e..13b2d60770 100644
--- a/crypto/property/defn_cache.c
+++ b/crypto/property/defn_cache.c
@@ -13,6 +13,7 @@
#include <openssl/lhash.h>
#include "internal/propertyerr.h"
#include "internal/property.h"
+#include "internal/core.h"
#include "property_local.h"
/*
@@ -74,11 +75,12 @@ OSSL_PROPERTY_LIST *ossl_prop_defn_get(OSSL_LIB_CTX *ctx, const char *prop)
property_defns = ossl_lib_ctx_get_data(ctx,
OSSL_LIB_CTX_PROPERTY_DEFN_INDEX,
&property_defns_method);
- if (property_defns == NULL)
+ if (property_defns == NULL || !ossl_lib_ctx_read_lock(ctx))
return NULL;
elem.prop = prop;
r = lh_PROPERTY_DEFN_ELEM_retrieve(property_defns, &elem);
+ ossl_lib_ctx_unlock(ctx);
return r != NULL ? r->defn : NULL;
}
@@ -88,6 +90,7 @@ int ossl_prop_defn_set(OSSL_LIB_CTX *ctx, const char *prop,
PROPERTY_DEFN_ELEM elem, *old, *p = NULL;
size_t len;
LHASH_OF(PROPERTY_DEFN_ELEM) *property_defns;
+ int res = 1;
property_defns = ossl_lib_ctx_get_data(ctx,
OSSL_LIB_CTX_PROPERTY_DEFN_INDEX,
@@ -98,10 +101,12 @@ int ossl_prop_defn_set(OSSL_LIB_CTX *ctx, const char *prop,
if (prop == NULL)
return 1;
+ if (!ossl_lib_ctx_write_lock(ctx))
+ return 0;
if (pl == NULL) {
elem.prop = prop;
lh_PROPERTY_DEFN_ELEM_delete(property_defns, &elem);
- return 1;
+ goto end;
}
len = strlen(prop);
p = OPENSSL_malloc(sizeof(*p) + len);
@@ -112,11 +117,14 @@ int ossl_prop_defn_set(OSSL_LIB_CTX *ctx, const char *prop,
old = lh_PROPERTY_DEFN_ELEM_insert(property_defns, p);
if (old != NULL) {
property_defn_free(old);
- return 1;
+ goto end;
}
if (!lh_PROPERTY_DEFN_ELEM_error(property_defns))
- return 1;
+ goto end;
}
OPENSSL_free(p);
- return 0;
+ res = 0;
+ end:
+ ossl_lib_ctx_unlock(ctx);
+ return res;
}
diff --git a/crypto/property/property.c b/crypto/property/property.c
index b6e295e5cd..c424e37bfb 100644
--- a/crypto/property/property.c
+++ b/crypto/property/property.c
@@ -117,17 +117,17 @@ static void ossl_method_free(METHOD *method)
(*method->free)(method->method);
}
-int ossl_property_read_lock(OSSL_METHOD_STORE *p)
+static __owur int ossl_property_read_lock(OSSL_METHOD_STORE *p)
{
return p != NULL ? CRYPTO_THREAD_read_lock(p->lock) : 0;
}
-int ossl_property_write_lock(OSSL_METHOD_STORE *p)
+static __owur int ossl_property_write_lock(OSSL_METHOD_STORE *p)
{
return p != NULL ? CRYPTO_THREAD_write_lock(p->lock) : 0;
}
-int ossl_property_unlock(OSSL_METHOD_STORE *p)
+static int ossl_property_unlock(OSSL_METHOD_STORE *p)
{
return p != 0 ? CRYPTO_THREAD_unlock(p->lock) : 0;
}
@@ -246,7 +246,10 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
* A write lock is used unconditionally because we wend our way down to the
* property string code which isn't locking friendly.
*/
- ossl_property_write_lock(store);
+ if (!ossl_property_write_lock(store)) {
+ OPENSSL_free(impl);
+ return 0;
+ }
ossl_method_cache_flush(store, nid);
if ((impl->properties = ossl_prop_defn_get(store->ctx, properties)) == NULL) {
impl->properties = ossl_parse_property(store->ctx, properties);
@@ -298,7 +301,8 @@ int ossl_method_store_remove(OSSL_METHOD_STORE *store, int nid,
if (nid <= 0 || method == NULL || store == NULL)
return 0;
- ossl_property_write_lock(store);
+ if (!ossl_property_write_lock(store))
+ return 0;
ossl_method_cache_flush(store, nid);
alg = ossl_method_store_retrieve(store, nid);
if (alg == NULL) {
@@ -349,7 +353,8 @@ int ossl_method_store_fetch(OSSL_METHOD_STORE *store, int nid,
* This only needs to be a read lock, because queries never create property
* names or value and thus don't modify any of the property string layer.
*/
- ossl_property_read_lock(store);
+ if (!ossl_property_read_lock(store))
+ return 0;
alg = ossl_method_store_retrieve(store, nid);
if (alg == NULL) {
ossl_property_unlock(store);
@@ -425,14 +430,16 @@ static void ossl_method_cache_flush(OSSL_METHOD_STORE *store, int nid)
}
}
-void ossl_method_store_flush_cache(OSSL_METHOD_STORE *store, int all)
+int ossl_method_store_flush_cache(OSSL_METHOD_STORE *store, int all)
{
void *arg = (all != 0 ? store->algs : NULL);
- ossl_property_write_lock(store);
+ if (!ossl_property_write_lock(store))
+ return 0;
ossl_sa_ALGORITHM_doall_arg(store->algs, &impl_cache_flush_alg, arg);
store->nelem = 0;
ossl_property_unlock(store);
+ return 1;
}
IMPLEMENT_LHASH_DOALL_ARG(QUERY, IMPL_CACHE_FLUSH);
@@ -508,7 +515,8 @@ int ossl_method_store_cache_get(OSSL_METHOD_STORE *store, int nid,
if (nid <= 0 || store == NULL)
return 0;
- ossl_property_read_lock(store);
+ if (!ossl_property_read_lock(store))
+ return 0;
alg = ossl_method_store_retrieve(store, nid);
if (alg == NULL)
goto err;
@@ -541,7 +549,8 @@ int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, int nid,
if (prop_query == NULL)
return 1;
- ossl_property_write_lock(store);
+ if (!ossl_property_write_lock(store))
+ return 0;
if (store->need_flush)
ossl_method_cache_flush_some(store);
alg = ossl_method_store_retrieve(store, nid);
diff --git a/crypto/property/property_local.h b/crypto/property/property_local.h
index 89020e606e..58db822851 100644
--- a/crypto/property/property_local.h
+++ b/crypto/property/property_local.h
@@ -27,9 +27,3 @@ int ossl_property_has_optional(const OSSL_PROPERTY_LIST *query);
OSSL_PROPERTY_LIST *ossl_prop_defn_get(OSSL_LIB_CTX *ctx, const char *prop);
int ossl_prop_defn_set(OSSL_LIB_CTX *ctx, const char *prop,
OSSL_PROPERTY_LIST *pl);
-
-/* Property cache lock / unlock */
-int ossl_property_write_lock(OSSL_METHOD_STORE *);
-int ossl_property_read_lock(OSSL_METHOD_STORE *);
-int ossl_property_unlock(OSSL_METHOD_STORE *);
-
diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index ac094f0bdd..f3a4f297bf 100644
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -956,7 +956,7 @@ int ossl_provider_self_test(const OSSL_PROVIDER *prov)
return 1;
ret = prov->self_test(prov->provctx);
if (ret == 0)
- evp_method_store_flush(ossl_provider_libctx(prov));
+ (void)evp_method_store_flush(ossl_provider_libctx(prov));
return ret;
}
diff --git a/doc/internal/man3/OSSL_METHOD_STORE.pod b/doc/internal/man3/OSSL_METHOD_STORE.pod
index 7fbd899754..46e682da00 100644
--- a/doc/internal/man3/OSSL_METHOD_STORE.pod
+++ b/doc/internal/man3/OSSL_METHOD_STORE.pod
@@ -104,8 +104,8 @@ ossl_method_store_new() returns a new method store object or NULL on failure.
ossl_method_store_free(), ossl_method_store_add(),
ossl_method_store_remove(), ossl_method_store_fetch(),
-ossl_method_store_cache_get()
-and ossl_method_store_cache_set() return B<1> on success and B<0> on error.
+ossl_method_store_cache_get(), ossl_method_store_cache_set() and
+ossl_method_store_flush_cache() return B<1> on success and B<0> on error.
ossl_method_store_free() and ossl_method_store_cleanup() do not return any value.
diff --git a/include/crypto/evp.h b/include/crypto/evp.h
index fbd0131e78..8ea5a2bf35 100644
--- a/include/crypto/evp.h
+++ b/include/crypto/evp.h
@@ -868,7 +868,7 @@ int evp_pkey_ctx_get1_id_len_prov(EVP_PKEY_CTX *ctx, size_t *id_len);
int evp_pkey_ctx_use_cached_data(EVP_PKEY_CTX *ctx);
# endif /* !defined(FIPS_MODULE) */
-void evp_method_store_flush(OSSL_LIB_CTX *libctx);
+int evp_method_store_flush(OSSL_LIB_CTX *libctx);
int evp_set_default_properties_int(OSSL_LIB_CTX *libctx, const char *propq,
int loadconfig);
diff --git a/include/internal/core.h b/include/internal/core.h
index c3c2b74a63..6e66bbeb9a 100644
--- a/include/internal/core.h
+++ b/include/internal/core.h
@@ -60,4 +60,8 @@ void ossl_algorithm_do_all(OSSL_LIB_CTX *libctx, int operation_id,
int no_store, void *data, int *result),
void *data);
+__owur int ossl_lib_ctx_write_lock(OSSL_LIB_CTX *ctx);
+__owur int ossl_lib_ctx_read_lock(OSSL_LIB_CTX *ctx);
+int ossl_lib_ctx_unlock(OSSL_LIB_CTX *ctx);
+
#endif
diff --git a/include/internal/property.h b/include/internal/property.h
index 3d00e3cb66..58ceddbb76 100644
--- a/include/internal/property.h
+++ b/include/internal/property.h
@@ -58,7 +58,7 @@ int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, int nid,
int (*method_up_ref)(void *),
void (*method_destruct)(void *));
-void ossl_method_store_flush_cache(OSSL_METHOD_STORE *store, int all);
+__owur int ossl_method_store_flush_cache(OSSL_METHOD_STORE *store, int all);
/* Merge two property queries together */
OSSL_PROPERTY_LIST *ossl_property_merge(const OSSL_PROPERTY_LIST *a,
diff --git a/test/threadstest.c b/test/threadstest.c
index 17b348b80c..83a3f978f9 100644
--- a/test/threadstest.c
+++ b/test/threadstest.c
@@ -348,7 +348,7 @@ static void thread_general_worker(void)
static void thread_multi_simple_fetch(void)
{
- EVP_MD *md = EVP_MD_fetch(NULL, "SHA2-256", NULL);
+ EVP_MD *md = EVP_MD_fetch(multi_libctx, "SHA2-256", NULL);
if (md != NULL)
EVP_MD_free(md);
@@ -501,6 +501,7 @@ static int test_multi(int idx)
OSSL_LIB_CTX_free(multi_libctx);
EVP_PKEY_free(shared_evp_pkey);
shared_evp_pkey = NULL;
+ multi_libctx = NULL;
return testresult;
}
@@ -532,6 +533,36 @@ static int test_multi_load(void)
return 1;
}
+static int test_multi_default(void)
+{
+ thread_t thread1, thread2;
+ int testresult = 0;
+ OSSL_PROVIDER *prov = NULL;
+
+ multi_success = 1;
+ multi_libctx = NULL;
+ prov = OSSL_PROVIDER_load(multi_libctx, "default");
+ if (!TEST_ptr(prov))
+ goto err;
+
+ if (!TEST_true(run_thread(&thread1, thread_multi_simple_fetch))
+ || !TEST_true(run_thread(&thread2, thread_multi_simple_fetch)))
+ goto err;
+
+ thread_multi_simple_fetch();
+
+ if (!TEST_true(wait_for_thread(thread1))
+ || !TEST_true(wait_for_thread(thread2))
+ || !TEST_true(multi_success))
+ goto err;
+
+ testresult = 1;
+
+ err:
+ OSSL_PROVIDER_unload(prov);
+ return testresult;
+}
+
typedef enum OPTION_choice {
OPT_ERR = -1,
OPT_EOF = 0,
@@ -573,6 +604,9 @@ int setup_tests(void)
if (!TEST_ptr(privkey))
return 0;
+ /* Keep first to validate auto creation of default library context */
+ ADD_TEST(test_multi_default);
+
ADD_TEST(test_lock);
ADD_TEST(test_once);
ADD_TEST(test_thread_local);
More information about the openssl-commits
mailing list