[openssl] openssl-3.0 update

Dr. Paul Dale pauli at openssl.org
Thu Jan 13 10:54:39 UTC 2022


The branch openssl-3.0 has been updated
       via  589e0ab4ebf35e1e73d826ad08160b9e6060e616 (commit)
       via  d1a488e944275a1b5db50ce02c1593aedb37c1f9 (commit)
       via  a69b93afd26d8da664e19847432cebe3c7d3fbb3 (commit)
       via  cc05c3ea8c585eb58a46602f94c59e3c17f4383d (commit)
       via  d1ec05915686019eec8fa8de9890292980fc5d6e (commit)
       via  3517a3e055d3ed27b70441e2ee087fbb709b58da (commit)
      from  cca25d5eb83b56ae27d81bd72bebf69c2f393e43 (commit)


- Log -----------------------------------------------------------------
commit 589e0ab4ebf35e1e73d826ad08160b9e6060e616
Author: Pauli <pauli at openssl.org>
Date:   Wed Jan 12 15:01:17 2022 +1100

    drbg: add handling for cases where TSAN isn't available
    
    Most of the DRGB code is run under lock from the EVP layer.  This is relied
    on to make the majority of TSAN operations safe.  However, it is still necessary
    to enable locking for all DRBGs created.
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/17479)

commit d1a488e944275a1b5db50ce02c1593aedb37c1f9
Author: Pauli <pauli at openssl.org>
Date:   Wed Jan 12 14:45:07 2022 +1100

    lhash: use lock when TSAN not available for statistics gathering
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/17479)

commit a69b93afd26d8da664e19847432cebe3c7d3fbb3
Author: Pauli <pauli at openssl.org>
Date:   Wed Jan 12 14:25:46 2022 +1100

    mem: do not produce usage counts when tsan is unavailable.
    
    Doing the tsan operations under lock would be difficult to arrange here (locks
    require memory allocation).
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/17479)

commit cc05c3ea8c585eb58a46602f94c59e3c17f4383d
Author: Pauli <pauli at openssl.org>
Date:   Wed Jan 12 14:22:23 2022 +1100

    core namemap: use updated tsan lock detection capabilities
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/17479)

commit d1ec05915686019eec8fa8de9890292980fc5d6e
Author: Pauli <pauli at openssl.org>
Date:   Wed Jan 12 13:26:38 2022 +1100

    tsan: make detecting the need for locking when using tsan easier
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/17479)

commit 3517a3e055d3ed27b70441e2ee087fbb709b58da
Author: Pauli <pauli at openssl.org>
Date:   Wed Jan 12 14:24:49 2022 +1100

    threadstest: add write check to lock checking
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/17479)

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

Summary of changes:
 crypto/core_namemap.c                  | 15 ++++------
 crypto/lhash/lh_stats.c                | 25 ++++++++++++----
 crypto/lhash/lhash.c                   | 55 ++++++++++++++++++++++++++--------
 crypto/lhash/lhash_local.h             |  3 ++
 crypto/mem.c                           | 14 ++++++---
 include/internal/tsan_assist.h         |  8 ++++-
 providers/implementations/rands/drbg.c |  4 +++
 test/threadstest.c                     |  2 ++
 8 files changed, 95 insertions(+), 31 deletions(-)

diff --git a/crypto/core_namemap.c b/crypto/core_namemap.c
index 2bee5ef194..6cb0ec5a06 100644
--- a/crypto/core_namemap.c
+++ b/crypto/core_namemap.c
@@ -37,11 +37,7 @@ struct ossl_namemap_st {
     CRYPTO_RWLOCK *lock;
     LHASH_OF(NAMENUM_ENTRY) *namenum;  /* Name->number mapping */
 
-#ifdef tsan_ld_acq
-    TSAN_QUALIFIER int max_number;     /* Current max number TSAN version */
-#else
-    int max_number;                    /* Current max number plain version */
-#endif
+    TSAN_QUALIFIER int max_number;     /* Current max number */
 };
 
 /* LHASH callbacks */
@@ -99,10 +95,7 @@ static const OSSL_LIB_CTX_METHOD stored_namemap_method = {
 
 int ossl_namemap_empty(OSSL_NAMEMAP *namemap)
 {
-#ifdef tsan_ld_acq
-    /* Have TSAN support */
-    return namemap == NULL || tsan_load(&namemap->max_number) == 0;
-#else
+#ifdef TSAN_REQUIRES_LOCKING
     /* No TSAN support */
     int rv;
 
@@ -114,6 +107,9 @@ int ossl_namemap_empty(OSSL_NAMEMAP *namemap)
     rv = namemap->max_number == 0;
     CRYPTO_THREAD_unlock(namemap->lock);
     return rv;
+#else
+    /* Have TSAN support */
+    return namemap == NULL || tsan_load(&namemap->max_number) == 0;
 #endif
 }
 
@@ -260,6 +256,7 @@ static int namemap_add_name_n(OSSL_NAMEMAP *namemap, int number,
         || (namenum->name = OPENSSL_strndup(name, name_len)) == NULL)
         goto err;
 
+    /* The tsan_counter use here is safe since we're under lock */
     namenum->number =
         number != 0 ? number : 1 + tsan_counter(&namemap->max_number);
     (void)lh_NAMENUM_ENTRY_insert(namemap->namenum, namenum);
diff --git a/crypto/lhash/lh_stats.c b/crypto/lhash/lh_stats.c
index 5e38c42580..0d4bc72608 100644
--- a/crypto/lhash/lh_stats.c
+++ b/crypto/lhash/lh_stats.c
@@ -61,6 +61,14 @@ void OPENSSL_LH_node_usage_stats(const OPENSSL_LHASH *lh, FILE *fp)
 
 void OPENSSL_LH_stats_bio(const OPENSSL_LHASH *lh, BIO *out)
 {
+    int omit_tsan = 0;
+
+#ifdef TSAN_REQUIRES_LOCKING
+    if (!CRYPTO_THREAD_read_lock(lh->tsan_lock)) {
+        BIO_printf(out, "unable to lock table, omitting TSAN counters\n");
+        omit_tsan = 1;
+    }
+#endif
     BIO_printf(out, "num_items             = %lu\n", lh->num_items);
     BIO_printf(out, "num_nodes             = %u\n",  lh->num_nodes);
     BIO_printf(out, "num_alloc_nodes       = %u\n",  lh->num_alloc_nodes);
@@ -68,15 +76,22 @@ void OPENSSL_LH_stats_bio(const OPENSSL_LHASH *lh, BIO *out)
     BIO_printf(out, "num_expand_reallocs   = %lu\n", lh->num_expand_reallocs);
     BIO_printf(out, "num_contracts         = %lu\n", lh->num_contracts);
     BIO_printf(out, "num_contract_reallocs = %lu\n", lh->num_contract_reallocs);
-    BIO_printf(out, "num_hash_calls        = %lu\n", lh->num_hash_calls);
-    BIO_printf(out, "num_comp_calls        = %lu\n", lh->num_comp_calls);
+    if (!omit_tsan) {
+        BIO_printf(out, "num_hash_calls        = %lu\n", lh->num_hash_calls);
+        BIO_printf(out, "num_comp_calls        = %lu\n", lh->num_comp_calls);
+    }
     BIO_printf(out, "num_insert            = %lu\n", lh->num_insert);
     BIO_printf(out, "num_replace           = %lu\n", lh->num_replace);
     BIO_printf(out, "num_delete            = %lu\n", lh->num_delete);
     BIO_printf(out, "num_no_delete         = %lu\n", lh->num_no_delete);
-    BIO_printf(out, "num_retrieve          = %lu\n", lh->num_retrieve);
-    BIO_printf(out, "num_retrieve_miss     = %lu\n", lh->num_retrieve_miss);
-    BIO_printf(out, "num_hash_comps        = %lu\n", lh->num_hash_comps);
+    if (!omit_tsan) {
+        BIO_printf(out, "num_retrieve          = %lu\n", lh->num_retrieve);
+        BIO_printf(out, "num_retrieve_miss     = %lu\n", lh->num_retrieve_miss);
+        BIO_printf(out, "num_hash_comps        = %lu\n", lh->num_hash_comps);
+#ifdef TSAN_REQUIRES_LOCKING
+        CRYPTO_THREAD_unlock(lh->tsan_lock);
+#endif
+    }
 }
 
 void OPENSSL_LH_node_stats_bio(const OPENSSL_LHASH *lh, BIO *out)
diff --git a/crypto/lhash/lhash.c b/crypto/lhash/lhash.c
index 63cf46af6e..82d0ec5b8b 100644
--- a/crypto/lhash/lhash.c
+++ b/crypto/lhash/lhash.c
@@ -44,6 +44,22 @@ static int expand(OPENSSL_LHASH *lh);
 static void contract(OPENSSL_LHASH *lh);
 static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh, const void *data, unsigned long *rhash);
 
+static ossl_inline int tsan_lock(const OPENSSL_LHASH *lh)
+{
+#ifdef TSAN_REQUIRES_LOCKING
+    if (!CRYPTO_THREAD_write_lock(lh->tsan_lock))
+        return 0;
+#endif
+    return 1;
+}
+
+static ossl_inline void tsan_unlock(const OPENSSL_LHASH *lh)
+{
+#ifdef TSAN_REQUIRES_LOCKING
+    CRYPTO_THREAD_unlock(lh->tsan_lock);
+#endif
+}
+
 OPENSSL_LHASH *OPENSSL_LH_new(OPENSSL_LH_HASHFUNC h, OPENSSL_LH_COMPFUNC c)
 {
     OPENSSL_LHASH *ret;
@@ -58,6 +74,10 @@ OPENSSL_LHASH *OPENSSL_LH_new(OPENSSL_LH_HASHFUNC h, OPENSSL_LH_COMPFUNC c)
     }
     if ((ret->b = OPENSSL_zalloc(sizeof(*ret->b) * MIN_NODES)) == NULL)
         goto err;
+#ifdef TSAN_REQUIRES_LOCKING
+    if ((ret->tsan_lock = CRYPTO_THREAD_lock_new()) == NULL)
+        goto err;
+#endif
     ret->comp = ((c == NULL) ? (OPENSSL_LH_COMPFUNC)strcmp : c);
     ret->hash = ((h == NULL) ? (OPENSSL_LH_HASHFUNC)OPENSSL_LH_strhash : h);
     ret->num_nodes = MIN_NODES / 2;
@@ -79,6 +99,9 @@ void OPENSSL_LH_free(OPENSSL_LHASH *lh)
         return;
 
     OPENSSL_LH_flush(lh);
+#ifdef TSAN_REQUIRES_LOCKING
+    CRYPTO_THREAD_lock_free(lh->tsan_lock);
+#endif
     OPENSSL_free(lh->b);
     OPENSSL_free(lh);
 }
@@ -166,21 +189,20 @@ void *OPENSSL_LH_retrieve(OPENSSL_LHASH *lh, const void *data)
 {
     unsigned long hash;
     OPENSSL_LH_NODE **rn;
-    void *ret;
 
+    /*-
+     * This should be atomic without tsan.
+     * It's not clear why it was done this way and not elsewhere.
+     */
     tsan_store((TSAN_QUALIFIER int *)&lh->error, 0);
 
     rn = getrn(lh, data, &hash);
 
-    if (*rn == NULL) {
-        tsan_counter(&lh->num_retrieve_miss);
-        return NULL;
-    } else {
-        ret = (*rn)->data;
-        tsan_counter(&lh->num_retrieve);
+    if (tsan_lock(lh)) {
+        tsan_counter(*rn == NULL ? &lh->num_retrieve_miss : &lh->num_retrieve);
+        tsan_unlock(lh);
     }
-
-    return ret;
+    return *rn == NULL ? NULL : (*rn)->data;
 }
 
 static void doall_util_fn(OPENSSL_LHASH *lh, int use_arg,
@@ -307,9 +329,14 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh,
     OPENSSL_LH_NODE **ret, *n1;
     unsigned long hash, nn;
     OPENSSL_LH_COMPFUNC cf;
+    int do_tsan = 1;
 
+#ifdef TSAN_REQUIRES_LOCKING
+    do_tsan = tsan_lock(lh);
+#endif
     hash = (*(lh->hash)) (data);
-    tsan_counter(&lh->num_hash_calls);
+    if (do_tsan)
+        tsan_counter(&lh->num_hash_calls);
     *rhash = hash;
 
     nn = hash % lh->pmax;
@@ -319,16 +346,20 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh,
     cf = lh->comp;
     ret = &(lh->b[(int)nn]);
     for (n1 = *ret; n1 != NULL; n1 = n1->next) {
-        tsan_counter(&lh->num_hash_comps);
+        if (do_tsan)
+            tsan_counter(&lh->num_hash_comps);
         if (n1->hash != hash) {
             ret = &(n1->next);
             continue;
         }
-        tsan_counter(&lh->num_comp_calls);
+        if (do_tsan)
+            tsan_counter(&lh->num_comp_calls);
         if (cf(n1->data, data) == 0)
             break;
         ret = &(n1->next);
     }
+    if (do_tsan)
+        tsan_unlock(lh);
     return ret;
 }
 
diff --git a/crypto/lhash/lhash_local.h b/crypto/lhash/lhash_local.h
index ad9dd4d346..35c0c5b49c 100644
--- a/crypto/lhash/lhash_local.h
+++ b/crypto/lhash/lhash_local.h
@@ -41,4 +41,7 @@ struct lhash_st {
     TSAN_QUALIFIER unsigned long num_retrieve_miss;
     TSAN_QUALIFIER unsigned long num_hash_comps;
     int error;
+#ifdef TSAN_REQUIRES_LOCKING
+    CRYPTO_RWLOCK *tsan_lock;
+#endif
 };
diff --git a/crypto/mem.c b/crypto/mem.c
index d682a3686f..242d88470a 100644
--- a/crypto/mem.c
+++ b/crypto/mem.c
@@ -26,11 +26,17 @@ static CRYPTO_free_fn free_impl = CRYPTO_free;
 #if !defined(OPENSSL_NO_CRYPTO_MDEBUG) && !defined(FIPS_MODULE)
 # include "internal/tsan_assist.h"
 
+# ifdef TSAN_REQUIRES_LOCKING
+#  define INCREMENT(x) /* empty */
+#  define LOAD(x) 0
+# else  /* TSAN_REQUIRES_LOCKING */
 static TSAN_QUALIFIER int malloc_count;
 static TSAN_QUALIFIER int realloc_count;
 static TSAN_QUALIFIER int free_count;
 
-# define INCREMENT(x) tsan_counter(&(x))
+#  define INCREMENT(x) tsan_counter(&(x))
+#  define LOAD(x)      tsan_load(&x)
+# endif /* TSAN_REQUIRES_LOCKING */
 
 static char *md_failstring;
 static long md_count;
@@ -79,11 +85,11 @@ void CRYPTO_get_mem_functions(CRYPTO_malloc_fn *malloc_fn,
 void CRYPTO_get_alloc_counts(int *mcount, int *rcount, int *fcount)
 {
     if (mcount != NULL)
-        *mcount = tsan_load(&malloc_count);
+        *mcount = LOAD(malloc_count);
     if (rcount != NULL)
-        *rcount = tsan_load(&realloc_count);
+        *rcount = LOAD(realloc_count);
     if (fcount != NULL)
-        *fcount = tsan_load(&free_count);
+        *fcount = LOAD(free_count);
 }
 
 /*
diff --git a/include/internal/tsan_assist.h b/include/internal/tsan_assist.h
index f8285b1d85..37631ab816 100644
--- a/include/internal/tsan_assist.h
+++ b/include/internal/tsan_assist.h
@@ -130,7 +130,13 @@
 
 #ifndef TSAN_QUALIFIER
 
-# define TSAN_QUALIFIER volatile
+# ifdef OPENSSL_THREADS
+#  define TSAN_QUALIFIER volatile
+#  define TSAN_REQUIRES_LOCKING
+# else  /* OPENSSL_THREADS */
+#  define TSAN_QUALIFIER
+# endif /* OPENSSL_THREADS */
+
 # define tsan_load(ptr) (*(ptr))
 # define tsan_store(ptr, val) (*(ptr) = (val))
 # define tsan_counter(ptr) ((*(ptr))++)
diff --git a/providers/implementations/rands/drbg.c b/providers/implementations/rands/drbg.c
index 8b899b99b1..16d382dced 100644
--- a/providers/implementations/rands/drbg.c
+++ b/providers/implementations/rands/drbg.c
@@ -837,6 +837,10 @@ PROV_DRBG *ossl_rand_drbg_new
             goto err;
         }
     }
+#ifdef TSAN_REQUIRES_LOCKING
+    if (!ossl_drbg_enable_locking(drbg))
+        goto err;
+#endif
     return drbg;
 
  err:
diff --git a/test/threadstest.c b/test/threadstest.c
index b7e781fb6b..bfda43d891 100644
--- a/test/threadstest.c
+++ b/test/threadstest.c
@@ -33,6 +33,8 @@ static int test_lock(void)
     int res;
 
     res = TEST_true(CRYPTO_THREAD_read_lock(lock))
+          && TEST_true(CRYPTO_THREAD_unlock(lock))
+          && TEST_true(CRYPTO_THREAD_write_lock(lock))
           && TEST_true(CRYPTO_THREAD_unlock(lock));
 
     CRYPTO_THREAD_lock_free(lock);


More information about the openssl-commits mailing list