From: Matt Caswell Date: Tue, 18 Jun 2019 17:37:38 +0000 (+0100) Subject: Provide an ability to deregister thread stop handlers X-Git-Tag: openssl-3.0.0-alpha1~1887 X-Git-Url: https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff_plain;h=6913f5fe05a38fa72213b5b5d1f41ef10ca908bd 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 (Merged from https://github.com/openssl/openssl/pull/9186) --- diff --git a/crypto/async/async.c b/crypto/async/async.c index bcb0030e52..43b16a7b7c 100644 --- 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 9eb477ccda..8752c11977 100644 --- 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 a69bdcd408..673a004f0f 100644 --- 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 8755e2164f..d5f0ebd7b7 100644 --- 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 b4ee177c8f..b398b05cd2 100644 --- a/crypto/initthread.c +++ b/crypto/initthread.c @@ -11,6 +11,7 @@ #include #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 10948ce271..274bdf94ba 100644 --- 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 5d6ea1e26e..33bc81c07f 100644 --- 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);