Provide an ability to deregister thread stop handlers

If a provider gets unloaded then any thread stop handlers that it had
registered will be left hanging. We should clean them up before tearing
down the provider.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9186)

diff --git a/crypto/async/async.c b/crypto/async/async.c
index bcb0030e52baeade2a1dab4c99680552f93c6b89..43b16a7b7c0288b85b9e9e2014ef6b0568700f76 100644 (file)
--- a/crypto/async/async.c
+++ b/crypto/async/async.c
@@ -36,7 +36,7 @@ static async_ctx *async_ctx_new(void)
 {
     async_ctx *nctx;
 
-    if (!ossl_init_thread_start(NULL, async_delete_thread_state))
+    if (!ossl_init_thread_start(NULL, NULL, async_delete_thread_state))
         return NULL;
 
     nctx = OPENSSL_malloc(sizeof(*nctx));
@@ -328,7 +328,7 @@ int ASYNC_init_thread(size_t max_size, size_t init_size)
     if (!OPENSSL_init_crypto(OPENSSL_INIT_ASYNC, NULL))
         return 0;
 
-    if (!ossl_init_thread_start(NULL, async_delete_thread_state))
+    if (!ossl_init_thread_start(NULL, NULL, async_delete_thread_state))
         return 0;
 
     pool = OPENSSL_zalloc(sizeof(*pool));

diff --git a/crypto/err/err.c b/crypto/err/err.c
index 9eb477ccda38ca42b617cc6a53b82112c455f9d2..8752c119775a6e2b946c9bc072ee7d708d244128 100644 (file)
--- a/crypto/err/err.c
+++ b/crypto/err/err.c
@@ -741,7 +741,7 @@ ERR_STATE *ERR_get_state(void)
             return NULL;
         }
 
-        if (!ossl_init_thread_start(NULL, err_delete_thread_state)
+        if (!ossl_init_thread_start(NULL, NULL, err_delete_thread_state)
                 || !CRYPTO_THREAD_set_local(&err_thread_local, state)) {
             ERR_STATE_free(state);
             CRYPTO_THREAD_set_local(&err_thread_local, NULL);

diff --git a/crypto/include/internal/cryptlib_int.h b/crypto/include/internal/cryptlib_int.h
index a69bdcd408cf314e8eb5efba55a49ff51067378c..673a004f0f6b8efca34392922b6f7971f0114c17 100644 (file)
--- a/crypto/include/internal/cryptlib_int.h
+++ b/crypto/include/internal/cryptlib_int.h
@@ -12,8 +12,9 @@
 
 /* This file is not scanned by mkdef.pl, whereas cryptlib.h is */
 
-int ossl_init_thread_start(void *arg,
+int ossl_init_thread_start(const void *index, void *arg,
                            OSSL_thread_stop_handler_fn handfn);
+int ossl_init_thread_deregister(void *index);
 int ossl_init_thread(void);
 void ossl_cleanup_thread(void);
 void ossl_ctx_thread_stop(void *arg);

diff --git a/crypto/init.c b/crypto/init.c
index 8755e2164f54a68123ea50224e45d431e225b158..d5f0ebd7b75d5f98ddc3a6859cb20fa2fdb940aa 100644 (file)
--- a/crypto/init.c
+++ b/crypto/init.c
@@ -428,8 +428,6 @@ void OPENSSL_cleanup(void)
         err_free_strings_int();
     }
 
-    ossl_cleanup_thread();
-
     /*
      * Note that cleanup order is important:
      * - rand_cleanup_int could call an ENGINE's RAND cleanup function so
@@ -457,6 +455,8 @@ void OPENSSL_cleanup(void)
     OSSL_TRACE(INIT, "OPENSSL_cleanup: openssl_ctx_default_deinit()\n");
     openssl_ctx_default_deinit();
 
+    ossl_cleanup_thread();
+
     OSSL_TRACE(INIT, "OPENSSL_cleanup: bio_cleanup()\n");
     bio_cleanup();
 

diff --git a/crypto/initthread.c b/crypto/initthread.c
index b4ee177c8f26595d73bd4a46ed8f30bd4b63301a..b398b05cd21c0c3fa137e47c7dae220c56ee135e 100644 (file)
--- a/crypto/initthread.c
+++ b/crypto/initthread.c
@@ -11,6 +11,7 @@
 #include <openssl/core_numbers.h>
 #include "internal/cryptlib_int.h"
 #include "internal/providercommon.h"
+#include "internal/thread_once.h"
 
 #ifdef FIPS_MODE
 /*
@@ -30,11 +31,52 @@ extern OSSL_core_thread_start_fn *c_thread_start;
 
 typedef struct thread_event_handler_st THREAD_EVENT_HANDLER;
 struct thread_event_handler_st {
+    const void *index;
     void *arg;
     OSSL_thread_stop_handler_fn handfn;
     THREAD_EVENT_HANDLER *next;
 };
 
+#ifndef FIPS_MODE
+DEFINE_SPECIAL_STACK_OF(THREAD_EVENT_HANDLER_PTR, THREAD_EVENT_HANDLER *)
+
+typedef struct global_tevent_register_st GLOBAL_TEVENT_REGISTER;
+struct global_tevent_register_st {
+    STACK_OF(THREAD_EVENT_HANDLER_PTR) *skhands;
+    CRYPTO_RWLOCK *lock;
+};
+
+static GLOBAL_TEVENT_REGISTER *glob_tevent_reg = NULL;
+
+static CRYPTO_ONCE tevent_register_runonce = CRYPTO_ONCE_STATIC_INIT;
+
+DEFINE_RUN_ONCE_STATIC(create_global_tevent_register)
+{
+    glob_tevent_reg = OPENSSL_zalloc(sizeof(*glob_tevent_reg));
+    if (glob_tevent_reg == NULL)
+        return 0;
+
+    glob_tevent_reg->skhands = sk_THREAD_EVENT_HANDLER_PTR_new_null();
+    glob_tevent_reg->lock = CRYPTO_THREAD_lock_new();
+    if (glob_tevent_reg->skhands == NULL || glob_tevent_reg->lock == NULL) {
+        sk_THREAD_EVENT_HANDLER_PTR_free(glob_tevent_reg->skhands);
+        CRYPTO_THREAD_lock_free(glob_tevent_reg->lock);
+        OPENSSL_free(glob_tevent_reg);
+        glob_tevent_reg = NULL;
+        return 0;
+    }
+
+    return 1;
+}
+
+static GLOBAL_TEVENT_REGISTER *get_global_tevent_register(void)
+{
+    if (!RUN_ONCE(&tevent_register_runonce, create_global_tevent_register))
+        return NULL;
+    return glob_tevent_reg;
+}
+#endif
+
 static void init_thread_stop(void *arg, THREAD_EVENT_HANDLER **hands);
 
 static THREAD_EVENT_HANDLER **
@@ -43,11 +85,41 @@ init_get_thread_local(CRYPTO_THREAD_LOCAL *local, int alloc, int keep)
     THREAD_EVENT_HANDLER **hands = CRYPTO_THREAD_get_local(local);
 
     if (alloc) {
-        if (hands == NULL
-            && (hands = OPENSSL_zalloc(sizeof(*hands))) != NULL
-            && !CRYPTO_THREAD_set_local(local, hands)) {
-            OPENSSL_free(hands);
-            return NULL;
+        if (hands == NULL) {
+#ifndef FIPS_MODE
+            GLOBAL_TEVENT_REGISTER *gtr;
+#endif
+
+            if ((hands = OPENSSL_zalloc(sizeof(*hands))) == NULL) {
+                OPENSSL_free(hands);
+                return NULL;
+            }
+
+#ifndef FIPS_MODE
+            /*
+             * The thread event handler is thread specific and is a linked
+             * list of all handler functions that should be called for the
+             * current thread. We also keep a global reference to that linked
+             * list, so that we can deregister handlers if necessary before all
+             * the threads are stopped.
+             */
+            gtr = get_global_tevent_register();
+            if (gtr == NULL) {
+                OPENSSL_free(hands);
+                return NULL;
+            }
+            CRYPTO_THREAD_write_lock(gtr->lock);
+            if (!sk_THREAD_EVENT_HANDLER_PTR_push(gtr->skhands, hands)) {
+                OPENSSL_free(hands);
+                CRYPTO_THREAD_unlock(gtr->lock);
+                return NULL;
+            }
+            CRYPTO_THREAD_unlock(gtr->lock);
+#endif
+            if (!CRYPTO_THREAD_set_local(local, hands)) {
+                OPENSSL_free(hands);
+                return NULL;
+            }
         }
     } else if (!keep) {
         CRYPTO_THREAD_set_local(local, NULL);
@@ -76,9 +148,33 @@ static union {
     CRYPTO_THREAD_LOCAL value;
 } destructor_key = { -1 };
 
+static void init_thread_remove_handlers(THREAD_EVENT_HANDLER **handsin)
+{
+    GLOBAL_TEVENT_REGISTER *gtr;
+    int i;
+
+    gtr = get_global_tevent_register();
+    if (gtr == NULL)
+        return;
+    CRYPTO_THREAD_write_lock(gtr->lock);
+    for (i = 0; i < sk_THREAD_EVENT_HANDLER_PTR_num(gtr->skhands); i++) {
+        THREAD_EVENT_HANDLER **hands
+            = sk_THREAD_EVENT_HANDLER_PTR_value(gtr->skhands, i);
+
+        if (hands == handsin) {
+            hands = sk_THREAD_EVENT_HANDLER_PTR_delete(gtr->skhands, i);
+            CRYPTO_THREAD_unlock(gtr->lock);
+            return;
+        }
+    }
+    CRYPTO_THREAD_unlock(gtr->lock);
+    return;
+}
+
 static void init_thread_destructor(void *hands)
 {
     init_thread_stop(NULL, (THREAD_EVENT_HANDLER **)hands);
+    init_thread_remove_handlers(hands);
     OPENSSL_free(hands);
 }
 
@@ -91,8 +187,11 @@ int ossl_init_thread(void)
     return 1;
 }
 
+static int init_thread_deregister(void *arg, int all);
+
 void ossl_cleanup_thread(void)
 {
+    init_thread_deregister(NULL, 1);
     CRYPTO_THREAD_cleanup_local(&destructor_key.value);
     destructor_key.sane = -1;
 }
@@ -114,6 +213,8 @@ void OPENSSL_thread_stop(void)
         THREAD_EVENT_HANDLER **hands
             = init_get_thread_local(&destructor_key.value, 0, 0);
         init_thread_stop(NULL, hands);
+
+        init_thread_remove_handlers(hands);
         OPENSSL_free(hands);
     }
 }
@@ -205,7 +306,8 @@ static void init_thread_stop(void *arg, THREAD_EVENT_HANDLER **hands)
     }
 }
 
-int ossl_init_thread_start(void *arg, OSSL_thread_stop_handler_fn handfn)
+int ossl_init_thread_start(const void *index, void *arg,
+                           OSSL_thread_stop_handler_fn handfn)
 {
     THREAD_EVENT_HANDLER **hands;
     THREAD_EVENT_HANDLER *hand;
@@ -252,8 +354,61 @@ int ossl_init_thread_start(void *arg, OSSL_thread_stop_handler_fn handfn)
 
     hand->handfn = handfn;
     hand->arg = arg;
+    hand->index = index;
     hand->next = *hands;
     *hands = hand;
 
     return 1;
 }
+
+#ifndef FIPS_MODE
+static int init_thread_deregister(void *index, int all)
+{
+    GLOBAL_TEVENT_REGISTER *gtr;
+    int i;
+
+    gtr = get_global_tevent_register();
+    if (!all)
+        CRYPTO_THREAD_write_lock(gtr->lock);
+    for (i = 0; i < sk_THREAD_EVENT_HANDLER_PTR_num(gtr->skhands); i++) {
+        THREAD_EVENT_HANDLER **hands
+            = sk_THREAD_EVENT_HANDLER_PTR_value(gtr->skhands, i);
+        THREAD_EVENT_HANDLER *curr = *hands, *prev = NULL, *tmp;
+
+        if (hands == NULL) {
+            if (!all)
+                CRYPTO_THREAD_unlock(gtr->lock);
+            return 0;
+        }
+        while (curr != NULL) {
+            if (all || curr->index == index) {
+                if (prev != NULL)
+                    prev->next = curr->next;
+                else
+                    *hands = curr->next;
+                tmp = curr;
+                curr = curr->next;
+                OPENSSL_free(tmp);
+                continue;
+            }
+            prev = curr;
+            curr = curr->next;
+        }
+        if (all)
+            OPENSSL_free(hands);
+    }
+    if (all) {
+        CRYPTO_THREAD_lock_free(gtr->lock);
+        sk_THREAD_EVENT_HANDLER_PTR_free(gtr->skhands);
+        OPENSSL_free(gtr);
+    } else {
+        CRYPTO_THREAD_unlock(gtr->lock);
+    }
+    return 1;
+}
+
+int ossl_init_thread_deregister(void *index)
+{
+    return init_thread_deregister(index, 0);
+}
+#endif

diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index 10948ce27173aed0b18d50c2c7a9cf8c29f3e219..274bdf94ba689fcdb4392925be59426ddab860ce 100644 (file)
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -269,6 +269,9 @@ void ossl_provider_free(OSSL_PROVIDER *prov)
          * When that happens, the provider is inactivated.
          */
         if (ref < 2 && prov->flag_initialized) {
+#ifndef FIPS_MODE
+            ossl_init_thread_deregister(prov);
+#endif
             if (prov->teardown != NULL)
                 prov->teardown(prov->provctx);
             prov->flag_initialized = 0;
@@ -670,7 +673,7 @@ static OPENSSL_CTX *core_get_libctx(const OSSL_PROVIDER *prov)
 static int core_thread_start(const OSSL_PROVIDER *prov,
                              OSSL_thread_stop_handler_fn handfn)
 {
-    return ossl_init_thread_start(prov->provctx, handfn);
+    return ossl_init_thread_start(prov, prov->provctx, handfn);
 }
 
 static const OSSL_DISPATCH core_dispatch_[] = {

diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c
index 5d6ea1e26e5bab3b0ecd525162e4d87532ae6063..33bc81c07f028ed491acb94f48fad08697a0982e 100644 (file)
--- a/crypto/rand/drbg_lib.c
+++ b/crypto/rand/drbg_lib.c
@@ -1339,7 +1339,7 @@ RAND_DRBG *OPENSSL_CTX_get0_public_drbg(OPENSSL_CTX *ctx)
 
     drbg = CRYPTO_THREAD_get_local(&dgbl->public_drbg);
     if (drbg == NULL) {
-        if (!ossl_init_thread_start(NULL, drbg_delete_thread_state))
+        if (!ossl_init_thread_start(NULL, NULL, drbg_delete_thread_state))
             return NULL;
         drbg = drbg_setup(ctx, dgbl->master_drbg, RAND_DRBG_TYPE_PUBLIC);
         CRYPTO_THREAD_set_local(&dgbl->public_drbg, drbg);
@@ -1366,7 +1366,7 @@ RAND_DRBG *OPENSSL_CTX_get0_private_drbg(OPENSSL_CTX *ctx)
 
     drbg = CRYPTO_THREAD_get_local(&dgbl->private_drbg);
     if (drbg == NULL) {
-        if (!ossl_init_thread_start(NULL, drbg_delete_thread_state))
+        if (!ossl_init_thread_start(NULL, NULL, drbg_delete_thread_state))
             return NULL;
         drbg = drbg_setup(ctx, dgbl->master_drbg, RAND_DRBG_TYPE_PRIVATE);
         CRYPTO_THREAD_set_local(&dgbl->private_drbg, drbg);