Make rcu_thread_key context-aware

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

(cherry picked from commit 24d16d3a1915a06a2130385a87de9a37fc09c4b9)

Fixes #24581

Reviewed-by: Paul Dale <ppzgs1@gmail.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/24630)

diff --git a/crypto/conf/conf_mod.c b/crypto/conf/conf_mod.c
index ca873150125c96a23fc9d1b33686a9af96d1bb3d..6d3683d76fee125bff6eeecc5f2bf3949c2cc83c 100644 (file)
--- a/crypto/conf/conf_mod.c
+++ b/crypto/conf/conf_mod.c
@@ -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;

diff --git a/crypto/context.c b/crypto/context.c
index 33d52a964b136b0ca1d2135e46a99129d8b58cf8..3d55084d7bd80a5c6a1b16428f96165bf6cd16c6 100644 (file)
--- a/crypto/context.c
+++ b/crypto/context.c
@@ -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;
@@ -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)
@@ -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;
 }
@@ -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;
 }
 
@@ -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;
+}

diff --git a/crypto/threads_none.c b/crypto/threads_none.c
index c57c59bde87647d53c04f4db282a6dd7ab25bc90..47a7c01f26aa274b3e2a4ccc066d0486d1e4db00 100644 (file)
--- a/crypto/threads_none.c
+++ b/crypto/threads_none.c
@@ -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;
 

diff --git a/crypto/threads_pthread.c b/crypto/threads_pthread.c
index 92346e1689ac3b907528e60933ef37d5fed46c62..358882ce0a97b25e386f2c987bf26204eab6931b 100644 (file)
--- a/crypto/threads_pthread.c
+++ b/crypto/threads_pthread.c
@@ -244,8 +244,6 @@ static ossl_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
@@ -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;
 
@@ -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)
 {
@@ -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++) {
@@ -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);
@@ -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);

diff --git a/crypto/threads_win.c b/crypto/threads_win.c
index 64354dc42f43b63d4a2ffe6821b7bce1c9bac5a0..60c18d4313d9776207a5c5f62f741010d3591373 100644 (file)
--- a/crypto/threads_win.c
+++ b/crypto/threads_win.c
@@ -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 
@@ -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;
@@ -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)
 {
@@ -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();
@@ -205,22 +184,32 @@ static ossl_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++) {
@@ -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;
 

diff --git a/doc/internal/man3/ossl_rcu_lock_new.pod b/doc/internal/man3/ossl_rcu_lock_new.pod
index 7b82f6bbd040b0fae0af5f51a107692f8b3acf73..57b5e4d73d2ff8e3bfd56d449f0a109abe18360e 100644 (file)
--- a/doc/internal/man3/ossl_rcu_lock_new.pod
+++ b/doc/internal/man3/ossl_rcu_lock_new.pod
@@ -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);
@@ -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 *
 

diff --git a/include/internal/cryptlib.h b/include/internal/cryptlib.h
index 64851fd8ed340c13dc90cc931c310cf0c56ecb74..6c2ac47275ae99d8d56f0de7d22588aded1c735f 100644 (file)
--- a/include/internal/cryptlib.h
+++ b/include/internal/cryptlib.h
@@ -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,

diff --git a/include/internal/rcu.h b/include/internal/rcu.h
index 7716a1c7f22f2e0d496e1c68470b0b52801eb619..90160e8da71d948c82c89b50894213510b05e8ef 100644 (file)
--- a/include/internal/rcu.h
+++ b/include/internal/rcu.h
@@ -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);

diff --git a/test/threadstest.c b/test/threadstest.c
index 5b6565a45f74f871ae884cdf63e594e5e23da98d..5858fbdf82f348659632480ec76cb70beae04e7c 100644 (file)
--- a/test/threadstest.c
+++ b/test/threadstest.c
@@ -435,9 +435,7 @@ static int _torture_rcu(void)
     writer2_done = 0;
     rcu_torture_result = 1;
 
-    rcu_lock = ossl_rcu_lock_new(1);
-    if (!rcu_lock)
-        goto out;
+    rcu_lock = ossl_rcu_lock_new(1, NULL);
 
     TEST_info("Staring rcu torture");
     t1 = ossl_time_now();

diff --git a/test/threadstest.h b/test/threadstest.h
index 8bdedd7052aca3f1f33c3cdb77e76b1bfc57d074..3125cec3780eaf830a26e884d21581c0d6f7c656 100644 (file)
--- a/test/threadstest.h
+++ b/test/threadstest.h
@@ -65,6 +65,7 @@ static void *thread_run(void *arg)
     *(void **) (&f) = arg;
 
     f();
+    OPENSSL_thread_stop();
     return NULL;
 }