Skip to content

Commit

Permalink
Make rcu_thread_key context-aware
Browse files Browse the repository at this point in the history
Currently, rcu has a global bit of data, the CRYPTO_THREAD_LOCAL object
to store per thread data.  This works in some cases, but fails in FIPS,
becuase it contains its own copy of the global key.

So
1) Make the rcu_thr_key a per-context variable, and force
   ossl_rcu_lock_new to be context aware

2) Store a pointer to the context in the lock object

3) Use the context to get the global thread key on read/write lock

4) Use ossl_thread_start_init to properly register a cleanup on thread
   exit

5) Fix up missed calls to OSSL_thread_stop() in our tests

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #24162)
  • Loading branch information
nhorman committed Apr 19, 2024
1 parent faa4a10 commit 24d16d3
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 68 deletions.
2 changes: 1 addition & 1 deletion crypto/conf/conf_mod.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ static void module_lists_free(void)

DEFINE_RUN_ONCE_STATIC(do_init_module_list_lock)
{
module_list_lock = ossl_rcu_lock_new(1);
module_list_lock = ossl_rcu_lock_new(1, NULL);
if (module_list_lock == NULL) {
ERR_raise(ERR_LIB_CONF, ERR_R_CRYPTO_LIB);
return 0;
Expand Down
16 changes: 15 additions & 1 deletion crypto/context.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ struct ossl_lib_ctx_st {
void *global_properties;
void *drbg;
void *drbg_nonce;
CRYPTO_THREAD_LOCAL rcu_local_key;
#ifndef FIPS_MODULE
void *provider_conf;
void *bio_core;
Expand Down Expand Up @@ -81,9 +82,12 @@ static int context_init(OSSL_LIB_CTX *ctx)
{
int exdata_done = 0;

if (!CRYPTO_THREAD_init_local(&ctx->rcu_local_key, NULL))
return 0;

ctx->lock = CRYPTO_THREAD_lock_new();
if (ctx->lock == NULL)
return 0;
goto err;

ctx->rand_crngt_lock = CRYPTO_THREAD_lock_new();
if (ctx->rand_crngt_lock == NULL)
Expand Down Expand Up @@ -209,6 +213,7 @@ static int context_init(OSSL_LIB_CTX *ctx)

CRYPTO_THREAD_lock_free(ctx->rand_crngt_lock);
CRYPTO_THREAD_lock_free(ctx->lock);
CRYPTO_THREAD_cleanup_local(&ctx->rcu_local_key);
memset(ctx, '\0', sizeof(*ctx));
return 0;
}
Expand Down Expand Up @@ -355,6 +360,7 @@ static int context_deinit(OSSL_LIB_CTX *ctx)
CRYPTO_THREAD_lock_free(ctx->lock);
ctx->rand_crngt_lock = NULL;
ctx->lock = NULL;
CRYPTO_THREAD_cleanup_local(&ctx->rcu_local_key);
return 1;
}

Expand Down Expand Up @@ -652,3 +658,11 @@ const char *ossl_lib_ctx_get_descriptor(OSSL_LIB_CTX *libctx)
return "Non-default library context";
#endif
}

CRYPTO_THREAD_LOCAL *ossl_lib_ctx_get_rcukey(OSSL_LIB_CTX *libctx)
{
libctx = ossl_lib_ctx_get_concrete(libctx);
if (libctx == NULL)
return NULL;
return &libctx->rcu_local_key;
}
2 changes: 1 addition & 1 deletion crypto/threads_none.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct rcu_lock_st {
struct rcu_cb_item *cb_items;
};

CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers)
CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers, OSSL_LIB_CTX *ctx)
{
struct rcu_lock_st *lock;

Expand Down
53 changes: 23 additions & 30 deletions crypto/threads_pthread.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,6 @@ static inline uint64_t fallback_atomic_or_fetch(uint64_t *p, uint64_t m)
# define ATOMIC_OR_FETCH(p, v, o) fallback_atomic_or_fetch(p, v)
# endif

static CRYPTO_THREAD_LOCAL rcu_thr_key;

/*
* users is broken up into 2 parts
* bits 0-15 current readers
Expand Down Expand Up @@ -300,6 +298,9 @@ struct rcu_lock_st {
/* Callbacks to call for next ossl_synchronize_rcu */
struct rcu_cb_item *cb_items;

/* The context we are being created against */
OSSL_LIB_CTX *ctx;

/* rcu generation counter for in-order retirement */
uint32_t id_ctr;

Expand Down Expand Up @@ -337,24 +338,6 @@ struct rcu_lock_st {
pthread_cond_t prior_signal;
};

/*
* Called on thread exit to free the pthread key
* associated with this thread, if any
*/
static void free_rcu_thr_data(void *ptr)
{
struct rcu_thr_data *data =
(struct rcu_thr_data *)CRYPTO_THREAD_get_local(&rcu_thr_key);

OPENSSL_free(data);
CRYPTO_THREAD_set_local(&rcu_thr_key, NULL);
}

static void ossl_rcu_init(void)
{
CRYPTO_THREAD_init_local(&rcu_thr_key, NULL);
}

/* Read side acquisition of the current qp */
static struct rcu_qp *get_hold_current_qp(struct rcu_lock_st *lock)
{
Expand Down Expand Up @@ -403,22 +386,31 @@ static struct rcu_qp *get_hold_current_qp(struct rcu_lock_st *lock)
return &lock->qp_group[qp_idx];
}

static void ossl_rcu_free_local_data(void *arg)
{
OSSL_LIB_CTX *ctx = arg;
CRYPTO_THREAD_LOCAL *lkey = ossl_lib_ctx_get_rcukey(ctx);
struct rcu_thr_data *data = CRYPTO_THREAD_get_local(lkey);
OPENSSL_free(data);
}

void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
{
struct rcu_thr_data *data;
int i, available_qp = -1;
CRYPTO_THREAD_LOCAL *lkey = ossl_lib_ctx_get_rcukey(lock->ctx);

/*
* we're going to access current_qp here so ask the
* processor to fetch it
*/
data = CRYPTO_THREAD_get_local(&rcu_thr_key);
data = CRYPTO_THREAD_get_local(lkey);

if (data == NULL) {
data = OPENSSL_zalloc(sizeof(*data));
OPENSSL_assert(data != NULL);
CRYPTO_THREAD_set_local(&rcu_thr_key, data);
ossl_init_thread_start(NULL, NULL, free_rcu_thr_data);
CRYPTO_THREAD_set_local(lkey, data);
ossl_init_thread_start(NULL, lock->ctx, ossl_rcu_free_local_data);
}

for (i = 0; i < MAX_QPS; i++) {
Expand All @@ -444,7 +436,8 @@ void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
void ossl_rcu_read_unlock(CRYPTO_RCU_LOCK *lock)
{
int i;
struct rcu_thr_data *data = CRYPTO_THREAD_get_local(&rcu_thr_key);
CRYPTO_THREAD_LOCAL *lkey = ossl_lib_ctx_get_rcukey(lock->ctx);
struct rcu_thr_data *data = CRYPTO_THREAD_get_local(lkey);
uint64_t ret;

assert(data != NULL);
Expand Down Expand Up @@ -637,22 +630,22 @@ void ossl_rcu_assign_uptr(void **p, void **v)
ATOMIC_STORE(pvoid, p, v, __ATOMIC_RELEASE);
}

static CRYPTO_ONCE rcu_init_once = CRYPTO_ONCE_STATIC_INIT;

CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers)
CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers, OSSL_LIB_CTX *ctx)
{
struct rcu_lock_st *new;

if (!CRYPTO_THREAD_run_once(&rcu_init_once, ossl_rcu_init))
return NULL;

if (num_writers < 1)
num_writers = 1;

ctx = ossl_lib_ctx_get_concrete(ctx);
if (ctx == NULL)
return 0;

new = OPENSSL_zalloc(sizeof(*new));
if (new == NULL)
return NULL;

new->ctx = ctx;
pthread_mutex_init(&new->write_lock, NULL);
pthread_mutex_init(&new->prior_lock, NULL);
pthread_mutex_init(&new->alloc_lock, NULL);
Expand Down
52 changes: 21 additions & 31 deletions crypto/threads_win.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ typedef struct {
} CRYPTO_win_rwlock;
# endif

static CRYPTO_THREAD_LOCAL rcu_thr_key;

# define READER_SHIFT 0
# define ID_SHIFT 32
# define READER_SIZE 32
Expand Down Expand Up @@ -92,6 +90,7 @@ struct rcu_thr_data {
*/
struct rcu_lock_st {
struct rcu_cb_item *cb_items;
OSSL_LIB_CTX *ctx;
uint32_t id_ctr;
struct rcu_qp *qp_group;
size_t group_count;
Expand All @@ -106,26 +105,6 @@ struct rcu_lock_st {
CRYPTO_CONDVAR *prior_signal;
};

/*
* Called on thread exit to free the pthread key
* associated with this thread, if any
*/
static void free_rcu_thr_data(void *ptr)
{
struct rcu_thr_data *data =
(struct rcu_thr_data *)CRYPTO_THREAD_get_local(&rcu_thr_key);

OPENSSL_free(data);
CRYPTO_THREAD_set_local(&rcu_thr_key, NULL);
}


static void ossl_rcu_init(void)
{
CRYPTO_THREAD_init_local(&rcu_thr_key, NULL);
ossl_init_thread_start(NULL, NULL, free_rcu_thr_data);
}

static struct rcu_qp *allocate_new_qp_group(struct rcu_lock_st *lock,
int count)
{
Expand All @@ -136,23 +115,23 @@ static struct rcu_qp *allocate_new_qp_group(struct rcu_lock_st *lock,
return new;
}

static CRYPTO_ONCE rcu_init_once = CRYPTO_ONCE_STATIC_INIT;

CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers)
CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers, OSSL_LIB_CTX *ctx)
{
struct rcu_lock_st *new;

if (!CRYPTO_THREAD_run_once(&rcu_init_once, ossl_rcu_init))
return NULL;

if (num_writers < 1)
num_writers = 1;

ctx = ossl_lib_ctx_get_concrete(ctx);
if (ctx == NULL)
return 0;

new = OPENSSL_zalloc(sizeof(*new));

if (new == NULL)
return NULL;

new->ctx = ctx;
new->write_lock = ossl_crypto_mutex_new();
new->alloc_signal = ossl_crypto_condvar_new();
new->prior_signal = ossl_crypto_condvar_new();
Expand Down Expand Up @@ -205,22 +184,32 @@ static inline struct rcu_qp *get_hold_current_qp(CRYPTO_RCU_LOCK *lock)
return &lock->qp_group[qp_idx];
}

static void ossl_rcu_free_local_data(void *arg)
{
OSSL_LIB_CTX *ctx = arg;
CRYPTO_THREAD_LOCAL *lkey = ossl_lib_ctx_get_rcukey(ctx);
struct rcu_thr_data *data = CRYPTO_THREAD_get_local(lkey);
OPENSSL_free(data);
}

void ossl_rcu_read_lock(CRYPTO_RCU_LOCK *lock)
{
struct rcu_thr_data *data;
int i;
int available_qp = -1;
CRYPTO_THREAD_LOCAL *lkey = ossl_lib_ctx_get_rcukey(lock->ctx);

/*
* we're going to access current_qp here so ask the
* processor to fetch it
*/
data = CRYPTO_THREAD_get_local(&rcu_thr_key);
data = CRYPTO_THREAD_get_local(lkey);

if (data == NULL) {
data = OPENSSL_zalloc(sizeof(*data));
OPENSSL_assert(data != NULL);
CRYPTO_THREAD_set_local(&rcu_thr_key, data);
CRYPTO_THREAD_set_local(lkey, data);
ossl_init_thread_start(NULL, lock->ctx, ossl_rcu_free_local_data);
}

for (i = 0; i < MAX_QPS; i++) {
Expand Down Expand Up @@ -253,7 +242,8 @@ void ossl_rcu_write_unlock(CRYPTO_RCU_LOCK *lock)

void ossl_rcu_read_unlock(CRYPTO_RCU_LOCK *lock)
{
struct rcu_thr_data *data = CRYPTO_THREAD_get_local(&rcu_thr_key);
CRYPTO_THREAD_LOCAL *lkey = ossl_lib_ctx_get_rcukey(lock->ctx);
struct rcu_thr_data *data = CRYPTO_THREAD_get_local(lkey);
int i;
LONG64 ret;

Expand Down
5 changes: 3 additions & 2 deletions doc/internal/man3/ossl_rcu_lock_new.pod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ ossl_rcu_assign_uptr

=head1 SYNOPSIS

CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers);
CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers, OSSL_LIB_CTX *ctx);
void 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);
Expand Down Expand Up @@ -65,7 +65,8 @@ ossl_rcu_lock_new() allocates a new RCU lock. The I<num_writers> param
indicates the number of write side threads which may execute
ossl_synchronize_rcu() in parallel. The value must be at least 1, but may be
larger to obtain increased write side throughput at the cost of additional
internal memory usage. A value of 1 is generally recommended.
internal memory usage. A value of 1 is generally recommended. The I<ctx>
parameter references the library context in which the lock is allocated.

=item *

Expand Down
1 change: 1 addition & 0 deletions include/internal/cryptlib.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ void ossl_lib_ctx_default_deinit(void);
OSSL_EX_DATA_GLOBAL *ossl_lib_ctx_get_ex_data_global(OSSL_LIB_CTX *ctx);

const char *ossl_lib_ctx_get_descriptor(OSSL_LIB_CTX *libctx);
CRYPTO_THREAD_LOCAL *ossl_lib_ctx_get_rcukey(OSSL_LIB_CTX *libctx);

OSSL_LIB_CTX *ossl_crypto_ex_data_get_ossl_lib_ctx(const CRYPTO_EX_DATA *ad);
int ossl_crypto_new_ex_data_ex(OSSL_LIB_CTX *ctx, int class_index, void *obj,
Expand Down
4 changes: 3 additions & 1 deletion include/internal/rcu.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@
# define OPENSSL_RCU_H
# pragma once

#include "crypto/context.h"

typedef void (*rcu_cb_fn)(void *data);

typedef struct rcu_lock_st CRYPTO_RCU_LOCK;

CRYPTO_RCU_LOCK *ossl_rcu_lock_new(int num_writers);
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);
void ossl_rcu_write_lock(CRYPTO_RCU_LOCK *lock);
Expand Down
2 changes: 1 addition & 1 deletion test/threadstest.c
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ static int _torture_rcu(void)
writer2_done = 0;
rcu_torture_result = 1;

rcu_lock = ossl_rcu_lock_new(1);
rcu_lock = ossl_rcu_lock_new(1, NULL);

TEST_info("Staring rcu torture");
t1 = ossl_time_now();
Expand Down
1 change: 1 addition & 0 deletions test/threadstest.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ static void *thread_run(void *arg)
*(void **) (&f) = arg;

f();
OPENSSL_thread_stop();
return NULL;
}

Expand Down

0 comments on commit 24d16d3

Please sign in to comment.