This should fix a lock-order-inversion.

[bernd-edlinger]

Fixes #12869

Checklist

• documentation is added or updated
• tests are added or updated

[bernd-edlinger]

This is unfortunately not fixed:

WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=21190)
  Cycle in lock order graph: M38 (0x7b10000005c0) => M61 (0x7b1000000800) => M66 (0x7b1000000780) => M38

  Mutex M61 acquired here while holding mutex M38 in main thread:
    #0 pthread_rwlock_wrlock ../../../../gcc-trunk/libsanitizer/tsan/tsan_interceptors_posix.cpp:1368 (libtsan.so.0+0x4198e)
    #1 CRYPTO_THREAD_write_lock crypto/threads_pthread.c:86 (libcrypto.so.3+0x275a8a)
    #2 ossl_provider_find crypto/provider_core.c:229 (libcrypto.so.3+0x27281c)
    #3 provider_conf_load crypto/provider_conf.c:118 (libcrypto.so.3+0x271b01)
    #4 provider_conf_init crypto/provider_conf.c:169 (libcrypto.so.3+0x271b01)
    #5 module_init crypto/conf/conf_mod.c:378 (libcrypto.so.3+0x15fb4a)
    #6 module_run crypto/conf/conf_mod.c:237 (libcrypto.so.3+0x15fb4a)
    #7 CONF_modules_load crypto/conf/conf_mod.c:136 (libcrypto.so.3+0x15fb4a)
    #8 CONF_modules_load crypto/conf/conf_mod.c:89 (libcrypto.so.3+0x15fb4a)
    #9 CONF_modules_load_file_ex crypto/conf/conf_mod.c:179 (libcrypto.so.3+0x160769)
    #10 CONF_modules_load_file crypto/conf/conf_mod.c:200 (libcrypto.so.3+0x1608a8)
    #11 openssl_config_int crypto/conf/conf_sap.c:63 (libcrypto.so.3+0x160ba1)
    #12 ossl_init_config crypto/init.c:237 (libcrypto.so.3+0x265312)
    #13 ossl_init_config_ossl_ crypto/init.c:235 (libcrypto.so.3+0x265312)
    #14 pthread_once ../../../../gcc-trunk/libsanitizer/tsan/tsan_interceptors_posix.cpp:1449 (libtsan.so.0+0x447a0)
    #15 CRYPTO_THREAD_run_once crypto/threads_pthread.c:126 (libcrypto.so.3+0x275b2e)
    #16 OPENSSL_init_crypto crypto/init.c:544 (libcrypto.so.3+0x265adb)
    #17 OPENSSL_init_ssl ssl/ssl_init.c:126 (libssl.so.3+0x38a93)
    #18 apps_startup apps/openssl.c:66 (openssl+0x426980)
    #19 main apps/openssl.c:260 (openssl+0x426980)

  Mutex M38 previously acquired by the same thread here:
    #0 pthread_rwlock_wrlock ../../../../gcc-trunk/libsanitizer/tsan/tsan_interceptors_posix.cpp:1368 (libtsan.so.0+0x4198e)
    #1 CRYPTO_THREAD_write_lock crypto/threads_pthread.c:86 (libcrypto.so.3+0x275a8a)
    #2 OPENSSL_init_crypto crypto/init.c:542 (libcrypto.so.3+0x265ab5)
    #3 OPENSSL_init_ssl ssl/ssl_init.c:126 (libssl.so.3+0x38a93)
    #4 apps_startup apps/openssl.c:66 (openssl+0x426980)
    #5 main apps/openssl.c:260 (openssl+0x426980)

  Mutex M66 acquired here while holding mutex M61 in main thread:
    #0 pthread_rwlock_wrlock ../../../../gcc-trunk/libsanitizer/tsan/tsan_interceptors_posix.cpp:1368 (libtsan.so.0+0x4198e)
    #1 CRYPTO_THREAD_write_lock crypto/threads_pthread.c:86 (libcrypto.so.3+0x275a8a)
    #2 ossl_namemap_add_names crypto/core_namemap.c:280 (libcrypto.so.3+0x261d4a)
    #3 construct_evp_method crypto/evp/evp_fetch.c:191 (libcrypto.so.3+0x22df67)
    #4 ossl_method_construct_this crypto/core_fetch.c:70 (libcrypto.so.3+0x2613cb)
    #5 algorithm_do_this crypto/core_algorithm.c:65 (libcrypto.so.3+0x261241)
    #6 provider_forall_loaded crypto/provider_core.c:674 (libcrypto.so.3+0x2733ff)
    #7 ossl_provider_forall_loaded crypto/provider_core.c:752 (libcrypto.so.3+0x2733ff)
    #8 ossl_algorithm_do_all crypto/core_algorithm.c:109 (libcrypto.so.3+0x261369)
    #9 ossl_method_construct crypto/core_fetch.c:124 (libcrypto.so.3+0x261836)
    #10 inner_evp_generic_fetch crypto/evp/evp_fetch.c:316 (libcrypto.so.3+0x22e65f)
    #11 evp_generic_fetch crypto/evp/evp_fetch.c:363 (libcrypto.so.3+0x22e65f)
    #12 EVP_KEYMGMT_fetch crypto/evp/keymgmt_meth.c:213 (libcrypto.so.3+0x23b1a3)
    #13 int_ctx_new crypto/evp/pmeth_lib.c:262 (libcrypto.so.3+0x24b3fe)
    #14 EVP_PKEY_CTX_new_id crypto/evp/pmeth_lib.c:445 (libcrypto.so.3+0x24b3fe)
    #15 init_gen_str apps/genpkey.c:319 (openssl+0x447e28)
    #16 genpkey_main apps/genpkey.c:111 (openssl+0x4483ff)
    #17 do_cmd apps/openssl.c:411 (openssl+0x450e34)
    #18 main apps/openssl.c:293 (openssl+0x426bfe)

  Mutex M61 previously acquired by the same thread here:
    #0 pthread_rwlock_rdlock ../../../../gcc-trunk/libsanitizer/tsan/tsan_interceptors_posix.cpp:1338 (libtsan.so.0+0x3ef9e)
    #1 CRYPTO_THREAD_read_lock crypto/threads_pthread.c:73 (libcrypto.so.3+0x275a5a)
    #2 ossl_provider_forall_loaded crypto/provider_core.c:745 (libcrypto.so.3+0x273397)
    #3 ossl_algorithm_do_all crypto/core_algorithm.c:109 (libcrypto.so.3+0x261369)
    #4 ossl_method_construct crypto/core_fetch.c:124 (libcrypto.so.3+0x261836)
    #5 inner_evp_generic_fetch crypto/evp/evp_fetch.c:316 (libcrypto.so.3+0x22e65f)
    #6 evp_generic_fetch crypto/evp/evp_fetch.c:363 (libcrypto.so.3+0x22e65f)
    #7 EVP_KEYMGMT_fetch crypto/evp/keymgmt_meth.c:213 (libcrypto.so.3+0x23b1a3)
    #8 int_ctx_new crypto/evp/pmeth_lib.c:262 (libcrypto.so.3+0x24b3fe)
    #9 EVP_PKEY_CTX_new_id crypto/evp/pmeth_lib.c:445 (libcrypto.so.3+0x24b3fe)
    #10 init_gen_str apps/genpkey.c:319 (openssl+0x447e28)
    #11 genpkey_main apps/genpkey.c:111 (openssl+0x4483ff)
    #12 do_cmd apps/openssl.c:411 (openssl+0x450e34)
    #13 main apps/openssl.c:293 (openssl+0x426bfe)

  Mutex M38 acquired here while holding mutex M66 in main thread:
    #0 pthread_rwlock_wrlock ../../../../gcc-trunk/libsanitizer/tsan/tsan_interceptors_posix.cpp:1368 (libtsan.so.0+0x4198e)
    #1 CRYPTO_THREAD_write_lock crypto/threads_pthread.c:86 (libcrypto.so.3+0x275a8a)
    #2 OPENSSL_init_crypto crypto/init.c:542 (libcrypto.so.3+0x265ab5)
    #3 OBJ_sn2nid crypto/objects/obj_dat.c:588 (libcrypto.so.3+0x28924b)
    #4 evp_pkey_name2type crypto/evp/p_lib.c:1015 (libcrypto.so.3+0x241d1c)
    #5 help_get_legacy_alg_type_from_keymgmt crypto/evp/pmeth_lib.c:162 (libcrypto.so.3+0x249f17)
    #6 do_name crypto/core_namemap.c:126 (libcrypto.so.3+0x261a38)
    #7 do_name crypto/core_namemap.c:123 (libcrypto.so.3+0x261a38)
    #8 doall_util_fn crypto/lhash/lhash.c:205 (libcrypto.so.3+0x25fb00)
    #9 OPENSSL_LH_doall_arg crypto/lhash/lhash.c:220 (libcrypto.so.3+0x25fb00)
    #10 lh_NAMENUM_ENTRY_doall_DOALL_NAMES_DATA crypto/core_namemap.c:129 (libcrypto.so.3+0x261c4f)
    #11 ossl_namemap_doall_names crypto/core_namemap.c:141 (libcrypto.so.3+0x261c4f)
    #12 evp_names_do_all crypto/evp/evp_fetch.c:554 (libcrypto.so.3+0x22f2cf)
    #13 EVP_KEYMGMT_names_do_all crypto/evp/keymgmt_meth.c:277 (libcrypto.so.3+0x23b378)
    #14 get_legacy_alg_type_from_keymgmt crypto/evp/pmeth_lib.c:169 (libcrypto.so.3+0x24b431)
    #15 int_ctx_new crypto/evp/pmeth_lib.c:277 (libcrypto.so.3+0x24b431)
    #16 EVP_PKEY_CTX_new_id crypto/evp/pmeth_lib.c:445 (libcrypto.so.3+0x24b431)
    #17 init_gen_str apps/genpkey.c:319 (openssl+0x447e28)
    #18 genpkey_main apps/genpkey.c:111 (openssl+0x4483ff)
    #19 do_cmd apps/openssl.c:411 (openssl+0x450e34)
    #20 main apps/openssl.c:293 (openssl+0x426bfe)

  Mutex M66 previously acquired by the same thread here:
    #0 pthread_rwlock_rdlock ../../../../gcc-trunk/libsanitizer/tsan/tsan_interceptors_posix.cpp:1338 (libtsan.so.0+0x3ef9e)
    #1 CRYPTO_THREAD_read_lock crypto/threads_pthread.c:73 (libcrypto.so.3+0x275a5a)
    #2 ossl_namemap_doall_names crypto/core_namemap.c:140 (libcrypto.so.3+0x261c33)
    #3 evp_names_do_all crypto/evp/evp_fetch.c:554 (libcrypto.so.3+0x22f2cf)
    #4 EVP_KEYMGMT_names_do_all crypto/evp/keymgmt_meth.c:277 (libcrypto.so.3+0x23b378)
    #5 get_legacy_alg_type_from_keymgmt crypto/evp/pmeth_lib.c:169 (libcrypto.so.3+0x24b431)
    #6 int_ctx_new crypto/evp/pmeth_lib.c:277 (libcrypto.so.3+0x24b431)
    #7 EVP_PKEY_CTX_new_id crypto/evp/pmeth_lib.c:445 (libcrypto.so.3+0x24b431)
    #8 init_gen_str apps/genpkey.c:319 (openssl+0x447e28)
    #9 genpkey_main apps/genpkey.c:111 (openssl+0x4483ff)
    #10 do_cmd apps/openssl.c:411 (openssl+0x450e34)
    #11 main apps/openssl.c:293 (openssl+0x426bfe)

SUMMARY: ThreadSanitizer: lock-order-inversion (potential deadlock) crypto/threads_pthread.c:86 in CRYPTO_THREAD_write_lock

[mspncp]

It looks good at first sight, at least the lock order inversion seems to be fixed without breaking #12565. However, in view of all the previous discussion, in particular #13288 (comment) I would appreciate to see a little bit more explaining text in the commit message. (This is a change request.)

Also, to be sure I would like to wait for @mattcaswell's independent review before considering this pull request ready-to-merge.

[mattcaswell]

LGTM

[openssl-machine]

This pull request is ready to merge

[bernd-edlinger]

Pushed to master 07af944