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/context.c | |
parent | b233ea82765e80038e4884564153f9c8543d9396 (diff) |
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/context.c')
-rw-r--r-- | crypto/context.c | 22 |
1 files changed, 18 insertions, 4 deletions
diff --git a/crypto/context.c b/crypto/context.c index 7efe475b70..5a46921778 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]); |