[openssl-commits] [openssl] master update

Andy Polyakov appro at openssl.org
Tue Aug 7 07:10:08 UTC 2018


The branch master has been updated
       via  8f15498563658726a7c2bce7abcf01bea08515de (commit)
       via  e519d6b563d95d630723784a5737ebe5ef74e4f3 (commit)
       via  d1f8b74c584d55a3c7f8f88d997ad69b67076c77 (commit)
       via  f21b5b64cbbc279ef31389e6ae312690575187da (commit)
       via  0da7358b0757fa35f2c3a8f51fa036466ae50fd7 (commit)
       via  9ef9088c1585e13b9727796f15f77da64dbbe623 (commit)
       via  cab76c0f6482df5140efa2ca93c9e2d972fcd9b0 (commit)
       via  ede3e6653c1127e852493655737327170567a453 (commit)
      from  8839324450b569a6253e0dd237ee3e417ef17771 (commit)


- Log -----------------------------------------------------------------
commit 8f15498563658726a7c2bce7abcf01bea08515de
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Aug 3 10:46:03 2018 +0200

    crypto/mem.c: switch to tsan_assist.h in CRYPTO_MDEBUG.
    
    Rationale is that it wasn't providing accurate statistics anyway.
    For statistics to be accurate CRYPTO_get_alloc_counts should acquire
    a lock and lock-free additions should not be an option.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6786)

commit e519d6b563d95d630723784a5737ebe5ef74e4f3
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Aug 3 10:20:59 2018 +0200

    engine/eng_lib.c: remove redundant #ifdef.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6786)

commit d1f8b74c584d55a3c7f8f88d997ad69b67076c77
Author: Andy Polyakov <appro at openssl.org>
Date:   Sun Jul 29 15:21:38 2018 +0200

    man3/OPENSSL_LH_COMPFUNC.pod: clarifications and updates.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6786)

commit f21b5b64cbbc279ef31389e6ae312690575187da
Author: Andy Polyakov <appro at openssl.org>
Date:   Sun Jul 29 14:37:17 2018 +0200

    x509v3/v3_purp.c: re-implement lock-free check for extensions cache validity.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6786)

commit 0da7358b0757fa35f2c3a8f51fa036466ae50fd7
Author: Andy Polyakov <appro at openssl.org>
Date:   Sun Jul 29 14:13:32 2018 +0200

    x509v3/v3_purp.c: resolve Thread Sanitizer nit.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6786)

commit 9ef9088c1585e13b9727796f15f77da64dbbe623
Author: Andy Polyakov <appro at openssl.org>
Date:   Sun Jul 29 14:12:53 2018 +0200

    ssl/*: switch to switch to Thread-Sanitizer-friendly primitives.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6786)

commit cab76c0f6482df5140efa2ca93c9e2d972fcd9b0
Author: Andy Polyakov <appro at openssl.org>
Date:   Sun Jul 29 14:11:49 2018 +0200

    lhash/lhash.c: switch to Thread-Sanitizer-friendly primitives.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6786)

commit ede3e6653c1127e852493655737327170567a453
Author: Andy Polyakov <appro at openssl.org>
Date:   Sun Jul 29 14:10:20 2018 +0200

    Add internal/tsan_assist.h.
    
    Goal here is to facilitate writing "thread-opportunistic" code that
    withstands Thread Sanitizer's scrutiny. "Thread-opportunistic" is when
    exact result is not required, e.g. some statistics, or execution flow
    doesn't have to be unambiguous.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6786)

-----------------------------------------------------------------------

Summary of changes:
 crypto/engine/eng_lib.c            |  6 +--
 crypto/include/internal/x509_int.h |  1 +
 crypto/lhash/lhash.c               | 14 ++++---
 crypto/lhash/lhash_lcl.h           | 12 +++---
 crypto/mem.c                       | 18 ++++----
 crypto/x509v3/v3_purp.c            | 11 ++++-
 doc/man3/OPENSSL_LH_COMPFUNC.pod   | 25 ++++--------
 include/internal/tsan_assist.h     | 84 ++++++++++++++++++++++++++++++++++++++
 ssl/ssl_lib.c                      | 41 ++++++-------------
 ssl/ssl_locl.h                     | 33 ++++++++-------
 ssl/ssl_sess.c                     | 20 ++++-----
 ssl/statem/extensions.c            |  8 ++--
 ssl/statem/statem_clnt.c           |  4 +-
 ssl/statem/statem_lib.c            | 23 ++++-------
 14 files changed, 177 insertions(+), 123 deletions(-)
 create mode 100644 include/internal/tsan_assist.h

diff --git a/crypto/engine/eng_lib.c b/crypto/engine/eng_lib.c
index 9028319..3ef3aae 100644
--- a/crypto/engine/eng_lib.c
+++ b/crypto/engine/eng_lib.c
@@ -75,14 +75,10 @@ int engine_free_util(ENGINE *e, int not_locked)
 
     if (e == NULL)
         return 1;
-#ifdef HAVE_ATOMICS
-    CRYPTO_DOWN_REF(&e->struct_ref, &i, global_engine_lock);
-#else
     if (not_locked)
-        CRYPTO_atomic_add(&e->struct_ref, -1, &i, global_engine_lock);
+        CRYPTO_DOWN_REF(&e->struct_ref, &i, global_engine_lock);
     else
         i = --e->struct_ref;
-#endif
     engine_ref_debug(e, 0, -1);
     if (i > 0)
         return 1;
diff --git a/crypto/include/internal/x509_int.h b/crypto/include/internal/x509_int.h
index 124cc53..1638de4 100644
--- a/crypto/include/internal/x509_int.h
+++ b/crypto/include/internal/x509_int.h
@@ -182,6 +182,7 @@ struct x509_st {
     unsigned char sha1_hash[SHA_DIGEST_LENGTH];
     X509_CERT_AUX *aux;
     CRYPTO_RWLOCK *lock;
+    volatile int ex_cached;
 } /* X509 */ ;
 
 /*
diff --git a/crypto/lhash/lhash.c b/crypto/lhash/lhash.c
index dca5007..f7ac9d0 100644
--- a/crypto/lhash/lhash.c
+++ b/crypto/lhash/lhash.c
@@ -157,16 +157,18 @@ void *OPENSSL_LH_retrieve(OPENSSL_LHASH *lh, const void *data)
     OPENSSL_LH_NODE **rn;
     void *ret;
 
-    lh->error = 0;
+    tsan_store((TSAN_QUALIFIER int *)&lh->error, 0);
+
     rn = getrn(lh, data, &hash);
 
     if (*rn == NULL) {
-        lh->num_retrieve_miss++;
+        tsan_counter(&lh->num_retrieve_miss);
         return NULL;
     } else {
         ret = (*rn)->data;
-        lh->num_retrieve++;
+        tsan_counter(&lh->num_retrieve);
     }
+
     return ret;
 }
 
@@ -296,7 +298,7 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh,
     OPENSSL_LH_COMPFUNC cf;
 
     hash = (*(lh->hash)) (data);
-    lh->num_hash_calls++;
+    tsan_counter(&lh->num_hash_calls);
     *rhash = hash;
 
     nn = hash % lh->pmax;
@@ -306,12 +308,12 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh,
     cf = lh->comp;
     ret = &(lh->b[(int)nn]);
     for (n1 = *ret; n1 != NULL; n1 = n1->next) {
-        lh->num_hash_comps++;
+        tsan_counter(&lh->num_hash_comps);
         if (n1->hash != hash) {
             ret = &(n1->next);
             continue;
         }
-        lh->num_comp_calls++;
+        tsan_counter(&lh->num_comp_calls);
         if (cf(n1->data, data) == 0)
             break;
         ret = &(n1->next);
diff --git a/crypto/lhash/lhash_lcl.h b/crypto/lhash/lhash_lcl.h
index 78691eb..8f79232 100644
--- a/crypto/lhash/lhash_lcl.h
+++ b/crypto/lhash/lhash_lcl.h
@@ -8,6 +8,8 @@
  */
 #include <openssl/crypto.h>
 
+#include "internal/tsan_assist.h"
+
 struct lhash_node_st {
     void *data;
     struct lhash_node_st *next;
@@ -29,14 +31,14 @@ struct lhash_st {
     unsigned long num_expand_reallocs;
     unsigned long num_contracts;
     unsigned long num_contract_reallocs;
-    unsigned long num_hash_calls;
-    unsigned long num_comp_calls;
+    TSAN_QUALIFIER unsigned long num_hash_calls;
+    TSAN_QUALIFIER unsigned long num_comp_calls;
     unsigned long num_insert;
     unsigned long num_replace;
     unsigned long num_delete;
     unsigned long num_no_delete;
-    unsigned long num_retrieve;
-    unsigned long num_retrieve_miss;
-    unsigned long num_hash_comps;
+    TSAN_QUALIFIER unsigned long num_retrieve;
+    TSAN_QUALIFIER unsigned long num_retrieve_miss;
+    TSAN_QUALIFIER unsigned long num_hash_comps;
     int error;
 };
diff --git a/crypto/mem.c b/crypto/mem.c
index 3364467..780053f 100644
--- a/crypto/mem.c
+++ b/crypto/mem.c
@@ -31,13 +31,13 @@ static void (*free_impl)(void *, const char *, int)
     = CRYPTO_free;
 
 #ifndef OPENSSL_NO_CRYPTO_MDEBUG
-static int malloc_count;
-static int realloc_count;
-static int free_count;
-static int dummy;
+# include "internal/tsan_assist.h"
 
-# define INCREMENT(x) CRYPTO_atomic_add(&x, 1, &dummy, memdbg_lock)
-# define GET(ret, val) CRYPTO_atomic_read(&val, ret, memdbg_lock)
+static TSAN_QUALIFIER int malloc_count;
+static TSAN_QUALIFIER int realloc_count;
+static TSAN_QUALIFIER int free_count;
+
+# define INCREMENT(x) tsan_counter(&(x))
 
 static char *md_failstring;
 static long md_count;
@@ -98,11 +98,11 @@ void CRYPTO_get_mem_functions(
 void CRYPTO_get_alloc_counts(int *mcount, int *rcount, int *fcount)
 {
     if (mcount != NULL)
-        GET(mcount, malloc_count);
+        *mcount = tsan_load(&malloc_count);
     if (rcount != NULL)
-        GET(rcount, realloc_count);
+        *rcount = tsan_load(&realloc_count);
     if (fcount != NULL)
-        GET(fcount, free_count);
+        *fcount = tsan_load(&free_count);
 }
 
 /*
diff --git a/crypto/x509v3/v3_purp.c b/crypto/x509v3/v3_purp.c
index b421512..5a535e2 100644
--- a/crypto/x509v3/v3_purp.c
+++ b/crypto/x509v3/v3_purp.c
@@ -13,6 +13,7 @@
 #include <openssl/x509v3.h>
 #include <openssl/x509_vfy.h>
 #include "internal/x509_int.h"
+#include "internal/tsan_assist.h"
 
 static void x509v3_cache_extensions(X509 *x);
 
@@ -351,10 +352,10 @@ static void x509v3_cache_extensions(X509 *x)
     ASN1_BIT_STRING *ns;
     EXTENDED_KEY_USAGE *extusage;
     X509_EXTENSION *ex;
-
     int i;
 
-    if (x->ex_flags & EXFLAG_SET)
+    /* fast lock-free check, see end of the function for details. */
+    if (tsan_load((TSAN_QUALIFIER int *)&x->ex_cached))
         return;
 
     CRYPTO_THREAD_write_lock(x->lock);
@@ -498,6 +499,12 @@ static void x509v3_cache_extensions(X509 *x)
     x509_init_sig_info(x);
     x->ex_flags |= EXFLAG_SET;
     CRYPTO_THREAD_unlock(x->lock);
+    /*
+     * It has to be placed after memory barrier, which is implied by unlock.
+     * Worst thing that can happen is that another thread proceeds to lock
+     * and checks x->ex_flags & EXFLAGS_SET. See beginning of the function.
+     */
+    tsan_store((TSAN_QUALIFIER int *)&x->ex_cached, 1);
 }
 
 /*-
diff --git a/doc/man3/OPENSSL_LH_COMPFUNC.pod b/doc/man3/OPENSSL_LH_COMPFUNC.pod
index ec21c79..a312ef7 100644
--- a/doc/man3/OPENSSL_LH_COMPFUNC.pod
+++ b/doc/man3/OPENSSL_LH_COMPFUNC.pod
@@ -18,7 +18,7 @@ lh_TYPE_doall, lh_TYPE_doall_arg, lh_TYPE_error - dynamic hash table
 
  DECLARE_LHASH_OF(TYPE);
 
- LHASH *lh_TYPE_new();
+ LHASH *lh_TYPE_new(OPENSSL_LH_HASHFUNC hash, OPENSSL_LH_COMPFUNC compare);
  void lh_TYPE_free(LHASH_OF(TYPE) *table);
 
  TYPE *lh_TYPE_insert(LHASH_OF(TYPE) *table, TYPE *data);
@@ -27,7 +27,7 @@ lh_TYPE_doall, lh_TYPE_doall_arg, lh_TYPE_error - dynamic hash table
 
  void lh_TYPE_doall(LHASH_OF(TYPE) *table, OPENSSL_LH_DOALL_FUNC func);
  void lh_TYPE_doall_arg(LHASH_OF(TYPE) *table, OPENSSL_LH_DOALL_FUNCARG func,
-          TYPE, TYPE *arg);
+                        TYPE *arg);
 
  int lh_TYPE_error(LHASH_OF(TYPE) *table);
 
@@ -171,25 +171,18 @@ lh_TYPE_retrieve() returns the hash table entry if it has been found,
 B<NULL> otherwise.
 
 lh_TYPE_error() returns 1 if an error occurred in the last operation, 0
-otherwise.
+otherwise. It's meaningful only after non-retrieve operations.
 
 lh_TYPE_free(), lh_TYPE_doall() and lh_TYPE_doall_arg() return no values.
 
 =head1 NOTE
 
-The various LHASH macros and callback types exist to make it possible
-to write type-checked code without resorting to function-prototype
-casting - an evil that makes application code much harder to
-audit/verify and also opens the window of opportunity for stack
-corruption and other hard-to-find bugs.  It also, apparently, violates
-ANSI-C.
-
-The LHASH code is not thread safe. All updating operations must be
-performed under a write lock. All retrieve operations should be performed
-under a read lock, I<unless> accurate usage statistics are desired.
-In which case, a write lock should be used for retrieve operations
-as well.  For output of the usage statistics, using the functions from
-L<OPENSSL_LH_stats(3)>, a read lock suffices.
+The LHASH code is not thread safe. All updating operations, as well as
+lh_TYPE_error call must be performed under a write lock. All retrieve
+operations should be performed under a read lock, I<unless> accurate
+usage statistics are desired. In which case, a write lock should be used
+for retrieve operations as well. For output of the usage statistics,
+using the functions from L<OPENSSL_LH_stats(3)>, a read lock suffices.
 
 The LHASH code regards table entries as constant data.  As such, it
 internally represents lh_insert()'d items with a "const void *"
diff --git a/include/internal/tsan_assist.h b/include/internal/tsan_assist.h
new file mode 100644
index 0000000..f6870a2
--- /dev/null
+++ b/include/internal/tsan_assist.h
@@ -0,0 +1,84 @@
+/*
+ * Copyright 2018 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the OpenSSL license (the "License").  You may not use
+ * this file except in compliance with the License.  You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+ 
+/*
+ * Goal here is to facilitate writing "thread-opportunistic" code that
+ * withstands Thread Sanitizer's scrutiny. "Thread-opportunistic" is when
+ * exact result is not required, e.g. some statistics, or execution flow
+ * doesn't have to be unambiguous. Simplest example is lazy "constant"
+ * initialization when one can synchronize on variable itself, e.g.
+ *
+ * if (var == NOT_YET_INITIALIZED)
+ *     var = function_returning_same_value();
+ *
+ * This does work provided that loads and stores are single-instuction
+ * operations (and integer ones are on *all* supported platforms), but
+ * it upsets Thread Sanitizer. Suggested solution is
+ *
+ * if (tsan_load(&var) == NOT_YET_INITIALIZED)
+ *     tsan_store(&var, function_returning_same_value());
+ *
+ * Production machine code would be the same, so one can wonder why
+ * bother. Having Thread Sanitizer accept "thread-opportunistic" code
+ * allows to move on trouble-shooting real bugs.
+ *
+ * We utilize the fact that compilers that implement Thread Sanitizer
+ * implement even atomic operations. Then it's assumed that
+ * ATOMIC_{LONG|INT}_LOCK_FREE are assigned same value as
+ * ATOMIC_POINTER_LOCK_FREE. And check for >= 2 ensures that correspodning
+ * code is inlined. It should be noted that statistics counters become
+ * accurate in such case.
+ */
+
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L \
+    && !defined(__STDC_NO_ATOMICS__)
+# include <stdatomic.h>
+
+# if defined(ATOMIC_POINTER_LOCK_FREE) \
+          && ATOMIC_POINTER_LOCK_FREE >= 2
+#  define TSAN_QUALIFIER _Atomic
+#  define tsan_load(ptr) atomic_load_explicit((ptr), memory_order_relaxed)
+#  define tsan_store(ptr, val) atomic_store_explicit((ptr), (val), memory_order_relaxed)
+#  define tsan_counter(ptr) atomic_fetch_add_explicit((ptr), 1, memory_order_relaxed)
+# endif
+
+#elif defined(__GNUC__) && defined(__ATOMIC_RELAXED)
+
+# if defined(__GCC_ATOMIC_POINTER_LOCK_FREE) \
+          && __GCC_ATOMIC_POINTER_LOCK_FREE >= 2
+#  define TSAN_QUALIFIER volatile
+#  define tsan_load(ptr) __atomic_load_n((ptr), __ATOMIC_RELAXED)
+#  define tsan_store(ptr, val) __atomic_store_n((ptr), (val), __ATOMIC_RELAXED)
+#  define tsan_counter(ptr) __atomic_fetch_add((ptr), 1, __ATOMIC_RELAXED)
+# endif
+
+#elif defined(_MSC_VER) && _MSC_VER>=1200
+
+# define TSAN_QUALIFIER volatile
+# define tsan_load(ptr) (*(ptr))
+# define tsan_store(ptr, val) (*(ptr) = (val))
+# pragma intrinsic(_InterlockedExchangeAdd)
+# ifdef _WIN64
+#  pragma intrinsic(_InterlockedExchangeAdd64)
+#  define tsan_counter(ptr) (sizeof(*ptr) == 8 ? _InterlockedExchangeAdd64((ptr), 1) \
+                                               : _InterlockedExchangeAdd((ptr), 1))
+# else
+#  define tsan_counter(ptr) _InterlockedExchangeAdd((ptr), 1)
+# endif
+
+#endif
+
+#ifndef TSAN_QUALIFIER
+
+# define TSAN_QUALIFIER volatile
+# define tsan_load(ptr) (*(ptr))
+# define tsan_store(ptr, val) (*(ptr) = (val))
+# define tsan_counter(ptr) ((*(ptr))++)
+
+#endif
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 15380e1..a486356 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2264,7 +2264,6 @@ LHASH_OF(SSL_SESSION) *SSL_CTX_sessions(SSL_CTX *ctx)
 long SSL_CTX_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg)
 {
     long l;
-    int i;
     /* For some cases with ctx == NULL perform syntax checks */
     if (ctx == NULL) {
         switch (cmd) {
@@ -2319,40 +2318,27 @@ long SSL_CTX_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg)
     case SSL_CTRL_SESS_NUMBER:
         return lh_SSL_SESSION_num_items(ctx->sessions);
     case SSL_CTRL_SESS_CONNECT:
-        return CRYPTO_atomic_read(&ctx->stats.sess_connect, &i, ctx->lock)
-                ? i : 0;
+        return tsan_load(&ctx->stats.sess_connect);
     case SSL_CTRL_SESS_CONNECT_GOOD:
-        return CRYPTO_atomic_read(&ctx->stats.sess_connect_good, &i, ctx->lock)
-                ? i : 0;
+        return tsan_load(&ctx->stats.sess_connect_good);
     case SSL_CTRL_SESS_CONNECT_RENEGOTIATE:
-        return CRYPTO_atomic_read(&ctx->stats.sess_connect_renegotiate, &i,
-                                  ctx->lock)
-                ? i : 0;
+        return tsan_load(&ctx->stats.sess_connect_renegotiate);
     case SSL_CTRL_SESS_ACCEPT:
-        return CRYPTO_atomic_read(&ctx->stats.sess_accept, &i, ctx->lock)
-                ? i : 0;
+        return tsan_load(&ctx->stats.sess_accept);
     case SSL_CTRL_SESS_ACCEPT_GOOD:
-        return CRYPTO_atomic_read(&ctx->stats.sess_accept_good, &i, ctx->lock)
-                ? i : 0;
+        return tsan_load(&ctx->stats.sess_accept_good);
     case SSL_CTRL_SESS_ACCEPT_RENEGOTIATE:
-        return CRYPTO_atomic_read(&ctx->stats.sess_accept_renegotiate, &i,
-                                  ctx->lock)
-                ? i : 0;
+        return tsan_load(&ctx->stats.sess_accept_renegotiate);
     case SSL_CTRL_SESS_HIT:
-        return CRYPTO_atomic_read(&ctx->stats.sess_hit, &i, ctx->lock)
-                ? i : 0;
+        return tsan_load(&ctx->stats.sess_hit);
     case SSL_CTRL_SESS_CB_HIT:
-        return CRYPTO_atomic_read(&ctx->stats.sess_cb_hit, &i, ctx->lock)
-                ? i : 0;
+        return tsan_load(&ctx->stats.sess_cb_hit);
     case SSL_CTRL_SESS_MISSES:
-        return CRYPTO_atomic_read(&ctx->stats.sess_miss, &i, ctx->lock)
-                ? i : 0;
+        return tsan_load(&ctx->stats.sess_miss);
     case SSL_CTRL_SESS_TIMEOUTS:
-        return CRYPTO_atomic_read(&ctx->stats.sess_timeout, &i, ctx->lock)
-                ? i : 0;
+        return tsan_load(&ctx->stats.sess_timeout);
     case SSL_CTRL_SESS_CACHE_FULL:
-        return CRYPTO_atomic_read(&ctx->stats.sess_cache_full, &i, ctx->lock)
-                ? i : 0;
+        return tsan_load(&ctx->stats.sess_cache_full);
     case SSL_CTRL_MODE:
         return (ctx->mode |= larg);
     case SSL_CTRL_CLEAR_MODE:
@@ -3426,13 +3412,12 @@ void ssl_update_cache(SSL *s, int mode)
 
     /* auto flush every 255 connections */
     if ((!(i & SSL_SESS_CACHE_NO_AUTO_CLEAR)) && ((i & mode) == mode)) {
-        int *stat, val;
+        TSAN_QUALIFIER int *stat;
         if (mode & SSL_SESS_CACHE_CLIENT)
             stat = &s->session_ctx->stats.sess_connect_good;
         else
             stat = &s->session_ctx->stats.sess_accept_good;
-        if (CRYPTO_atomic_read(stat, &val, s->session_ctx->lock)
-            && (val & 0xff) == 0xff)
+        if ((tsan_load(stat) & 0xff) == 0xff)
             SSL_CTX_flush_sessions(s->session_ctx, (unsigned long)time(NULL));
     }
 }
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index e7258d4..a1a880c 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -33,6 +33,7 @@
 # include "packet_locl.h"
 # include "internal/dane.h"
 # include "internal/refcount.h"
+# include "internal/tsan_assist.h"
 
 # ifdef OPENSSL_BUILD_SHLIBSSL
 #  undef OPENSSL_EXTERN
@@ -779,21 +780,23 @@ struct ssl_ctx_st {
                                     const unsigned char *data, int len,
                                     int *copy);
     struct {
-        int sess_connect;       /* SSL new conn - started */
-        int sess_connect_renegotiate; /* SSL reneg - requested */
-        int sess_connect_good;  /* SSL new conne/reneg - finished */
-        int sess_accept;        /* SSL new accept - started */
-        int sess_accept_renegotiate; /* SSL reneg - requested */
-        int sess_accept_good;   /* SSL accept/reneg - finished */
-        int sess_miss;          /* session lookup misses */
-        int sess_timeout;       /* reuse attempt on timeouted session */
-        int sess_cache_full;    /* session removed due to full cache */
-        int sess_hit;           /* session reuse actually done */
-        int sess_cb_hit;        /* session-id that was not in the cache was
-                                 * passed back via the callback.  This
-                                 * indicates that the application is supplying
-                                 * session-id's from other processes - spooky
-                                 * :-) */
+        TSAN_QUALIFIER int sess_connect;       /* SSL new conn - started */
+        TSAN_QUALIFIER int sess_connect_renegotiate; /* SSL reneg - requested */
+        TSAN_QUALIFIER int sess_connect_good;  /* SSL new conne/reneg - finished */
+        TSAN_QUALIFIER int sess_accept;        /* SSL new accept - started */
+        TSAN_QUALIFIER int sess_accept_renegotiate; /* SSL reneg - requested */
+        TSAN_QUALIFIER int sess_accept_good;   /* SSL accept/reneg - finished */
+        TSAN_QUALIFIER int sess_miss;          /* session lookup misses */
+        TSAN_QUALIFIER int sess_timeout;       /* reuse attempt on timeouted session */
+        TSAN_QUALIFIER int sess_cache_full;    /* session removed due to full cache */
+        TSAN_QUALIFIER int sess_hit;           /* session reuse actually done */
+        TSAN_QUALIFIER int sess_cb_hit;        /* session-id that was not in
+                                                * the cache was passed back via
+                                                * the callback. This indicates
+                                                * that the application is
+                                                * supplying session-id's from
+                                                * other processes - spooky
+                                                * :-) */
     } stats;
 
     CRYPTO_REF_COUNT references;
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index d4a4808..5ad2792 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -448,7 +448,6 @@ SSL_SESSION *lookup_sess_in_cache(SSL *s, const unsigned char *sess_id,
                                   size_t sess_id_len)
 {
     SSL_SESSION *ret = NULL;
-    int discard;
 
     if ((s->session_ctx->session_cache_mode
          & SSL_SESS_CACHE_NO_INTERNAL_LOOKUP) == 0) {
@@ -469,8 +468,7 @@ SSL_SESSION *lookup_sess_in_cache(SSL *s, const unsigned char *sess_id,
         }
         CRYPTO_THREAD_unlock(s->session_ctx->lock);
         if (ret == NULL)
-            CRYPTO_atomic_add(&s->session_ctx->stats.sess_miss, 1, &discard,
-                              s->session_ctx->lock);
+            tsan_counter(&s->session_ctx->stats.sess_miss);
     }
 
     if (ret == NULL && s->session_ctx->get_session_cb != NULL) {
@@ -479,8 +477,7 @@ SSL_SESSION *lookup_sess_in_cache(SSL *s, const unsigned char *sess_id,
         ret = s->session_ctx->get_session_cb(s, sess_id, sess_id_len, &copy);
 
         if (ret != NULL) {
-            CRYPTO_atomic_add(&s->session_ctx->stats.sess_cb_hit, 1, &discard,
-                              s->session_ctx->lock);
+            tsan_counter(&s->session_ctx->stats.sess_cb_hit);
 
             /*
              * Increment reference count now if the session callback asks us
@@ -533,7 +530,7 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello)
     /* This is used only by servers. */
 
     SSL_SESSION *ret = NULL;
-    int fatal = 0, discard;
+    int fatal = 0;
     int try_session_cache = 0;
     SSL_TICKET_STATUS r;
 
@@ -612,8 +609,7 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello)
     }
 
     if (ret->timeout < (long)(time(NULL) - ret->time)) { /* timeout */
-        CRYPTO_atomic_add(&s->session_ctx->stats.sess_timeout, 1, &discard,
-                          s->session_ctx->lock);
+        tsan_counter(&s->session_ctx->stats.sess_timeout);
         if (try_session_cache) {
             /* session was from the cache, so remove it */
             SSL_CTX_remove_session(s->session_ctx, ret);
@@ -641,8 +637,7 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello)
         s->session = ret;
     }
 
-    CRYPTO_atomic_add(&s->session_ctx->stats.sess_hit, 1, &discard,
-                      s->session_ctx->lock);
+    tsan_counter(&s->session_ctx->stats.sess_hit);
     s->verify_result = s->session->verify_result;
     return 1;
 
@@ -669,7 +664,7 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello)
 
 int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c)
 {
-    int ret = 0, discard;
+    int ret = 0;
     SSL_SESSION *s;
 
     /*
@@ -736,8 +731,7 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c)
                 if (!remove_session_lock(ctx, ctx->session_cache_tail, 0))
                     break;
                 else
-                    CRYPTO_atomic_add(&ctx->stats.sess_cache_full, 1, &discard,
-                                      ctx->lock);
+                    tsan_counter(&ctx->stats.sess_cache_full);
             }
         }
     }
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 85945ac..12c712e 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -912,7 +912,7 @@ static int init_server_name(SSL *s, unsigned int context)
 
 static int final_server_name(SSL *s, unsigned int context, int sent)
 {
-    int ret = SSL_TLSEXT_ERR_NOACK, discard;
+    int ret = SSL_TLSEXT_ERR_NOACK;
     int altmp = SSL_AD_UNRECOGNIZED_NAME;
     int was_ticket = (SSL_get_options(s) & SSL_OP_NO_TICKET) == 0;
 
@@ -960,10 +960,8 @@ static int final_server_name(SSL *s, unsigned int context, int sent)
      * exceed sess_accept (zero) for the new context.
      */
     if (SSL_IS_FIRST_HANDSHAKE(s) && s->ctx != s->session_ctx) {
-        CRYPTO_atomic_add(&s->ctx->stats.sess_accept, 1, &discard,
-                          s->ctx->lock);
-        CRYPTO_atomic_add(&s->session_ctx->stats.sess_accept, -1, &discard,
-                          s->session_ctx->lock);
+        tsan_counter(&s->ctx->stats.sess_accept);
+        tsan_counter(&s->session_ctx->stats.sess_accept);
     }
 
     /*
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index e846f77..3dc29cc 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -1409,7 +1409,6 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     unsigned int compression;
     unsigned int sversion;
     unsigned int context;
-    int discard;
     RAW_EXTENSION *extensions = NULL;
 #ifndef OPENSSL_NO_COMP
     SSL_COMP *comp;
@@ -1616,8 +1615,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
                 || (SSL_IS_TLS13(s)
                     && s->session->ext.tick_identity
                        != TLSEXT_PSK_BAD_IDENTITY)) {
-            CRYPTO_atomic_add(&s->session_ctx->stats.sess_miss, 1, &discard,
-                              s->session_ctx->lock);
+            tsan_counter(&s->session_ctx->stats.sess_miss);
             if (!ssl_get_new_session(s, 0)) {
                 /* SSLfatal() already called */
                 goto err;
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index ebb21de..caed61a 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -132,23 +132,18 @@ int tls_setup_handshake(SSL *s)
         }
         if (SSL_IS_FIRST_HANDSHAKE(s)) {
             /* N.B. s->session_ctx == s->ctx here */
-            CRYPTO_atomic_add(&s->session_ctx->stats.sess_accept, 1, &i,
-                              s->session_ctx->lock);
+            tsan_counter(&s->session_ctx->stats.sess_accept);
         } else {
             /* N.B. s->ctx may not equal s->session_ctx */
-            CRYPTO_atomic_add(&s->ctx->stats.sess_accept_renegotiate, 1, &i,
-                              s->ctx->lock);
+            tsan_counter(&s->ctx->stats.sess_accept_renegotiate);
 
             s->s3->tmp.cert_request = 0;
         }
     } else {
-        int discard;
         if (SSL_IS_FIRST_HANDSHAKE(s))
-            CRYPTO_atomic_add(&s->session_ctx->stats.sess_connect, 1, &discard,
-                              s->session_ctx->lock);
+            tsan_counter(&s->session_ctx->stats.sess_connect);
         else
-            CRYPTO_atomic_add(&s->session_ctx->stats.sess_connect_renegotiate,
-                              1, &discard, s->session_ctx->lock);
+            tsan_counter(&s->session_ctx->stats.sess_connect_renegotiate);
 
         /* mark client_random uninitialized */
         memset(s->s3->client_random, 0, sizeof(s->s3->client_random));
@@ -1009,7 +1004,6 @@ unsigned long ssl3_output_cert_chain(SSL *s, WPACKET *pkt, CERT_PKEY *cpk)
  */
 WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs, int stop)
 {
-    int discard;
     void (*cb) (const SSL *ssl, int type, int val) = NULL;
 
     if (clearbufs) {
@@ -1055,8 +1049,7 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs, int stop)
                 ssl_update_cache(s, SSL_SESS_CACHE_SERVER);
 
             /* N.B. s->ctx may not equal s->session_ctx */
-            CRYPTO_atomic_add(&s->ctx->stats.sess_accept_good, 1, &discard,
-                              s->ctx->lock);
+            tsan_counter(&s->ctx->stats.sess_accept_good);
             s->handshake_func = ossl_statem_accept;
 
             if (SSL_IS_DTLS(s) && !s->hit) {
@@ -1084,12 +1077,10 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst, int clearbufs, int stop)
                 ssl_update_cache(s, SSL_SESS_CACHE_CLIENT);
             }
             if (s->hit)
-                CRYPTO_atomic_add(&s->session_ctx->stats.sess_hit, 1, &discard,
-                                  s->session_ctx->lock);
+                tsan_counter(&s->session_ctx->stats.sess_hit);
 
             s->handshake_func = ossl_statem_connect;
-            CRYPTO_atomic_add(&s->session_ctx->stats.sess_connect_good, 1,
-                              &discard, s->session_ctx->lock);
+            tsan_counter(&s->session_ctx->stats.sess_connect_good);
 
             if (SSL_IS_DTLS(s) && s->hit) {
                 /*


More information about the openssl-commits mailing list