From 36424806d699233b9a90a3a97fff3011828e2548 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 10 May 2023 16:27:03 +0100 Subject: Don't take a write lock when freeing an EVP_PKEY When freeing the last reference to an EVP_PKEY there is no point in taking the lock for the key. It is the last reference and is being freed so must only be being used by a single thread. This should not have been the source of any contention so its unclear to what extent this will improve performance. But we should not be locking when we don't need to. Partially fixes #20286 Reviewed-by: Hugo Landau Reviewed-by: Paul Dale Reviewed-by: Richard Levitte Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/20932) --- crypto/evp/keymgmt_lib.c | 8 ++------ crypto/evp/p_lib.c | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) (limited to 'crypto/evp') diff --git a/crypto/evp/keymgmt_lib.c b/crypto/evp/keymgmt_lib.c index 2d0238ee27..47c802bfb4 100644 --- a/crypto/evp/keymgmt_lib.c +++ b/crypto/evp/keymgmt_lib.c @@ -194,7 +194,7 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt, * operation cache. In that case, we know that |i| is zero. */ if (pk->dirty_cnt != pk->dirty_cnt_copy) - evp_keymgmt_util_clear_operation_cache(pk, 0); + evp_keymgmt_util_clear_operation_cache(pk); /* Add the new export to the operation cache */ if (!evp_keymgmt_util_cache_keydata(pk, keymgmt, import_data.keydata, @@ -219,15 +219,11 @@ static void op_cache_free(OP_CACHE_ELEM *e) OPENSSL_free(e); } -int evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk, int locking) +int evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk) { if (pk != NULL) { - if (locking && pk->lock != NULL && !CRYPTO_THREAD_write_lock(pk->lock)) - return 0; sk_OP_CACHE_ELEM_pop_free(pk->operation_cache, op_cache_free); pk->operation_cache = NULL; - if (locking && pk->lock != NULL) - CRYPTO_THREAD_unlock(pk->lock); } return 1; diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c index fa51304c97..d0a7fde1e6 100644 --- a/crypto/evp/p_lib.c +++ b/crypto/evp/p_lib.c @@ -1757,7 +1757,7 @@ void evp_pkey_free_legacy(EVP_PKEY *x) static void evp_pkey_free_it(EVP_PKEY *x) { /* internal function; x is never NULL */ - evp_keymgmt_util_clear_operation_cache(x, 1); + evp_keymgmt_util_clear_operation_cache(x); #ifndef FIPS_MODULE evp_pkey_free_legacy(x); #endif @@ -1936,7 +1936,7 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx, if (!CRYPTO_THREAD_write_lock(pk->lock)) goto end; if (pk->ameth->dirty_cnt(pk) != pk->dirty_cnt_copy - && !evp_keymgmt_util_clear_operation_cache(pk, 0)) { + && !evp_keymgmt_util_clear_operation_cache(pk)) { CRYPTO_THREAD_unlock(pk->lock); evp_keymgmt_freedata(tmp_keymgmt, keydata); keydata = NULL; -- cgit v1.2.3