diff options
author | Matt Caswell <matt@openssl.org> | 2023-05-10 16:27:03 +0100 |
---|---|---|
committer | Tomas Mraz <tomas@openssl.org> | 2023-05-29 16:08:34 +0200 |
commit | 36424806d699233b9a90a3a97fff3011828e2548 (patch) | |
tree | 72ee30300c18d7fddde134a2792acad601c91124 /crypto | |
parent | 95a8aa6dc0e283b1560dd3258d2e9115c02659b1 (diff) |
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 <hlandau@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20932)
Diffstat (limited to 'crypto')
-rw-r--r-- | crypto/evp/keymgmt_lib.c | 8 | ||||
-rw-r--r-- | crypto/evp/p_lib.c | 4 |
2 files changed, 4 insertions, 8 deletions
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; |