wbrawner/openssl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix init_get_thread_local()

Browse source 
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 <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9401)
This commit is contained in:
Dr. Matthias St. Pierre 2019-07-17 19:14:01 +02:00
parent e7aa7c11c7
commit 3b438ef95b
1 changed files with 41 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

70
crypto/initthread.c
View file

@ -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) {
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;
}
#ifndef FIPS_MODE
if (!init_thread_push_handlers(hands)) {
CRYPTO_THREAD_set_local(local, NULL);
OPENSSL_free(hands);
return NULL;
}
#endif
}
} 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);