lhash: use lock when TSAN not available for statistics gathering

Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/17479)

diff --git a/crypto/lhash/lh_stats.c b/crypto/lhash/lh_stats.c
index 5e38c42580aa970edab18c73cd37a24a7b92e1ea..0d4bc72608d8ce8770efe1f997d3723f13b5bd9f 100644 (file)
--- 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 b2fa4c518bcc67d53c7156536a464c3cf102fc42..62e2be4c18a8e98f1fcf6b952328903ee78a983a 100644 (file)
--- 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 ad9dd4d346ebd50640128277d5a7609ca7b6bde5..35c0c5b49c919ed6a6a351e8455a39cab08c42bf 100644 (file)
--- 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
 };