mmo-ref/dep-openssl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Make FIPS self test state access atomic

Browse source 
Direct access to the FIPS self-test state array caused race conditions in
multi-threaded environments when checking or updating test status.

Introduce atomic accessor functions `ossl_get_self_test_state` and
`ossl_set_self_test_state`, backed by a global lock, to ensure thread-safe
state transitions. Replace all direct structure accesses with these new
functions.

Signed-off-by: Simo Sorce <simo@redhat.com>

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/30009)
This commit is contained in:
Simo Sorce 2026-02-13 22:38:26 -05:00 • committed by Pauli
parent 9a15137096
commit cc7195da30
10 changed files with 164 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

7
crypto/threads_none.c
View file

@ -275,6 +275,13 @@ int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock)
return 1;
}
int CRYPTO_atomic_store_int(int *dst, int val, CRYPTO_RWLOCK *lock)
{
*dst = val;
return 1;
}
int openssl_init_fork_handlers(void)
{
return 0;

23
crypto/threads_pthread.c
View file

@ -1201,6 +1201,29 @@ int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock)
return 1;
}
int CRYPTO_atomic_store_int(int *dst, int val, CRYPTO_RWLOCK *lock)
{
#if defined(__GNUC__) && defined(__ATOMIC_ACQ_REL) && !defined(BROKEN_CLANG_ATOMICS)
if (__atomic_is_lock_free(sizeof(*dst), dst)) {
__atomic_store(dst, &val, __ATOMIC_RELEASE);
return 1;
}
#elif defined(__sun) && (defined(__SunOS_5_10) || defined(__SunOS_5_11))
/* This will work for all future Solaris versions. */
if (dst != NULL) {
atomic_swap_uint((unsigned int)dst, (unsigned int)val);
return 1;
}
#endif
if (lock == NULL || !CRYPTO_THREAD_write_lock(lock))
return 0;
*dst = val;
if (!CRYPTO_THREAD_unlock(lock))
return 0;
return 1;
}
#ifndef FIPS_MODULE
int openssl_init_fork_handlers(void)
{

16
crypto/threads_win.c
View file

@ -732,6 +732,22 @@ int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock)
#endif
}
int CRYPTO_atomic_store_int(int *dst, int val, CRYPTO_RWLOCK *lock)
{
#if (defined(NO_INTERLOCKEDOR64))
if (lock == NULL || !CRYPTO_THREAD_read_lock(lock))
return 0;
*dst = val;
if (!CRYPTO_THREAD_unlock(lock))
return 0;
return 1;
#else
InterlockedExchange(dst, val);
return 1;
#endif
}
int openssl_init_fork_handlers(void)
{
return 0;

9
doc/man3/CRYPTO_THREAD_run_once.pod
View file

@ -7,6 +7,7 @@ CRYPTO_THREAD_lock_new, CRYPTO_THREAD_read_lock, CRYPTO_THREAD_write_lock,
CRYPTO_THREAD_unlock, CRYPTO_THREAD_lock_free,
CRYPTO_atomic_add, CRYPTO_atomic_add64, CRYPTO_atomic_and, CRYPTO_atomic_or,
CRYPTO_atomic_load, CRYPTO_atomic_store, CRYPTO_atomic_load_int,
CRYPTO_atomic_store_int,
OSSL_set_max_threads, OSSL_get_max_threads,
OSSL_get_thread_support_flags, OSSL_THREAD_SUPPORT_FLAG_THREAD_POOL,
OSSL_THREAD_SUPPORT_FLAG_DEFAULT_SPAWN - OpenSSL thread support
@ -34,6 +35,7 @@ OSSL_THREAD_SUPPORT_FLAG_DEFAULT_SPAWN - OpenSSL thread support
int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock);
int CRYPTO_atomic_store(uint64_t *dst, uint64_t val, CRYPTO_RWLOCK *lock);
int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock);
int CRYPTO_atomic_store_int(int *dst, int val, CRYPTO_RWLOCK *lock);
int OSSL_set_max_threads(OSSL_LIB_CTX *ctx, uint64_t max_threads);
uint64_t OSSL_get_max_threads(OSSL_LIB_CTX *ctx);
@ -148,6 +150,11 @@ on an I<int> value instead of a I<uint64_t> value.
=item *
CRYPTO_atomic_store_int() works identically to CRYPTO_atomic_store() but
operates on an I<int> value instead of a I<uint64_t> value.
=item *
OSSL_set_max_threads() sets the maximum number of threads to be used by the
thread pool. If the argument is 0, thread pooling is disabled. OpenSSL will
not create any threads and existing threads in the thread pool will be torn
@ -273,6 +280,8 @@ OSSL_get_thread_support_flags() were added in OpenSSL 3.2.
CRYPTO_atomic_store(), CRYPTO_atomic_add64(), CRYPTO_atomic_and()
were added in OpenSSL 3.4.
CRYPTO_atomic_store_int() was added in OpenSSL 4.0.
=head1 COPYRIGHT
Copyright 2000-2024 The OpenSSL Project Authors. All Rights Reserved.

1
include/openssl/crypto.h.in
View file

@ -97,6 +97,7 @@ int CRYPTO_atomic_or(uint64_t *val, uint64_t op, uint64_t *ret,
int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock);
int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock);
int CRYPTO_atomic_store(uint64_t *dst, uint64_t val, CRYPTO_RWLOCK *lock);
int CRYPTO_atomic_store_int(int *dst, int val, CRYPTO_RWLOCK *lock);
/* No longer needed, so this is a no-op */
#define OPENSSL_malloc_init() \

26
providers/fips/fipsprov.c
View file

@ -1354,12 +1354,16 @@ static int FIPS_kat_deferred(OSSL_LIB_CTX *libctx, self_test_id_t id)
if (CRYPTO_THREAD_get_local_ex(CRYPTO_THREAD_LOCAL_FIPS_DEFERRED_KEY,
libctx)
!= NULL) {
enum st_test_state state;
/*
* record this test as invoked by the original test, for marking
* it later as also satisfied
*/
if (st_all_tests[id].state == SELF_TEST_STATE_DEFER)
st_all_tests[id].state = SELF_TEST_STATE_IMPLICIT;
if (!ossl_get_self_test_state(id, &state))
return 0;
if (state == SELF_TEST_STATE_DEFER)
/* ignore errors, worst case we do additional testing */
ossl_set_self_test_state(id, SELF_TEST_STATE_IMPLICIT);
/*
* A self test is in progress for this thread so we let this
* thread continue and perform the test while all other
@ -1373,6 +1377,7 @@ static int FIPS_kat_deferred(OSSL_LIB_CTX *libctx, self_test_id_t id)
bool unset_key = false;
OSSL_CALLBACK *cb = NULL;
void *cb_arg = NULL;
enum st_test_state state;
/*
* check again as another thread may have just performed this
@ -1381,7 +1386,9 @@ static int FIPS_kat_deferred(OSSL_LIB_CTX *libctx, self_test_id_t id)
* deferred testing is only valid when SELF_TEST_post
* marks tests with SELF_TEST_STATE_DEFER, under lock.
*/
switch (st_all_tests[id].state) {
if (!ossl_get_self_test_state(id, &state))
goto done;
switch (state) {
case SELF_TEST_STATE_DEFER:
break;
case SELF_TEST_STATE_PASSED:
@ -1456,6 +1463,7 @@ static void deferred_test_error(int category)
int ossl_deferred_self_test(OSSL_LIB_CTX *libctx, self_test_id_t id)
{
enum st_test_state state;
int ret;
if (id >= ST_ID_MAX) {
@ -1464,7 +1472,11 @@ int ossl_deferred_self_test(OSSL_LIB_CTX *libctx, self_test_id_t id)
}
/* return immediately if the test is marked as passed */
if (st_all_tests[id].state == SELF_TEST_STATE_PASSED)
if (!ossl_get_self_test_state(id, &state)) {
ossl_set_error_state(NULL);
return 0;
}
if (state == SELF_TEST_STATE_PASSED)
return 1;
/*
@ -1475,7 +1487,11 @@ int ossl_deferred_self_test(OSSL_LIB_CTX *libctx, self_test_id_t id)
* in FIPS_kat_deferred() so this race is of no real consequence.
*/
ret = FIPS_kat_deferred(libctx, id);
if (!ret || st_all_tests[id].state == SELF_TEST_STATE_FAILED)
if (!ossl_get_self_test_state(id, &state)) {
ossl_set_error_state(NULL);
return 0;
}
if (!ret || state == SELF_TEST_STATE_FAILED)
deferred_test_error(st_all_tests[id].category);
return ret;
}

42
providers/fips/self_test.c
View file

@ -69,6 +69,27 @@ DEFINE_RUN_ONCE_STATIC(do_fips_self_test_init)
return self_test_lock != NULL;
}
static CRYPTO_RWLOCK *self_test_states_lock = NULL;
static CRYPTO_ONCE fips_self_test_states_lock_init = CRYPTO_ONCE_STATIC_INIT;
DEFINE_RUN_ONCE_STATIC(do_fips_self_test_states_lock_init)
{
self_test_states_lock = CRYPTO_THREAD_lock_new();
return self_test_states_lock != NULL;
}
int ossl_get_self_test_state(self_test_id_t id, enum st_test_state *state)
{
return CRYPTO_atomic_load_int((int *)&st_all_tests[id].state, (int *)state,
self_test_states_lock);
}
int ossl_set_self_test_state(self_test_id_t id, enum st_test_state state)
{
return CRYPTO_atomic_store_int((int *)&st_all_tests[id].state, state,
self_test_states_lock);
}
/*
* Declarations for the DEP entry/exit points.
* Ones not required or incorrect need to be undefined or redefined respectively.
@ -272,6 +293,9 @@ int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, void *fips_global,
if (!RUN_ONCE(&fips_self_test_init, do_fips_self_test_init))
return 0;
if (!RUN_ONCE(&fips_self_test_states_lock_init, do_fips_self_test_states_lock_init))
return 0;
loclstate = tsan_load(&FIPS_state);
if (loclstate == FIPS_STATE_RUNNING) {
@ -338,8 +362,17 @@ int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, void *fips_global,
&& strcmp(st->defer_tests, "1") == 0) {
/* Mark all non executed tests as deferred */
for (int i = 0; i < ST_ID_MAX; i++) {
if (st_all_tests[i].state == SELF_TEST_STATE_INIT)
st_all_tests[i].state = SELF_TEST_STATE_DEFER;
enum st_test_state state;
if (!ossl_get_self_test_state(i, &state)) {
errored = 1;
goto locked_end;
}
if (state == SELF_TEST_STATE_INIT) {
if (!ossl_set_self_test_state(i, SELF_TEST_STATE_DEFER)) {
errored = 1;
goto locked_end;
}
}
}
}
@ -347,7 +380,10 @@ int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, void *fips_global,
/* ensure all states are cleared so all tests are forcibly
* repeated */
for (int i = 0; i < ST_ID_MAX; i++) {
st_all_tests[i].state = SELF_TEST_STATE_INIT;
if (!ossl_set_self_test_state(i, SELF_TEST_STATE_INIT)) {
errored = 1;
goto locked_end;
}
}
}

2
providers/fips/self_test.h
View file

@ -178,3 +178,5 @@ typedef struct self_test_st {
} ST_DEFINITION;
extern ST_DEFINITION st_all_tests[ST_ID_MAX];
int ossl_get_self_test_state(self_test_id_t id, enum st_test_state *state);
int ossl_set_self_test_state(self_test_id_t id, enum st_test_state state);

62
providers/fips/self_test_kats.c
View file

@ -1131,10 +1131,11 @@ static int SELF_TEST_kats_single(OSSL_SELF_TEST *st, OSSL_LIB_CTX *libctx,
ret = 0;
break;
}
if (ret)
st_all_tests[id].state = SELF_TEST_STATE_PASSED;
else {
st_all_tests[id].state = SELF_TEST_STATE_FAILED;
if (ret) {
if (!ossl_set_self_test_state(id, SELF_TEST_STATE_PASSED))
return 0;
} else {
ossl_set_self_test_state(id, SELF_TEST_STATE_FAILED);
ERR_raise(ERR_LIB_PROV, PROV_R_SELF_TEST_KAT_FAILURE);
}
@ -1162,6 +1163,7 @@ int SELF_TEST_kats_execute(OSSL_SELF_TEST *st, OSSL_LIB_CTX *libctx,
self_test_id_t id, int switch_rand)
{
EVP_RAND_CTX *saved_rand = NULL;
enum st_test_state state;
int ret;
if (id >= ST_ID_MAX || st_all_tests[id].id != id) {
@ -1173,7 +1175,9 @@ int SELF_TEST_kats_execute(OSSL_SELF_TEST *st, OSSL_LIB_CTX *libctx,
* Dependency chains may cause a test to be referenced multiple times,
* immediately return if not in initial state.
*/
switch (st_all_tests[id].state) {
if (!ossl_get_self_test_state(id, &state))
return 0;
switch (state) {
case SELF_TEST_STATE_INIT:
case SELF_TEST_STATE_DEFER:
break;
@ -1201,7 +1205,8 @@ int SELF_TEST_kats_execute(OSSL_SELF_TEST *st, OSSL_LIB_CTX *libctx,
}
/* Mark test as in progress */
st_all_tests[id].state = SELF_TEST_STATE_IN_PROGRESS;
if (!ossl_set_self_test_state(id, SELF_TEST_STATE_IN_PROGRESS))
return 0;
/* check if there are dependent tests to run */
if (st_all_tests[id].depends_on) {
@ -1212,7 +1217,9 @@ int SELF_TEST_kats_execute(OSSL_SELF_TEST *st, OSSL_LIB_CTX *libctx,
}
/* may have already been run through dependency chains */
switch (st_all_tests[id].state) {
if (!ossl_get_self_test_state(id, &state))
return 0;
switch (state) {
case SELF_TEST_STATE_IN_PROGRESS:
ret = SELF_TEST_kats_single(st, libctx, id);
break;
@ -1221,7 +1228,7 @@ int SELF_TEST_kats_execute(OSSL_SELF_TEST *st, OSSL_LIB_CTX *libctx,
break;
default:
/* ensure all states are set to failed if we get here */
st_all_tests[id].state = SELF_TEST_STATE_FAILED;
ossl_set_self_test_state(id, SELF_TEST_STATE_FAILED);
ret = 0;
}
@ -1230,21 +1237,33 @@ int SELF_TEST_kats_execute(OSSL_SELF_TEST *st, OSSL_LIB_CTX *libctx,
* ensure they are all executed as well otherwise we could not
* mark it as passed.
*/
if (st_all_tests[id].state == SELF_TEST_STATE_PASSED)
for (int i = 0; i < ST_ID_MAX; i++)
if (st_all_tests[i].state == SELF_TEST_STATE_IMPLICIT
if (!ossl_get_self_test_state(id, &state))
return 0;
if (state == SELF_TEST_STATE_PASSED)
for (int i = 0; i < ST_ID_MAX; i++) {
enum st_test_state istate;
if (!ossl_get_self_test_state(i, &istate))
return 0;
if (istate == SELF_TEST_STATE_IMPLICIT
&& st_all_tests[i].depends_on != NULL)
if (!(ret = SELF_TEST_kat_deps(st, libctx, &st_all_tests[i])))
break;
}
done:
/*
* now mark (pass or fail) all the algorithm tests that have been marked
* by this test implicitly tested.
*/
for (int i = 0; i < ST_ID_MAX; i++)
if (st_all_tests[i].state == SELF_TEST_STATE_IMPLICIT)
st_all_tests[i].state = st_all_tests[id].state;
if (!ossl_get_self_test_state(id, &state))
return 0;
for (int i = 0; i < ST_ID_MAX; i++) {
enum st_test_state istate;
if (!ossl_get_self_test_state(i, &istate))
return 0;
if (istate == SELF_TEST_STATE_IMPLICIT)
ossl_set_self_test_state(i, state);
}
if (switch_rand) {
RAND_set0_private(libctx, saved_rand);
@ -1277,10 +1296,14 @@ int SELF_TEST_kats(OSSL_SELF_TEST *st, OSSL_LIB_CTX *libctx)
return 0;
}
for (i = 0; i < ST_ID_MAX; i++)
if (st_all_tests[i].state == SELF_TEST_STATE_INIT)
for (i = 0; i < ST_ID_MAX; i++) {
enum st_test_state state;
if (!ossl_get_self_test_state(i, &state))
return 0;
if (state == SELF_TEST_STATE_INIT)
if (!SELF_TEST_kats_execute(st, libctx, i, 0))
ret = 0;
}
RAND_set0_private(libctx, saved_rand);
/* The above call will cause main_rand to be freed */
@ -1290,10 +1313,15 @@ int SELF_TEST_kats(OSSL_SELF_TEST *st, OSSL_LIB_CTX *libctx)
int ossl_self_test_in_progress(self_test_id_t id)
{
enum st_test_state state;
if (id >= ST_ID_MAX)
return 0;
if (st_all_tests[id].state == SELF_TEST_STATE_IN_PROGRESS)
if (!ossl_get_self_test_state(id, &state))
return 0;
if (state == SELF_TEST_STATE_IN_PROGRESS)
return 1;
return 0;
}

1
util/libcrypto.num
View file

@ -3521,6 +3521,7 @@ CRYPTO_atomic_or ? 4_0_0 EXIST::FUNCTION:
CRYPTO_atomic_load ? 4_0_0 EXIST::FUNCTION:
CRYPTO_atomic_load_int ? 4_0_0 EXIST::FUNCTION:
CRYPTO_atomic_store ? 4_0_0 EXIST::FUNCTION:
CRYPTO_atomic_store_int ? 4_0_0 EXIST::FUNCTION:
OPENSSL_strlcpy ? 4_0_0 EXIST::FUNCTION:
OPENSSL_strlcat ? 4_0_0 EXIST::FUNCTION:
OPENSSL_strnlen ? 4_0_0 EXIST::FUNCTION: