From: Neil Horman Date: Wed, 18 Jun 2025 15:16:47 +0000 (-0400) Subject: Refactor init_get_thread_local to be more understandable X-Git-Tag: openssl-3.6.0-alpha1~547 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d259b8b85567410afa02acf2ba9dbbfb8ae53f61;p=thirdparty%2Fopenssl.git Refactor init_get_thread_local to be more understandable We currently have a single function that does thread_local key allocation/cleanup/fetching for our OSSL_init_thread_start/stop apis, and its pretty confusing. Wrap it up in some helper functions to make it more clear at the call sites what we're trying to do. Reviewed-by: Saša Nedvědický Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/27794) --- diff --git a/crypto/initthread.c b/crypto/initthread.c index ea591dcb897..50da74fec36 100644 --- a/crypto/initthread.c +++ b/crypto/initthread.c @@ -84,6 +84,25 @@ static GLOBAL_TEVENT_REGISTER *get_global_tevent_register(void) #endif #ifndef FIPS_MODULE +/* + * Since per-thread-specific-data destructors are not universally + * available, i.e. not on Windows, only below CRYPTO_THREAD_LOCAL key + * is assumed to have destructor associated. And then an effort is made + * to call this single destructor on non-pthread platform[s]. + * + * Initial value is "impossible". It is used as guard value to shortcut + * destructor for threads terminating before libcrypto is initialized or + * after it's de-initialized. Access to the key doesn't have to be + * serialized for the said threads, because they didn't use libcrypto + * and it doesn't matter if they pick "impossible" or dereference real + * key value and pull NULL past initialization in the first thread that + * intends to use libcrypto. + */ +static union { + long sane; + CRYPTO_THREAD_LOCAL value; +} destructor_key = { -1 }; + static int init_thread_push_handlers(THREAD_EVENT_HANDLER **hands); static void init_thread_remove_handlers(THREAD_EVENT_HANDLER **handsin); static void init_thread_destructor(void *hands); @@ -91,11 +110,32 @@ static int init_thread_deregister(void *arg, int all); #endif static void init_thread_stop(void *arg, THREAD_EVENT_HANDLER **hands); -#ifndef FIPS_MODULE +static THREAD_EVENT_HANDLER ** get_thread_event_handler(OSSL_LIB_CTX *ctx) +{ +#ifdef FIPS_MODULE + return CRYPTO_THREAD_get_local_ex(CRYPTO_THREAD_LOCAL_TEVENT_KEY, ctx); +#else + if (destructor_key.sane != -1) + return CRYPTO_THREAD_get_local(&destructor_key.value); + return NULL; +#endif +} + +static int set_thread_event_handler(OSSL_LIB_CTX *ctx, THREAD_EVENT_HANDLER **hands) +{ +#ifdef FIPS_MODULE + return CRYPTO_THREAD_set_local_ex(CRYPTO_THREAD_LOCAL_TEVENT_KEY, ctx, hands); +#else + if (destructor_key.sane != -1) + return CRYPTO_THREAD_set_local(&destructor_key.value, hands); + return 0; +#endif +} + static THREAD_EVENT_HANDLER ** -init_get_thread_local(CRYPTO_THREAD_LOCAL *local, int alloc, int keep) +init_manage_thread_local(OSSL_LIB_CTX *ctx, int alloc, int keep) { - THREAD_EVENT_HANDLER **hands = CRYPTO_THREAD_get_local(local); + THREAD_EVENT_HANDLER **hands = get_thread_event_handler(ctx); if (alloc) { if (hands == NULL) { @@ -103,70 +143,41 @@ init_get_thread_local(CRYPTO_THREAD_LOCAL *local, int alloc, int keep) if ((hands = OPENSSL_zalloc(sizeof(*hands))) == NULL) return NULL; - if (!CRYPTO_THREAD_set_local(local, hands)) { + if (!set_thread_event_handler(ctx, hands)) { OPENSSL_free(hands); return NULL; } - +#ifndef FIPS_MODULE if (!init_thread_push_handlers(hands)) { - CRYPTO_THREAD_set_local(local, NULL); + set_thread_event_handler(ctx, NULL); OPENSSL_free(hands); return NULL; } +#endif } } else if (!keep) { - CRYPTO_THREAD_set_local(local, NULL); + set_thread_event_handler(ctx, NULL); } return hands; } -#else -static THREAD_EVENT_HANDLER ** -init_get_thread_local_ex(OSSL_LIB_CTX *ctx, int alloc, int keep) +static ossl_inline THREAD_EVENT_HANDLER **init_fetch_clear_thread_local(OSSL_LIB_CTX *ctx) { - THREAD_EVENT_HANDLER **hands = CRYPTO_THREAD_get_local_ex(CRYPTO_THREAD_LOCAL_TEVENT_KEY, ctx); - - if (alloc) { - if (hands == NULL) { - - if ((hands = OPENSSL_zalloc(sizeof(*hands))) == NULL) - return NULL; - - if (!CRYPTO_THREAD_set_local_ex(CRYPTO_THREAD_LOCAL_TEVENT_KEY, ctx, hands)) { - OPENSSL_free(hands); - return NULL; - } - } - } else if (!keep) { - CRYPTO_THREAD_set_local_ex(CRYPTO_THREAD_LOCAL_TEVENT_KEY, ctx, NULL); - } + return init_manage_thread_local(ctx, 0, 0); +} - return hands; +static ossl_inline ossl_unused THREAD_EVENT_HANDLER **init_fetch_thread_local(OSSL_LIB_CTX *ctx) +{ + return init_manage_thread_local(ctx, 0, 1); } -#endif +static ossl_inline THREAD_EVENT_HANDLER **init_fetch_alloc_thread_local(OSSL_LIB_CTX *ctx) +{ + return init_manage_thread_local(ctx, 1, 0); +} #ifndef FIPS_MODULE -/* - * Since per-thread-specific-data destructors are not universally - * available, i.e. not on Windows, only below CRYPTO_THREAD_LOCAL key - * is assumed to have destructor associated. And then an effort is made - * to call this single destructor on non-pthread platform[s]. - * - * Initial value is "impossible". It is used as guard value to shortcut - * destructor for threads terminating before libcrypto is initialized or - * after it's de-initialized. Access to the key doesn't have to be - * serialized for the said threads, because they didn't use libcrypto - * and it doesn't matter if they pick "impossible" or dereference real - * key value and pull NULL past initialization in the first thread that - * intends to use libcrypto. - */ -static union { - long sane; - CRYPTO_THREAD_LOCAL value; -} destructor_key = { -1 }; - /* * The thread event handler list is a thread specific linked list * of callback functions which are invoked in list order by the @@ -255,8 +266,8 @@ void OPENSSL_thread_stop_ex(OSSL_LIB_CTX *ctx) void OPENSSL_thread_stop(void) { if (destructor_key.sane != -1) { - THREAD_EVENT_HANDLER **hands - = init_get_thread_local(&destructor_key.value, 0, 0); + THREAD_EVENT_HANDLER **hands = init_fetch_clear_thread_local(NULL); + init_thread_stop(NULL, hands); init_thread_remove_handlers(hands); @@ -267,8 +278,8 @@ void OPENSSL_thread_stop(void) void ossl_ctx_thread_stop(OSSL_LIB_CTX *ctx) { if (destructor_key.sane != -1) { - THREAD_EVENT_HANDLER **hands - = init_get_thread_local(&destructor_key.value, 0, 1); + THREAD_EVENT_HANDLER **hands = init_fetch_thread_local(ctx); + init_thread_stop(ctx, hands); } } @@ -325,7 +336,7 @@ void ossl_ctx_thread_stop(OSSL_LIB_CTX *ctx) { THREAD_EVENT_HANDLER **hands; - hands = init_get_thread_local_ex(ctx, 0, 0); + hands = init_fetch_clear_thread_local(ctx); init_thread_stop(ctx, hands); OPENSSL_free(hands); } @@ -380,26 +391,19 @@ int ossl_init_thread_start(const void *index, void *arg, { THREAD_EVENT_HANDLER **hands; THREAD_EVENT_HANDLER *hand; + OSSL_LIB_CTX *ctx = NULL; #ifdef FIPS_MODULE - OSSL_LIB_CTX *ctx = arg; - /* * In FIPS mode the list of THREAD_EVENT_HANDLERs is unique per combination * of OSSL_LIB_CTX and thread. This is because in FIPS mode each * OSSL_LIB_CTX gets informed about thread stop events individually. */ - hands = init_get_thread_local_ex(ctx, 1, 0); -#else - /* - * Outside of FIPS mode the list of THREAD_EVENT_HANDLERs is unique per - * thread, but may hold multiple OSSL_LIB_CTXs. We only get told about - * thread stop events globally, so we have to ensure all affected - * OSSL_LIB_CTXs are informed. - */ - CRYPTO_THREAD_LOCAL *local = &destructor_key.value; - hands = init_get_thread_local(local, 1, 0); + + ctx = arg; #endif + hands = init_fetch_alloc_thread_local(ctx); + if (hands == NULL) return 0;