The branch master has been updated via 3b438ef95b5b1c45602b1332921209363e4776bd (commit) from e7aa7c11c71e84b2d6c06b1c81d130e8c1fba296 (commit)
- Log ----------------------------------------------------------------- commit 3b438ef95b5b1c45602b1332921209363e4776bd Author: Dr. Matthias St. Pierre <matthias.st.pie...@ncp-e.com> Date: Wed Jul 17 19:14:01 2019 +0200 Fix init_get_thread_local() Previously, init_get_thread_local() pushed the thread event handler list onto the global register before calling CRYPTO_THREAD_set_local(), and when the latter failed, forgot to pop the list from the stack again. Instead of cleaning the stack on error, this commit avoids the situation entirely by postponing the push operation until all other operations succeeded. This reordering also significantly reduces the scope of the critical section. Another simplification of the code is achieved by moving the push operation onto the register (which is disabled in FIPS mode) into a separate function. Reviewed-by: Richard Levitte <levi...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9401) ----------------------------------------------------------------------- Summary of changes: crypto/initthread.c | 64 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/crypto/initthread.c b/crypto/initthread.c index b3f45b9..7de8a36 100644 --- a/crypto/initthread.c +++ b/crypto/initthread.c @@ -77,6 +77,12 @@ static GLOBAL_TEVENT_REGISTER *get_global_tevent_register(void) } #endif +#ifndef FIPS_MODE +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); +static int init_thread_deregister(void *arg, int all); +#endif static void init_thread_stop(void *arg, THREAD_EVENT_HANDLER **hands); static THREAD_EVENT_HANDLER ** @@ -86,40 +92,22 @@ init_get_thread_local(CRYPTO_THREAD_LOCAL *local, int alloc, int keep) if (alloc) { if (hands == NULL) { -#ifndef FIPS_MODE - GLOBAL_TEVENT_REGISTER *gtr; -#endif - if ((hands = OPENSSL_zalloc(sizeof(*hands))) == NULL) { - OPENSSL_free(hands); + if ((hands = OPENSSL_zalloc(sizeof(*hands))) == NULL) 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) { + if (!CRYPTO_THREAD_set_local(local, hands)) { OPENSSL_free(hands); return NULL; } - CRYPTO_THREAD_write_lock(gtr->lock); - if (!sk_THREAD_EVENT_HANDLER_PTR_push(gtr->skhands, hands)) { + +#ifndef FIPS_MODE + if (!init_thread_push_handlers(hands)) { + CRYPTO_THREAD_set_local(local, NULL); 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); @@ -148,6 +136,32 @@ static union { 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 + * current thread in case of certain events. (Currently, there is + * only one type of event, the 'thread stop' event.) + * + * We also keep a global reference to that linked list, so that we + * can deregister handlers if necessary before all the threads are + * stopped. + */ +static int init_thread_push_handlers(THREAD_EVENT_HANDLER **hands) +{ + int ret; + GLOBAL_TEVENT_REGISTER *gtr; + + gtr = get_global_tevent_register(); + if (gtr == NULL) + return 0; + + CRYPTO_THREAD_write_lock(gtr->lock); + ret = (sk_THREAD_EVENT_HANDLER_PTR_push(gtr->skhands, hands) != 0); + CRYPTO_THREAD_unlock(gtr->lock); + + return ret; +} + static void init_thread_remove_handlers(THREAD_EVENT_HANDLER **handsin) { GLOBAL_TEVENT_REGISTER *gtr; @@ -187,8 +201,6 @@ 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);