From: Neil Horman Date: Wed, 29 Apr 2026 22:39:08 +0000 (-0400) Subject: Fix persniketyness in tsan X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8f9f0d2da0669a49fcb943eb433af1368461258a;p=thirdparty%2Fopenssl.git Fix persniketyness in tsan TSAN seems to be having a problem with atomic_load_ptr and atomic_store_ptr. Both are, by default, __ATOMIC_RELAXED operations. According to the tsan docs, it flags these operations as a race because, while they are indivisible, they create no happens-before constraint, meaning they can be reordered. An exemplar race that is reported is: WARNING: ThreadSanitizer: data race (pid=2139404) Read of size 4 at 0x723400002308 by thread T39: #0 EVP_MD_up_ref crypto/evp/digest.c:995 (threadstest+0x45032d) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #1 evp_md_up_ref crypto/evp/digest.c:974 (threadstest+0x450242) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #2 ossl_method_up_ref crypto/property/property.c:201 (threadstest+0x4b7a55) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #3 ossl_method_store_cache_get_locked crypto/property/property.c:941 (threadstest+0x4b9922) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #4 ossl_method_store_cache_get crypto/property/property.c:974 (threadstest+0x4b9a47) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #5 inner_evp_generic_fetch crypto/evp/evp_fetch.c:314 (threadstest+0x458186) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #6 evp_generic_fetch crypto/evp/evp_fetch.c:404 (threadstest+0x4586dc) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #7 EVP_MD_fetch crypto/evp/digest.c:985 (threadstest+0x4502d7) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #8 derive_kdk crypto/rsa/rsa_ossl.c:472 (threadstest+0x4cf738) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #9 rsa_ossl_private_decrypt crypto/rsa/rsa_ossl.c:646 (threadstest+0x4d0174) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #10 RSA_private_decrypt crypto/rsa/rsa_crpt.c:48 (threadstest+0x4c6971) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #11 rsa_decrypt providers/implementations/asymciphers/rsa_enc.c:321 (threadstest+0x51cab7) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #12 EVP_PKEY_decrypt crypto/evp/asymcipher.c:280 (threadstest+0x44a9ca) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #13 thread_shared_evp_pkey test/threadstest.c:966 (threadstest+0x404be7) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #14 thread_run test/threadstest.h:67 (threadstest+0x40132d) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) Previous write of size 8 at 0x723400002308 by main thread (mutexes: write M0): #0 memset (libtsan.so.2+0x4c1eb) (BuildId: 40906101a3a1e1f1ececafafda314aee009d688a) #1 CRYPTO_zalloc crypto/mem.c:228 (threadstest+0x48679d) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #2 evp_md_new crypto/evp/digest.c:758 (threadstest+0x44f35e) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #3 evp_md_from_algorithm crypto/evp/digest.c:839 (threadstest+0x44f885) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #4 construct_evp_method crypto/evp/evp_fetch.c:230 (threadstest+0x457ec9) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #5 ossl_method_construct_this crypto/core_fetch.c:110 (threadstest+0x4801bf) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #6 algorithm_do_map crypto/core_algorithm.c:77 (threadstest+0x47f7a3) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #7 algorithm_do_this crypto/core_algorithm.c:122 (threadstest+0x47f987) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #8 ossl_provider_doall_activated crypto/provider_core.c:1609 (threadstest+0x49a42a) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #9 ossl_algorithm_do_all crypto/core_algorithm.c:164 (threadstest+0x47fb14) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #10 ossl_method_construct crypto/core_fetch.c:157 (threadstest+0x4803d0) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #11 inner_evp_generic_fetch crypto/evp/evp_fetch.c:333 (threadstest+0x4583a2) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #12 evp_generic_fetch crypto/evp/evp_fetch.c:404 (threadstest+0x4586dc) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #13 EVP_MD_fetch crypto/evp/digest.c:985 (threadstest+0x4502d7) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #14 derive_kdk crypto/rsa/rsa_ossl.c:472 (threadstest+0x4cf738) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #15 rsa_ossl_private_decrypt crypto/rsa/rsa_ossl.c:646 (threadstest+0x4d0174) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #16 RSA_private_decrypt crypto/rsa/rsa_crpt.c:48 (threadstest+0x4c6971) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #17 rsa_decrypt providers/implementations/asymciphers/rsa_enc.c:321 (threadstest+0x51cab7) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) #18 EVP_PKEY_decrypt crypto/evp/asymcipher.c:280 (threadstest+0x44a9ca) (BuildId: f34377d95e3c1d13ab9aa3204d2f1f7840d1c84a) What tsan is saying here is that the memset in evp_md_new may get re-ordered such that the contents of the EVP_MD may still be getting zeroed at the time we have (a) found the EVP_MD in the method store cache, and (b) attempted to do an up_ref on it. This is plainly impossible, especially given that, in order to reach the method store cache, it must be places in the method store algorithm sparse array, which still requires the taking of the method store write lock. But for some reason tsan fails to see the memory fence that creates. It seems the simplest solution to correct this is, if we are running under tsan, use __ATOMIC_ACQUIRE and __ATOMIC_RELEASE on CRYPTO_atomic_[load|store]_ptr to make sure tsan sees the proper memory ordering. Reviewed-by: Saša Nedvědický Reviewed-by: Bob Beck Reviewed-by: Nikola Pajkovsky MergeDate: Tue Jun 9 18:17:19 2026 (Merged from https://github.com/openssl/openssl/pull/31018) --- diff --git a/crypto/threads_pthread.c b/crypto/threads_pthread.c index a2adcb9afe8..7cf820b5467 100644 --- a/crypto/threads_pthread.c +++ b/crypto/threads_pthread.c @@ -53,9 +53,14 @@ #define TSAN_FAKE_LOCK(x) \ __tsan_mutex_pre_lock((x), 0); \ __tsan_mutex_post_lock((x), 0, 0) + +#define TSAN_LOAD_MEM_ORDER __ATOMIC_ACQUIRE +#define TSAN_STORE_MEM_ORDER __ATOMIC_RELEASE #else #define TSAN_FAKE_UNLOCK(x) #define TSAN_FAKE_LOCK(x) +#define TSAN_LOAD_MEM_ORDER __ATOMIC_RELAXED +#define TSAN_STORE_MEM_ORDER __ATOMIC_RELAXED #endif #if defined(__sun) @@ -1245,7 +1250,7 @@ int CRYPTO_atomic_store_int(int *dst, int val, CRYPTO_RWLOCK *lock) int CRYPTO_atomic_load_ptr(void **ptr, void **ret, CRYPTO_RWLOCK *lock) { #if defined(__GNUC__) && defined(__ATOMIC_RELAXED) && !defined(BROKEN_CLANG_ATOMICS) - *ret = __atomic_load_n(ptr, __ATOMIC_RELAXED); + *ret = __atomic_load_n(ptr, TSAN_LOAD_MEM_ORDER); return 1; #else if (lock == NULL || !CRYPTO_THREAD_read_lock(lock)) @@ -1260,7 +1265,7 @@ int CRYPTO_atomic_load_ptr(void **ptr, void **ret, CRYPTO_RWLOCK *lock) int CRYPTO_atomic_store_ptr(void **dst, void **val, CRYPTO_RWLOCK *lock) { #if defined(__GNUC__) && defined(__ATOMIC_RELAXED) && !defined(BROKEN_CLANG_ATOMICS) - __atomic_store(dst, val, __ATOMIC_RELAXED); + __atomic_store(dst, val, TSAN_STORE_MEM_ORDER); return 1; #else if (lock == NULL || !CRYPTO_THREAD_write_lock(lock))