Make thread sanitizer cope with rcu locks

This is unfortunate, but seems necessecary

tsan in gcc/clang tracks data races by recording memory references made
while various locks are held.  If it finds that a given address is
read/written while under lock (or under no locks without the use of
atomics), it issues a warning

this creates a specific problem for rcu, because on the write side of a
critical section, we write data under the protection of a lock, but by
definition the read side has no lock, and so rcu warns us about it,
which is really a false positive, because we know that, even if a
pointer changes its value, the data it points to will be valid.

The best way to fix it, short of implementing tsan hooks for rcu locks
in any thread sanitizer in the field, is to 'fake it'.  If thread
sanitization is activated, then in ossl_rcu_write_[lock|unlock] we add
annotations to make the sanitizer think that, after the write lock is
taken, that we immediately unlock it, and lock it right before we unlock
it again.  In this way tsan thinks there are no locks held while
referencing protected data on the read or write side.

we still need to use atomics to ensure that tsan recognizes that we are
doing atomic accesses safely, but thats ok, and we still get warnings if
we don't do that properly

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_pthread.c b/crypto/threads_pthread.c
index 03ef60d576050db1561c6d80783a2997ac83797d..33e225b9957e33b5064760870d94458db2575bd7 100644 (file)
--- a/crypto/threads_pthread.c
+++ b/crypto/threads_pthread.c
@@ -16,6 +16,24 @@
 #include "internal/rcu.h"
 #include "rcu_internal.h"
 
+#if defined(__clang__) && defined(__has_feature)
+# if __has_feature(thread_sanitizer)
+#  define __SANITIZE_THREAD__
+# endif
+#endif
+
+#if defined(__SANITIZE_THREAD__)
+# include <sanitizer/tsan_interface.h>
+# define TSAN_FAKE_UNLOCK(x)   __tsan_mutex_pre_unlock((x), 0); \
+__tsan_mutex_post_unlock((x), 0)
+
+# define TSAN_FAKE_LOCK(x)  __tsan_mutex_pre_lock((x), 0); \
+__tsan_mutex_post_lock((x), 0, 0)
+#else
+# define TSAN_FAKE_UNLOCK(x)
+# define TSAN_FAKE_LOCK(x)
+#endif
+
 #if defined(__sun)
 # include <atomic.h>
 #endif
@@ -548,10 +566,12 @@ static struct rcu_qp *allocate_new_qp_group(CRYPTO_RCU_LOCK *lock,
 void ossl_rcu_write_lock(CRYPTO_RCU_LOCK *lock)
 {
     pthread_mutex_lock(&lock->write_lock);
+    TSAN_FAKE_UNLOCK(&lock->write_lock);
 }
 
 void ossl_rcu_write_unlock(CRYPTO_RCU_LOCK *lock)
 {
+    TSAN_FAKE_LOCK(&lock->write_lock);
     pthread_mutex_unlock(&lock->write_lock);
 }
 
@@ -565,8 +585,10 @@ void ossl_synchronize_rcu(CRYPTO_RCU_LOCK *lock)
      * __ATOMIC_ACQ_REL is used here to ensure that we get any prior published
      * writes before we read, and publish our write immediately
      */
-    cb_items = ATOMIC_EXCHANGE_N(prcu_cb_item, &lock->cb_items, NULL,
-                                 __ATOMIC_ACQ_REL);
+    pthread_mutex_lock(&lock->write_lock);
+    cb_items = lock->cb_items;
+    lock->cb_items = NULL;
+    pthread_mutex_unlock(&lock->write_lock);
 
     qp = update_qp(lock);