diff options
author | Pauli <pauli@openssl.org> | 2023-06-13 11:39:23 +1000 |
---|---|---|
committer | Pauli <pauli@openssl.org> | 2023-06-14 16:45:16 +1000 |
commit | f5690fecb3b0db604e4826323ea5a5e2c403e62f (patch) | |
tree | 977c330078d67716e6b8d87d0ce7c9d8a5591076 /providers | |
parent | c553c08232f7dc7eab0a4b9a739b9295feb0d666 (diff) |
fips: use memory ordering rather than locks
The FIPS provider accesses it's current state under lock.
This is overkill, little or no synchronisation is actually required in
practice (because it's essentially a read only setting). Switch to using
TSAN operations in preference.
Fixes #21179
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21187)
(cherry picked from commit 8e9ca334528e0a923c4deb0af250a60510974be0)
Diffstat (limited to 'providers')
-rw-r--r-- | providers/fips/self_test.c | 50 |
1 files changed, 14 insertions, 36 deletions
diff --git a/providers/fips/self_test.c b/providers/fips/self_test.c index 10804d9f59..9a55bfb79d 100644 --- a/providers/fips/self_test.c +++ b/providers/fips/self_test.c @@ -17,6 +17,7 @@ #include <openssl/proverr.h> #include <openssl/rand.h> #include "internal/e_os.h" +#include "internal/tsan_assist.h" #include "prov/providercommon.h" /* @@ -48,7 +49,6 @@ static int FIPS_conditional_error_check = 1; static CRYPTO_RWLOCK *self_test_lock = NULL; -static CRYPTO_RWLOCK *fips_state_lock = NULL; static unsigned char fixed_key[32] = { FIPS_KEY_ELEMENTS }; static CRYPTO_ONCE fips_self_test_init = CRYPTO_ONCE_STATIC_INIT; @@ -60,7 +60,6 @@ DEFINE_RUN_ONCE_STATIC(do_fips_self_test_init) * platform then we just leak it deliberately. */ self_test_lock = CRYPTO_THREAD_lock_new(); - fips_state_lock = CRYPTO_THREAD_lock_new(); return self_test_lock != NULL; } @@ -156,12 +155,12 @@ void __TERM__cleanup(void) { # define DEP_INITIAL_STATE FIPS_STATE_SELFTEST #endif -static int FIPS_state = DEP_INITIAL_STATE; +static TSAN_QUALIFIER int FIPS_state = DEP_INITIAL_STATE; #if defined(DEP_INIT_ATTRIBUTE) DEP_INIT_ATTRIBUTE void init(void) { - FIPS_state = FIPS_STATE_SELFTEST; + tsan_store(&FIPS_state, FIPS_STATE_SELFTEST); } #endif @@ -169,7 +168,6 @@ DEP_INIT_ATTRIBUTE void init(void) DEP_FINI_ATTRIBUTE void cleanup(void) { CRYPTO_THREAD_lock_free(self_test_lock); - CRYPTO_THREAD_lock_free(fips_state_lock); } #endif @@ -291,10 +289,7 @@ err: static void set_fips_state(int state) { - if (ossl_assert(CRYPTO_THREAD_write_lock(fips_state_lock) != 0)) { - FIPS_state = state; - CRYPTO_THREAD_unlock(fips_state_lock); - } + tsan_store(&FIPS_state, state); } /* This API is triggered either on loading of the FIPS module or on demand */ @@ -314,10 +309,7 @@ int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test) if (!RUN_ONCE(&fips_self_test_init, do_fips_self_test_init)) return 0; - if (!CRYPTO_THREAD_read_lock(fips_state_lock)) - return 0; - loclstate = FIPS_state; - CRYPTO_THREAD_unlock(fips_state_lock); + loclstate = tsan_load(&FIPS_state); if (loclstate == FIPS_STATE_RUNNING) { if (!on_demand_test) @@ -329,24 +321,17 @@ int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test) if (!CRYPTO_THREAD_write_lock(self_test_lock)) return 0; - if (!CRYPTO_THREAD_read_lock(fips_state_lock)) { - CRYPTO_THREAD_unlock(self_test_lock); - return 0; - } - if (FIPS_state == FIPS_STATE_RUNNING) { - CRYPTO_THREAD_unlock(fips_state_lock); + loclstate = tsan_load(&FIPS_state); + if (loclstate == FIPS_STATE_RUNNING) { if (!on_demand_test) { CRYPTO_THREAD_unlock(self_test_lock); return 1; } set_fips_state(FIPS_STATE_SELFTEST); - } else if (FIPS_state != FIPS_STATE_SELFTEST) { - CRYPTO_THREAD_unlock(fips_state_lock); + } else if (loclstate != FIPS_STATE_SELFTEST) { CRYPTO_THREAD_unlock(self_test_lock); ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_STATE); return 0; - } else { - CRYPTO_THREAD_unlock(fips_state_lock); } if (st == NULL @@ -469,20 +454,13 @@ void ossl_set_error_state(const char *type) int ossl_prov_is_running(void) { - int res; - static unsigned int rate_limit = 0; + int res, loclstate; + static TSAN_QUALIFIER unsigned int rate_limit = 0; - if (!CRYPTO_THREAD_read_lock(fips_state_lock)) - return 0; - res = FIPS_state == FIPS_STATE_RUNNING - || FIPS_state == FIPS_STATE_SELFTEST; - if (FIPS_state == FIPS_STATE_ERROR) { - CRYPTO_THREAD_unlock(fips_state_lock); - if (!CRYPTO_THREAD_write_lock(fips_state_lock)) - return 0; - if (rate_limit++ < FIPS_ERROR_REPORTING_RATE_LIMIT) + loclstate = tsan_load(&FIPS_state); + res = loclstate == FIPS_STATE_RUNNING || loclstate == FIPS_STATE_SELFTEST; + if (loclstate == FIPS_STATE_ERROR) + if (tsan_add(&rate_limit, 1) < FIPS_ERROR_REPORTING_RATE_LIMIT) ERR_raise(ERR_LIB_PROV, PROV_R_FIPS_MODULE_IN_ERROR_STATE); - } - CRYPTO_THREAD_unlock(fips_state_lock); return res; } |