From 6d15357aeb893c6e8b4c7a8188c18f4db54c0612 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 11 May 2023 11:25:07 +0100 Subject: [PATCH] When we're just reading EX_CALLBACK data just get a read lock The crypto_ex_data code was always obtaining a write lock in all functions regardless of whether we were only reading EX_CALLBACK data or actually changing it. Changes to the EX_CALLBACK data are rare, with many reads so we should change to a read lock where we can. We hit this every time we create or free any object that can have ex_data associated with it (e.g. BIOs, SSL, etc) Partially fixes #20286 Reviewed-by: Tomas Mraz Reviewed-by: Hugo Landau Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/20943) --- crypto/ex_data.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/crypto/ex_data.c b/crypto/ex_data.c index 26c225001c..48321febe4 100644 --- a/crypto/ex_data.c +++ b/crypto/ex_data.c @@ -26,8 +26,10 @@ int ossl_do_ex_data_init(OSSL_LIB_CTX *ctx) * Return the EX_CALLBACKS from the |ex_data| array that corresponds to * a given class. On success, *holds the lock.* * The |global| parameter is assumed to be non null (checked by the caller). + * If |read| is 1 then a read lock is obtained. Otherwise it is a write lock. */ -static EX_CALLBACKS *get_and_lock(OSSL_EX_DATA_GLOBAL *global, int class_index) +static EX_CALLBACKS *get_and_lock(OSSL_EX_DATA_GLOBAL *global, int class_index, + int read) { EX_CALLBACKS *ip; @@ -44,8 +46,14 @@ static EX_CALLBACKS *get_and_lock(OSSL_EX_DATA_GLOBAL *global, int class_index) return NULL; } - if (!CRYPTO_THREAD_write_lock(global->ex_data_lock)) - return NULL; + if (read) { + if (!CRYPTO_THREAD_read_lock(global->ex_data_lock)) + return NULL; + } else { + if (!CRYPTO_THREAD_write_lock(global->ex_data_lock)) + return NULL; + } + ip = &global->ex_data[class_index]; return ip; } @@ -112,7 +120,7 @@ int ossl_crypto_free_ex_index_ex(OSSL_LIB_CTX *ctx, int class_index, int idx) if (global == NULL) return 0; - ip = get_and_lock(global, class_index); + ip = get_and_lock(global, class_index, 0); if (ip == NULL) return 0; @@ -153,7 +161,7 @@ int ossl_crypto_get_ex_new_index_ex(OSSL_LIB_CTX *ctx, int class_index, if (global == NULL) return -1; - ip = get_and_lock(global, class_index); + ip = get_and_lock(global, class_index, 0); if (ip == NULL) return -1; @@ -219,7 +227,7 @@ int ossl_crypto_new_ex_data_ex(OSSL_LIB_CTX *ctx, int class_index, void *obj, if (global == NULL) return 0; - ip = get_and_lock(global, class_index); + ip = get_and_lock(global, class_index, 1); if (ip == NULL) return 0; @@ -280,7 +288,7 @@ int CRYPTO_dup_ex_data(int class_index, CRYPTO_EX_DATA *to, if (global == NULL) return 0; - ip = get_and_lock(global, class_index); + ip = get_and_lock(global, class_index, 1); if (ip == NULL) return 0; @@ -367,7 +375,7 @@ void CRYPTO_free_ex_data(int class_index, void *obj, CRYPTO_EX_DATA *ad) if (global == NULL) goto err; - ip = get_and_lock(global, class_index); + ip = get_and_lock(global, class_index, 1); if (ip == NULL) goto err; @@ -434,7 +442,7 @@ int ossl_crypto_alloc_ex_data_intern(int class_index, void *obj, if (global == NULL) return 0; - ip = get_and_lock(global, class_index); + ip = get_and_lock(global, class_index, 1); if (ip == NULL) return 0; f = sk_EX_CALLBACK_value(ip->meth, idx);