Fix failure checking on rcu_read_lock

during memfail testing:
https://github.com/openssl/openssl/actions/runs/16794088536/job/47561223902

We get lots of test failures in ossl_rcu_read_lock.  This occurs
because we have a few cases in the read lock path that attempt mallocs,
which, if they fail, trigger an assert or a silent failure, which isn't
really appropriate.  We should instead fail gracefully, by informing the
caller that the lock failed, like we do for CRYPTO_THREAD_read_lock.

Fortunately, these are all internal apis, so we can convert
ossl_rcu_read_lock to return an int indicating success/failure, and fail
gracefully during the test, rather than hitting an assert abort.

Fixes openssl/project#1315

Reviewed-by: Paul Yang <paulyang.inf@gmail.com>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/28195)
This commit is contained in:
Neil Horman 2025-08-07 09:50:58 -04:00
parent 7f780be216
commit 036a46d2a4
10 changed files with 49 additions and 20 deletions

View File

@ -411,7 +411,9 @@ static CONF_MODULE *module_find(const char *name)
if (!RUN_ONCE(&init_module_list_lock, do_init_module_list_lock))
return NULL;
ossl_rcu_read_lock(module_list_lock);
if (!ossl_rcu_read_lock(module_list_lock))
return NULL;
mods = ossl_rcu_deref(&supported_modules);
for (i = 0; i < sk_CONF_MODULE_num(mods); i++) {

View File

@ -233,9 +233,9 @@ err:
return NULL;
}
void ossl_ht_read_lock(HT *htable)
int ossl_ht_read_lock(HT *htable)
{
ossl_rcu_read_lock(htable->lock);
return ossl_rcu_read_lock(htable->lock);
}
void ossl_ht_read_unlock(HT *htable)

View File

@ -37,9 +37,9 @@ void ossl_rcu_lock_free(CRYPTO_RCU_LOCK *lock)
OPENSSL_free(lock);
}
void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
int ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
{
return;
return 1;
}
void ossl_rcu_write_lock(CRYPTO_RCU_LOCK *lock)

View File

@ -328,7 +328,7 @@ static void ossl_rcu_free_local_data(void *arg)
OPENSSL_free(data);
}
void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
int ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
{
struct rcu_thr_data *data;
int i, available_qp = -1;
@ -341,9 +341,18 @@ void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
if (data == NULL) {
data = OPENSSL_zalloc(sizeof(*data));
OPENSSL_assert(data != NULL);
CRYPTO_THREAD_set_local_ex(CRYPTO_THREAD_LOCAL_RCU_KEY, lock->ctx, data);
ossl_init_thread_start(NULL, lock->ctx, ossl_rcu_free_local_data);
if (data == NULL)
return 0;
if (!CRYPTO_THREAD_set_local_ex(CRYPTO_THREAD_LOCAL_RCU_KEY, lock->ctx, data)) {
OPENSSL_free(data);
return 0;
}
if (!ossl_init_thread_start(NULL, lock->ctx, ossl_rcu_free_local_data)) {
OPENSSL_free(data);
CRYPTO_THREAD_set_local_ex(CRYPTO_THREAD_LOCAL_RCU_KEY, lock->ctx, NULL);
return 0;
}
}
for (i = 0; i < MAX_QPS; i++) {
@ -352,7 +361,7 @@ void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
/* If we have a hold on this lock already, we're good */
if (data->thread_qps[i].lock == lock) {
data->thread_qps[i].depth++;
return;
return 1;
}
}
@ -364,6 +373,7 @@ void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
data->thread_qps[available_qp].qp = get_hold_current_qp(lock);
data->thread_qps[available_qp].depth = 1;
data->thread_qps[available_qp].lock = lock;
return 1;
}
void ossl_rcu_read_unlock(CRYPTO_RCU_LOCK *lock)

View File

@ -228,7 +228,7 @@ static void ossl_rcu_free_local_data(void *arg)
OPENSSL_free(data);
}
void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
int ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
{
struct rcu_thr_data *data;
int i;
@ -242,9 +242,17 @@ void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
if (data == NULL) {
data = OPENSSL_zalloc(sizeof(*data));
OPENSSL_assert(data != NULL);
CRYPTO_THREAD_set_local_ex(CRYPTO_THREAD_LOCAL_RCU_KEY, lock->ctx, data);
ossl_init_thread_start(NULL, lock->ctx, ossl_rcu_free_local_data);
if (data == NULL)
return 0;
if (!CRYPTO_THREAD_set_local_ex(CRYPTO_THREAD_LOCAL_RCU_KEY, lock->ctx, data)) {
OPENSSL_free(data);
return 0;
}
if (!ossl_init_thread_start(NULL, lock->ctx, ossl_rcu_free_local_data)) {
OPENSSL_free(data);
CRYPTO_THREAD_set_local_ex(CRYPTO_THREAD_LOCAL_RCU_KEY, lock->ctx, NULL);
return 0;
}
}
for (i = 0; i < MAX_QPS; i++) {
@ -252,7 +260,7 @@ void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
available_qp = i;
/* If we have a hold on this lock already, we're good */
if (data->thread_qps[i].lock == lock)
return;
return 1;
}
/*
@ -263,6 +271,7 @@ void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
data->thread_qps[available_qp].qp = get_hold_current_qp(lock);
data->thread_qps[available_qp].depth = 1;
data->thread_qps[available_qp].lock = lock;
return 1;
}
void ossl_rcu_write_lock(CRYPTO_RCU_LOCK *lock)

View File

@ -276,7 +276,8 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
HT_SET_KEY_FIELD(&key, fuzzkey, keyval);
/* lock the table for reading */
ossl_ht_read_lock(fuzzer_table);
if (!ossl_ht_read_lock(fuzzer_table))
return 0;
/*
* If the value to find is not already allocated

View File

@ -280,7 +280,7 @@ void ossl_ht_free(HT *htable);
/*
* Lock the table for reading
*/
void ossl_ht_read_lock(HT *htable);
int ossl_ht_read_lock(HT *htable);
/*
* Lock the table for writing

View File

@ -19,7 +19,7 @@ typedef struct rcu_lock_st CRYPTO_RCU_LOCK;
CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers, OSSL_LIB_CTX *ctx);
void ossl_rcu_lock_free(CRYPTO_RCU_LOCK *lock);
void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock);
int ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock);
void ossl_rcu_write_lock(CRYPTO_RCU_LOCK *lock);
void ossl_rcu_write_unlock(CRYPTO_RCU_LOCK *lock);
void ossl_rcu_read_unlock(CRYPTO_RCU_LOCK *lock);

View File

@ -573,7 +573,8 @@ static void do_mt_hash_work(void)
}
switch(behavior) {
case DO_LOOKUP:
ossl_ht_read_lock(m_ht);
if (!ossl_ht_read_lock(m_ht))
break;
m = ossl_ht_mt_TEST_MT_ENTRY_get(m_ht, TO_HT_KEY(&key), &v);
if (m != NULL && m != expected_m) {
worker_exits[num] = "Read unexpected value from hashtable";

View File

@ -374,7 +374,13 @@ static void reader_fn(int *iterations)
CRYPTO_atomic_add(&writer1_done, 0, &lw1, atomiclock);
CRYPTO_atomic_add(&writer2_done, 0, &lw2, atomiclock);
count++;
ossl_rcu_read_lock(rcu_lock);
if (!ossl_rcu_read_lock(rcu_lock)) {
TEST_info("rcu torture read lock failed");
rcu_torture_result = 0;
*iterations = count;
return;
}
valp = ossl_rcu_deref(&writer_ptr);
val = (valp == NULL) ? 0 : *valp;