ssl/*: switch to switch to Thread-Sanitizer-friendly primitives.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6786)

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 15380e1b72d4e8bc60414e0ddfa2a844a6d2dd0d..a486356c2ae6cdd3bc2ef8ffeabbe5461ca3a6bf 100644 (file)
--- 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 e7258d439a05429181220e8e5de7b0213090d07b..a1a880c05c496c72a12c571103785aa70fc3783e 100644 (file)
--- 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 d4a4808f197a1d7337e9d4b70b111afca98c5008..5ad2792a1b4c9087f684b4eaa748f8e6cdf0ea52 100644 (file)
--- 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 85945acc0df3ba0e1632510048c16433c6c4b82f..12c712e8e6cc730cc0c7ac63453c25dcdea02a0a 100644 (file)
--- 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 e846f772a6e4492787192e2490a0b7c85f75b1c2..3dc29cc75749f9236198a3b42c1b4812e874d4ac 100644 (file)
--- 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 ebb21deb8ba5266870721a9b785ac81d50740fce..caed61aaac0439b11525c0bc8467ed7cf1aba577 100644 (file)
--- 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) {
                 /*