[openssl] master update
Matt Caswell
matt at openssl.org
Thu Aug 29 09:55:42 UTC 2019
The branch master has been updated
via ed71e917e9fb763adfb36a9cee0e0935aee898e2 (commit)
via 505f46602043c7c28884e4c13f3cfa9419ae2f15 (commit)
via 770de3462c0d655a6543a6c1a2c0bda7b57178f9 (commit)
from c92d0c5c6550346cffb942000e99aa88452bde6d (commit)
- Log -----------------------------------------------------------------
commit ed71e917e9fb763adfb36a9cee0e0935aee898e2
Author: Matt Caswell <matt at openssl.org>
Date: Wed Aug 14 18:09:28 2019 +0100
Fix data races in EVP_CIPHER_fetch and EVP_MD_fetch
Don't modify the cipher/md we just fetched - it could be shared by multiple
threads.
Reviewed-by: Paul Dale <paul.dale at oracle.com>
Reviewed-by: Richard Levitte <levitte at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9590)
commit 505f46602043c7c28884e4c13f3cfa9419ae2f15
Author: Matt Caswell <matt at openssl.org>
Date: Wed Aug 14 15:00:35 2019 +0100
Make sure we pre-initialise properties
Simplify the initialisation of the core by pre-initialising properties.
Reviewed-by: Paul Dale <paul.dale at oracle.com>
Reviewed-by: Richard Levitte <levitte at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9590)
commit 770de3462c0d655a6543a6c1a2c0bda7b57178f9
Author: Matt Caswell <matt at openssl.org>
Date: Wed Aug 14 14:43:11 2019 +0100
Fix context locking
Some parts of OPENSSL_CTX intialisation can get quite complex (e.g. RAND).
This can lead to complex interactions where different parts of the library
try to initialise while other parts are still initialising. This can lead
to deadlocks because both parts want to obtain the init lock.
We separate out the init lock so that it is only used to manage the
dynamic list of indexes. Each part of the library gets its own
initialisation lock.
Fixes #9454
Reviewed-by: Paul Dale <paul.dale at oracle.com>
Reviewed-by: Richard Levitte <levitte at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9590)
-----------------------------------------------------------------------
Summary of changes:
crypto/context.c | 52 ++++++++++++++++++++++++++++++++++++++----
crypto/evp/digest.c | 23 +++++++++----------
crypto/evp/evp_enc.c | 27 +++++++++-------------
crypto/property/property.c | 11 ---------
crypto/property/property_lcl.h | 1 -
include/internal/property.h | 3 +++
6 files changed, 73 insertions(+), 44 deletions(-)
diff --git a/crypto/context.c b/crypto/context.c
index cc1ec0e664..a2e19bac54 100644
--- a/crypto/context.c
+++ b/crypto/context.c
@@ -9,6 +9,7 @@
#include "internal/cryptlib_int.h"
#include "internal/thread_once.h"
+#include "internal/property.h"
struct openssl_ctx_onfree_list_st {
openssl_ctx_onfree_fn *fn;
@@ -28,6 +29,9 @@ struct openssl_ctx_st {
/* Map internal static indexes to dynamically created indexes */
int dyn_indexes[OPENSSL_CTX_MAX_INDEXES];
+ /* Keep a separate lock for each index */
+ CRYPTO_RWLOCK *index_locks[OPENSSL_CTX_MAX_INDEXES];
+
CRYPTO_RWLOCK *oncelock;
int run_once_done[OPENSSL_CTX_MAX_RUN_ONCE];
int run_once_ret[OPENSSL_CTX_MAX_RUN_ONCE];
@@ -44,6 +48,7 @@ static OPENSSL_CTX *default_context = NULL;
static int context_init(OPENSSL_CTX *ctx)
{
size_t i;
+ int exdata_done = 0;
ctx->lock = CRYPTO_THREAD_lock_new();
if (ctx->lock == NULL)
@@ -53,11 +58,17 @@ static int context_init(OPENSSL_CTX *ctx)
if (ctx->oncelock == NULL)
goto err;
- for (i = 0; i < OPENSSL_CTX_MAX_INDEXES; i++)
+ for (i = 0; i < OPENSSL_CTX_MAX_INDEXES; i++) {
+ ctx->index_locks[i] = CRYPTO_THREAD_lock_new();
ctx->dyn_indexes[i] = -1;
+ if (ctx->index_locks[i] == NULL)
+ goto err;
+ }
+ /* OPENSSL_CTX is built on top of ex_data so we initialise that directly */
if (!do_ex_data_init(ctx))
goto err;
+ exdata_done = 1;
if (!crypto_new_ex_data_ex(ctx, CRYPTO_EX_INDEX_OPENSSL_CTX, NULL,
&ctx->data)) {
@@ -65,8 +76,14 @@ static int context_init(OPENSSL_CTX *ctx)
goto err;
}
+ /* Everything depends on properties, so we also pre-initialise that */
+ if (!ossl_property_parse_init(ctx))
+ goto err;
+
return 1;
err:
+ if (exdata_done)
+ crypto_cleanup_all_ex_data_int(ctx);
CRYPTO_THREAD_lock_free(ctx->oncelock);
CRYPTO_THREAD_lock_free(ctx->lock);
ctx->lock = NULL;
@@ -76,6 +93,7 @@ static int context_init(OPENSSL_CTX *ctx)
static int context_deinit(OPENSSL_CTX *ctx)
{
struct openssl_ctx_onfree_list_st *tmp, *onfree;
+ int i;
if (ctx == NULL)
return 1;
@@ -91,6 +109,9 @@ static int context_deinit(OPENSSL_CTX *ctx)
}
CRYPTO_free_ex_data(CRYPTO_EX_INDEX_OPENSSL_CTX, NULL, &ctx->data);
crypto_cleanup_all_ex_data_int(ctx);
+ for (i = 0; i < OPENSSL_CTX_MAX_INDEXES; i++)
+ CRYPTO_THREAD_lock_free(ctx->index_locks[i]);
+
CRYPTO_THREAD_lock_free(ctx->oncelock);
CRYPTO_THREAD_lock_free(ctx->lock);
ctx->lock = NULL;
@@ -187,25 +208,48 @@ void *openssl_ctx_get_data(OPENSSL_CTX *ctx, int index,
const OPENSSL_CTX_METHOD *meth)
{
void *data = NULL;
+ int dynidx;
ctx = openssl_ctx_get_concrete(ctx);
if (ctx == NULL)
return NULL;
CRYPTO_THREAD_read_lock(ctx->lock);
+ dynidx = ctx->dyn_indexes[index];
+ CRYPTO_THREAD_unlock(ctx->lock);
- if (ctx->dyn_indexes[index] == -1
- && !openssl_ctx_init_index(ctx, index, meth)) {
+ if (dynidx != -1) {
+ CRYPTO_THREAD_read_lock(ctx->index_locks[index]);
+ data = CRYPTO_get_ex_data(&ctx->data, dynidx);
+ CRYPTO_THREAD_unlock(ctx->index_locks[index]);
+ return data;
+ }
+
+ CRYPTO_THREAD_write_lock(ctx->index_locks[index]);
+ CRYPTO_THREAD_write_lock(ctx->lock);
+
+ dynidx = ctx->dyn_indexes[index];
+ if (dynidx != -1) {
CRYPTO_THREAD_unlock(ctx->lock);
+ data = CRYPTO_get_ex_data(&ctx->data, dynidx);
+ CRYPTO_THREAD_unlock(ctx->index_locks[index]);
+ return data;
+ }
+
+ if (!openssl_ctx_init_index(ctx, index, meth)) {
+ CRYPTO_THREAD_unlock(ctx->lock);
+ CRYPTO_THREAD_unlock(ctx->index_locks[index]);
return NULL;
}
+ CRYPTO_THREAD_unlock(ctx->lock);
+
/* The alloc call ensures there's a value there */
if (CRYPTO_alloc_ex_data(CRYPTO_EX_INDEX_OPENSSL_CTX, NULL,
&ctx->data, ctx->dyn_indexes[index]))
data = CRYPTO_get_ex_data(&ctx->data, ctx->dyn_indexes[index]);
- CRYPTO_THREAD_unlock(ctx->lock);
+ CRYPTO_THREAD_unlock(ctx->index_locks[index]);
return data;
}
diff --git a/crypto/evp/digest.c b/crypto/evp/digest.c
index b829b814dd..dc7f922a11 100644
--- a/crypto/evp/digest.c
+++ b/crypto/evp/digest.c
@@ -630,6 +630,17 @@ static void *evp_md_from_dispatch(const char *name, const OSSL_DISPATCH *fns,
return NULL;
}
+#ifndef FIPS_MODE
+ /*
+ * FIPS module note: since internal fetches will be entirely
+ * provider based, we know that none of its code depends on legacy
+ * NIDs or any functionality that use them.
+ *
+ * TODO(3.x) get rid of the need for legacy NIDs
+ */
+ md->type = OBJ_sn2nid(name);
+#endif
+
for (; fns->function_id != 0; fns++) {
switch (fns->function_id) {
case OSSL_FUNC_DIGEST_NEWCTX:
@@ -736,18 +747,6 @@ EVP_MD *EVP_MD_fetch(OPENSSL_CTX *ctx, const char *algorithm,
evp_md_from_dispatch, evp_md_up_ref,
evp_md_free);
-#ifndef FIPS_MODE
- /* TODO(3.x) get rid of the need for legacy NIDs */
- if (md != NULL) {
- /*
- * FIPS module note: since internal fetches will be entirely
- * provider based, we know that none of its code depends on legacy
- * NIDs or any functionality that use them.
- */
- md->type = OBJ_sn2nid(algorithm);
- }
-#endif
-
return md;
}
diff --git a/crypto/evp/evp_enc.c b/crypto/evp/evp_enc.c
index 5723fe888e..96a15ef897 100644
--- a/crypto/evp/evp_enc.c
+++ b/crypto/evp/evp_enc.c
@@ -1251,10 +1251,6 @@ static void *evp_cipher_from_dispatch(const char *name,
EVP_CIPHER *cipher = NULL;
int fnciphcnt = 0, fnctxcnt = 0;
- /*
- * The legacy NID is set by EVP_CIPHER_fetch() if the name exists in
- * the object database.
- */
if ((cipher = EVP_CIPHER_meth_new(0, 0, 0)) == NULL
|| (cipher->name = OPENSSL_strdup(name)) == NULL) {
EVP_CIPHER_meth_free(cipher);
@@ -1262,6 +1258,17 @@ static void *evp_cipher_from_dispatch(const char *name,
return NULL;
}
+#ifndef FIPS_MODE
+ /*
+ * FIPS module note: since internal fetches will be entirely
+ * provider based, we know that none of its code depends on legacy
+ * NIDs or any functionality that use them.
+ *
+ * TODO(3.x) get rid of the need for legacy NIDs
+ */
+ cipher->nid = OBJ_sn2nid(name);
+#endif
+
for (; fns->function_id != 0; fns++) {
switch (fns->function_id) {
case OSSL_FUNC_CIPHER_NEWCTX:
@@ -1382,18 +1389,6 @@ EVP_CIPHER *EVP_CIPHER_fetch(OPENSSL_CTX *ctx, const char *algorithm,
evp_cipher_from_dispatch, evp_cipher_up_ref,
evp_cipher_free);
-#ifndef FIPS_MODE
- /* TODO(3.x) get rid of the need for legacy NIDs */
- if (cipher != NULL) {
- /*
- * FIPS module note: since internal fetches will be entirely
- * provider based, we know that none of its code depends on legacy
- * NIDs or any functionality that use them.
- */
- cipher->nid = OBJ_sn2nid(algorithm);
- }
-#endif
-
return cipher;
}
diff --git a/crypto/property/property.c b/crypto/property/property.c
index 182ea6454b..e94c5de87d 100644
--- a/crypto/property/property.c
+++ b/crypto/property/property.c
@@ -84,12 +84,6 @@ int ossl_property_unlock(OSSL_METHOD_STORE *p)
return p != 0 ? CRYPTO_THREAD_unlock(p->lock) : 0;
}
-static openssl_ctx_run_once_fn do_method_store_init;
-int do_method_store_init(OPENSSL_CTX *ctx)
-{
- return ossl_property_parse_init(ctx);
-}
-
static unsigned long query_hash(const QUERY *a)
{
return OPENSSL_LH_strhash(a->query);
@@ -132,11 +126,6 @@ OSSL_METHOD_STORE *ossl_method_store_new(OPENSSL_CTX *ctx)
{
OSSL_METHOD_STORE *res;
- if (!openssl_ctx_run_once(ctx,
- OPENSSL_CTX_METHOD_STORE_RUN_ONCE_INDEX,
- do_method_store_init))
- return NULL;
-
res = OPENSSL_zalloc(sizeof(*res));
if (res != NULL) {
res->ctx = ctx;
diff --git a/crypto/property/property_lcl.h b/crypto/property/property_lcl.h
index 5fa34cea82..25cfde4649 100644
--- a/crypto/property/property_lcl.h
+++ b/crypto/property/property_lcl.h
@@ -21,7 +21,6 @@ OSSL_PROPERTY_IDX ossl_property_value(OPENSSL_CTX *ctx, const char *s,
int create);
/* Property list functions */
-int ossl_property_parse_init(OPENSSL_CTX *ctx);
void ossl_property_free(OSSL_PROPERTY_LIST *p);
int ossl_property_has_optional(const OSSL_PROPERTY_LIST *query);
int ossl_property_match_count(const OSSL_PROPERTY_LIST *query,
diff --git a/include/internal/property.h b/include/internal/property.h
index 3c6d6a9002..842c7dea17 100644
--- a/include/internal/property.h
+++ b/include/internal/property.h
@@ -15,6 +15,9 @@
typedef struct ossl_method_store_st OSSL_METHOD_STORE;
+/* Initialisation */
+int ossl_property_parse_init(OPENSSL_CTX *ctx);
+
/* Implementation store functions */
OSSL_METHOD_STORE *ossl_method_store_new(OPENSSL_CTX *ctx);
void ossl_method_store_free(OSSL_METHOD_STORE *store);
More information about the openssl-commits
mailing list