[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