Modify ENGINE_pkey_asn1_find_str() to use a read lock instead of a write

ENGINE_pkey_asn1_find_str() does not make any modifications to fields
controlled by the global_engine_lock. The only change made is the struct_ref
field which is controlled separately. Therefore we can afford to only take
a read lock. This also impacts EVP_PKEY_asn1_find_str().

This lock ends up being obtained indirectly from numerous public API
functions including EVP_PKEY_key_gen(), EVP_PKEY_new_raw_public_key_ex(),
EVP_PKEY_copy_parameters() etc. This occurs even if no engines are actually
in use.

Some tests showed this lock being obtained 6 times after a "warmed up"
s_server instance with default configuration processed a handshake from a
default s_client. When processing a resumption handshake from s_client it
was obtained 8 times.

Partially fixes #20286

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20950)

diff --git a/crypto/engine/tb_asnmth.c b/crypto/engine/tb_asnmth.c
index fac038356bf4bcfb6dee834ea011348f80c114ce..c72bf9d22fe6f073e7c18667ead4f714249486c4 100644 (file)
--- a/crypto/engine/tb_asnmth.c
+++ b/crypto/engine/tb_asnmth.c
@@ -201,7 +201,7 @@ const EVP_PKEY_ASN1_METHOD *ENGINE_pkey_asn1_find_str(ENGINE **pe,
         return NULL;
     }
 
-    if (!CRYPTO_THREAD_write_lock(global_engine_lock))
+    if (!CRYPTO_THREAD_read_lock(global_engine_lock))
         return NULL;
     engine_table_doall(pkey_asn1_meth_table, look_str_cb, &fstr);
     /* If found obtain a structural reference to engine */