Add CRYPTO_atomic_store api

Generally we can get away with just using CRYPTO_atomic_load to do
stores by reversing the source and target variables, but doing so
creates a problem for the thread sanitizer as CRYPTO_atomic_load hard
codes an __ATOMIC_ACQUIRE constraint, which confuses tsan into thinking
that loads and stores aren't properly ordered, leading to RAW/WAR
hazzards getting reported.  Instead create a CRYPTO_atomic_store api
that is identical to the load variant, save for the fact that the value
is a unit64_t rather than a pointer that gets stored using an
__ATOMIC_RELEASE constraint, satisfying tsan.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23671)
This commit is contained in:
Neil Horman 2024-03-08 11:58:07 -05:00 committed by Pauli
parent f39a862818
commit 7e45ac6891
6 changed files with 56 additions and 1 deletions

View File

@ -226,6 +226,13 @@ int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock)
return 1;
}
int CRYPTO_atomic_store(uint64_t *dst, uint64_t val, CRYPTO_RWLOCK *lock)
{
*dst = val;
return 1;
}
int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock)
{
*ret = *val;

View File

@ -919,6 +919,29 @@ int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock)
return 1;
}
int CRYPTO_atomic_store(uint64_t *dst, uint64_t val, CRYPTO_RWLOCK *lock)
{
# if defined(__GNUC__) && defined(__ATOMIC_ACQUIRE) && !defined(BROKEN_CLANG_ATOMICS)
if (__atomic_is_lock_free(sizeof(*dst), dst)) {
__atomic_store(dst, &val, __ATOMIC_RELEASE);
return 1;
}
# elif defined(__sun) && (defined(__SunOS_5_10) || defined(__SunOS_5_11))
/* This will work for all future Solaris versions. */
if (ret != NULL) {
atomic_swap_64(dst, val);
return 1;
}
# endif
if (lock == NULL || !CRYPTO_THREAD_read_lock(lock))
return 0;
*dst = val;
if (!CRYPTO_THREAD_unlock(lock))
return 0;
return 1;
}
int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock)
{
# if defined(__GNUC__) && defined(__ATOMIC_ACQUIRE) && !defined(BROKEN_CLANG_ATOMICS)

View File

@ -593,6 +593,22 @@ int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock)
#endif
}
int CRYPTO_atomic_store(uint64_t *dst, uint64_t val, CRYPTO_RWLOCK *lock)
{
#if (defined(NO_INTERLOCKEDOR64))
if (lock == NULL || !CRYPTO_THREAD_read_lock(lock))
return 0;
*dst = val;
if (!CRYPTO_THREAD_unlock(lock))
return 0;
return 1;
#else
InterlockedExchange64(dst, val);
return 1;
#endif
}
int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock)
{
#if (defined(NO_INTERLOCKEDOR64))

View File

@ -5,7 +5,7 @@
CRYPTO_THREAD_run_once,
CRYPTO_THREAD_lock_new, CRYPTO_THREAD_read_lock, CRYPTO_THREAD_write_lock,
CRYPTO_THREAD_unlock, CRYPTO_THREAD_lock_free,
CRYPTO_atomic_add, CRYPTO_atomic_or, CRYPTO_atomic_load,
CRYPTO_atomic_add, CRYPTO_atomic_or, CRYPTO_atomic_load, CRYPTO_atomic_store,
CRYPTO_atomic_load_int,
OSSL_set_max_threads, OSSL_get_max_threads,
OSSL_get_thread_support_flags, OSSL_THREAD_SUPPORT_FLAG_THREAD_POOL,
@ -28,6 +28,7 @@ OSSL_THREAD_SUPPORT_FLAG_DEFAULT_SPAWN - OpenSSL thread support
int CRYPTO_atomic_or(uint64_t *val, uint64_t op, uint64_t *ret,
CRYPTO_RWLOCK *lock);
int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock);
int CRYPTO_atomic_store(uint64_t *dst, uint64_t val, CRYPTO_RWLOCK *lock);
int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock);
int OSSL_set_max_threads(OSSL_LIB_CTX *ctx, uint64_t max_threads);
@ -112,6 +113,12 @@ NULL, then the function will fail.
=item *
CRYPTO_atomic_store() atomically stores the contents of I<val> into I<*dst>.
I<lock> will be locked, unless atomic operations are supported on the specific
plaform.
=item *
CRYPTO_atomic_load_int() works identically to CRYPTO_atomic_load() but operates
on an I<int> value instead of a I<uint64_t> value.

View File

@ -90,6 +90,7 @@ int CRYPTO_atomic_or(uint64_t *val, uint64_t op, uint64_t *ret,
CRYPTO_RWLOCK *lock);
int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock);
int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock);
int CRYPTO_atomic_store(uint64_t *dst, uint64_t val, CRYPTO_RWLOCK *lock);
/* No longer needed, so this is a no-op */
#define OPENSSL_malloc_init() while(0) continue

View File

@ -5548,3 +5548,4 @@ X509_STORE_get1_objects 5675 3_3_0 EXIST::FUNCTION:
OPENSSL_LH_set_thunks 5676 3_3_0 EXIST::FUNCTION:
OPENSSL_LH_doall_arg_thunk 5677 3_3_0 EXIST::FUNCTION:
OSSL_HTTP_REQ_CTX_set_max_response_hdr_lines 5678 3_3_0 EXIST::FUNCTION:HTTP
CRYPTO_atomic_store ? 3_4_0 EXIST::FUNCTION: