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)

diff --git a/crypto/threads_none.c b/crypto/threads_none.c
index 47a7c01f26aa274b3e2a4ccc066d0486d1e4db00..e0387650fece80b59dc1b160208588893f7e3842 100644 (file)
--- a/crypto/threads_none.c
+++ b/crypto/threads_none.c
@@ -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;

diff --git a/crypto/threads_pthread.c b/crypto/threads_pthread.c
index 69b68e5226d7a15e64b0c3f3157c0a13a7583d5c..8e411671d9f76c201bbd933849928e0802db9778 100644 (file)
--- a/crypto/threads_pthread.c
+++ b/crypto/threads_pthread.c
@@ -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)

diff --git a/crypto/threads_win.c b/crypto/threads_win.c
index 6bcbaea10fca762cdd69eae9e208812fa1aa4c06..ea72670f2230d49449f9021e1b61a664306fecaa 100644 (file)
--- a/crypto/threads_win.c
+++ b/crypto/threads_win.c
@@ -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))

diff --git a/doc/man3/CRYPTO_THREAD_run_once.pod b/doc/man3/CRYPTO_THREAD_run_once.pod
index 470b741c109a1ded4a567fc07c6b5ff008b7e277..3658a39278cd0c30016e2c9f5c3b5d6640aa5e9c 100644 (file)
--- a/doc/man3/CRYPTO_THREAD_run_once.pod
+++ b/doc/man3/CRYPTO_THREAD_run_once.pod
@@ -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.
 

diff --git a/include/openssl/crypto.h.in b/include/openssl/crypto.h.in
index b2d691b90f0ec2ffea36b7e4a5eb75816afc3853..220a2df354352cb751133f52a30c2bfd0c2859de 100644 (file)
--- a/include/openssl/crypto.h.in
+++ b/include/openssl/crypto.h.in
@@ -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

diff --git a/util/libcrypto.num b/util/libcrypto.num
index 75813690dcefe63e80026728ade0bb17bedff0be..0c83e9598ee29b410dd2c5de67d608b37ac5642a 100644 (file)
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -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: