Commit 9ef9088c authored by Andy Polyakov's avatar Andy Polyakov
Browse files

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



Reviewed-by: default avatarPaul Dale <paul.dale@oracle.com>
Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6786)
parent cab76c0f

ssl/ssl_lib.c

+13 −28
Original line number Diff line number Diff line
@@ -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));

    }

}

ssl/ssl_locl.h

+18 −15
Original line number Diff line number Diff line
@@ -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,20 +780,22 @@ 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;

ssl/ssl_sess.c

+7 −13
Original line number Diff line number Diff line
@@ -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);

            }

        }

    }

ssl/statem/extensions.c

+3 −5
Original line number Diff line number Diff line
@@ -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);

    }



    /*

ssl/statem/statem_clnt.c

+1 −3
Original line number Diff line number Diff line
@@ -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;