diff options
author | Matt Caswell <matt@openssl.org> | 2021-01-26 17:00:25 +0000 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2021-02-02 12:21:21 +0000 |
commit | 04b9435a991585d0f9a775a203cc3986d4872a6e (patch) | |
tree | 745fdbd73fab4b9c3bfb27af89e276ea46710f63 /crypto | |
parent | b233ea82765e80038e4884564153f9c8543d9396 (diff) | |
download | openssl-04b9435a991585d0f9a775a203cc3986d4872a6e.zip openssl-04b9435a991585d0f9a775a203cc3986d4872a6e.tar.gz openssl-04b9435a991585d0f9a775a203cc3986d4872a6e.tar.bz2 |
Always ensure we hold ctx->lock when calling CRYPTO_get_ex_data()
Otherwise we can get data races.
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13987)
Diffstat (limited to 'crypto')
-rw-r--r-- | crypto/context.c | 22 | ||||
-rw-r--r-- | crypto/ex_data.c | 13 |
2 files changed, 28 insertions, 7 deletions
diff --git a/crypto/context.c b/crypto/context.c index 7efe475..5a46921 100644 --- a/crypto/context.c +++ b/crypto/context.c @@ -283,7 +283,9 @@ void *ossl_lib_ctx_get_data(OSSL_LIB_CTX *ctx, int index, if (dynidx != -1) { CRYPTO_THREAD_read_lock(ctx->index_locks[index]); + CRYPTO_THREAD_read_lock(ctx->lock); data = CRYPTO_get_ex_data(&ctx->data, dynidx); + CRYPTO_THREAD_unlock(ctx->lock); CRYPTO_THREAD_unlock(ctx->index_locks[index]); return data; } @@ -293,8 +295,8 @@ void *ossl_lib_ctx_get_data(OSSL_LIB_CTX *ctx, int index, 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->lock); CRYPTO_THREAD_unlock(ctx->index_locks[index]); return data; } @@ -307,10 +309,22 @@ void *ossl_lib_ctx_get_data(OSSL_LIB_CTX *ctx, int index, CRYPTO_THREAD_unlock(ctx->lock); - /* The alloc call ensures there's a value there */ - if (CRYPTO_alloc_ex_data(CRYPTO_EX_INDEX_OSSL_LIB_CTX, NULL, - &ctx->data, ctx->dyn_indexes[index])) + /* + * The alloc call ensures there's a value there. We release the ctx->lock + * for this, because the allocation itself may recursively call + * ossl_lib_ctx_get_data for other indexes (never this one). The allocation + * will itself aquire the ctx->lock when it actually comes to store the + * allocated data (see ossl_lib_ctx_generic_new() above). We call + * ossl_crypto_alloc_ex_data_intern() here instead of CRYPTO_alloc_ex_data(). + * They do the same thing except that the latter calls CRYPTO_get_ex_data() + * as well - which we must not do without holding the ctx->lock. + */ + if (ossl_crypto_alloc_ex_data_intern(CRYPTO_EX_INDEX_OSSL_LIB_CTX, NULL, + &ctx->data, ctx->dyn_indexes[index])) { + CRYPTO_THREAD_read_lock(ctx->lock); data = CRYPTO_get_ex_data(&ctx->data, ctx->dyn_indexes[index]); + CRYPTO_THREAD_unlock(ctx->lock); + } CRYPTO_THREAD_unlock(ctx->index_locks[index]); diff --git a/crypto/ex_data.c b/crypto/ex_data.c index 5de99b4..0d87ea7 100644 --- a/crypto/ex_data.c +++ b/crypto/ex_data.c @@ -392,16 +392,23 @@ void CRYPTO_free_ex_data(int class_index, void *obj, CRYPTO_EX_DATA *ad) int CRYPTO_alloc_ex_data(int class_index, void *obj, CRYPTO_EX_DATA *ad, int idx) { - EX_CALLBACK *f; - EX_CALLBACKS *ip; void *curval; - OSSL_EX_DATA_GLOBAL *global; curval = CRYPTO_get_ex_data(ad, idx); /* Already there, no need to allocate */ if (curval != NULL) return 1; + return ossl_crypto_alloc_ex_data_intern(class_index, obj, ad, idx); +} + +int ossl_crypto_alloc_ex_data_intern(int class_index, void *obj, + CRYPTO_EX_DATA *ad, int idx) +{ + EX_CALLBACK *f; + EX_CALLBACKS *ip; + OSSL_EX_DATA_GLOBAL *global; + global = ossl_lib_ctx_get_ex_data_global(ad->ctx); if (global == NULL) return 0; |