From ef7a9b44f04ef18b652cb47cd9eb3826301cca9e Mon Sep 17 00:00:00 2001 From: Hugo Landau Date: Tue, 17 May 2022 13:47:57 +0100 Subject: [PATCH] Make OSSL_LIB_CTX_load_config thread safe Fixes #18226. Reviewed-by: Matt Caswell Reviewed-by: Paul Dale Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/18331) --- crypto/conf/conf_mod.c | 106 ++++++++++++++++++++++++++++++++++------- test/threadstest.c | 14 ++++++ 2 files changed, 102 insertions(+), 18 deletions(-) diff --git a/crypto/conf/conf_mod.c b/crypto/conf/conf_mod.c index 36b054ca516..6286282108e 100644 --- a/crypto/conf/conf_mod.c +++ b/crypto/conf/conf_mod.c @@ -62,8 +62,10 @@ struct conf_imodule_st { void *usr_data; }; -static STACK_OF(CONF_MODULE) *supported_modules = NULL; -static STACK_OF(CONF_IMODULE) *initialized_modules = NULL; +static CRYPTO_ONCE init_module_list_lock = CRYPTO_ONCE_STATIC_INIT; +static CRYPTO_RWLOCK *module_list_lock = NULL; +static STACK_OF(CONF_MODULE) *supported_modules = NULL; /* protected by lock */ +static STACK_OF(CONF_IMODULE) *initialized_modules = NULL; /* protected by lock */ static CRYPTO_ONCE load_builtin_modules = CRYPTO_ONCE_STATIC_INIT; @@ -80,6 +82,29 @@ static int module_init(CONF_MODULE *pmod, const char *name, const char *value, static CONF_MODULE *module_load_dso(const CONF *cnf, const char *name, const char *value); +static void module_lists_free(void) +{ + CRYPTO_THREAD_lock_free(module_list_lock); + module_list_lock = NULL; + + sk_CONF_MODULE_free(supported_modules); + supported_modules = NULL; + + sk_CONF_IMODULE_free(initialized_modules); + initialized_modules = NULL; +} + +DEFINE_RUN_ONCE_STATIC(do_init_module_list_lock) +{ + module_list_lock = CRYPTO_THREAD_lock_new(); + if (module_list_lock == NULL) { + ERR_raise(ERR_LIB_CONF, ERR_R_MALLOC_FAILURE); + return 0; + } + + return 1; +} + static int conf_diagnostics(const CONF *cnf) { return _CONF_get_number(cnf, NULL, "config_diagnostics") != 0; @@ -294,31 +319,42 @@ static CONF_MODULE *module_add(DSO *dso, const char *name, conf_init_func *ifunc, conf_finish_func *ffunc) { CONF_MODULE *tmod = NULL; + + if (!RUN_ONCE(&init_module_list_lock, do_init_module_list_lock)) + return NULL; + + if (!CRYPTO_THREAD_write_lock(module_list_lock)) + return NULL; + if (supported_modules == NULL) supported_modules = sk_CONF_MODULE_new_null(); if (supported_modules == NULL) - return NULL; + goto err; if ((tmod = OPENSSL_zalloc(sizeof(*tmod))) == NULL) { ERR_raise(ERR_LIB_CONF, ERR_R_MALLOC_FAILURE); - return NULL; + goto err; } tmod->dso = dso; tmod->name = OPENSSL_strdup(name); tmod->init = ifunc; tmod->finish = ffunc; - if (tmod->name == NULL) { - OPENSSL_free(tmod); - return NULL; - } + if (tmod->name == NULL) + goto err; + + if (!sk_CONF_MODULE_push(supported_modules, tmod)) + goto err; - if (!sk_CONF_MODULE_push(supported_modules, tmod)) { + CRYPTO_THREAD_unlock(module_list_lock); + return tmod; + + err: + CRYPTO_THREAD_unlock(module_list_lock); + if (tmod != NULL) { OPENSSL_free(tmod->name); OPENSSL_free(tmod); - return NULL; } - - return tmod; + return NULL; } /* @@ -339,14 +375,22 @@ static CONF_MODULE *module_find(const char *name) else nchar = strlen(name); + if (!RUN_ONCE(&init_module_list_lock, do_init_module_list_lock)) + return NULL; + + if (!CRYPTO_THREAD_read_lock(module_list_lock)) + return NULL; + for (i = 0; i < sk_CONF_MODULE_num(supported_modules); i++) { tmod = sk_CONF_MODULE_value(supported_modules, i); - if (strncmp(tmod->name, name, nchar) == 0) + if (strncmp(tmod->name, name, nchar) == 0) { + CRYPTO_THREAD_unlock(module_list_lock); return tmod; + } } + CRYPTO_THREAD_unlock(module_list_lock); return NULL; - } /* initialize a module */ @@ -379,21 +423,30 @@ static int module_init(CONF_MODULE *pmod, const char *name, const char *value, goto err; } + if (!RUN_ONCE(&init_module_list_lock, do_init_module_list_lock)) + goto err; + + if (!CRYPTO_THREAD_write_lock(module_list_lock)) + goto err; + if (initialized_modules == NULL) { initialized_modules = sk_CONF_IMODULE_new_null(); - if (!initialized_modules) { + if (initialized_modules == NULL) { + CRYPTO_THREAD_unlock(module_list_lock); ERR_raise(ERR_LIB_CONF, ERR_R_MALLOC_FAILURE); goto err; } } if (!sk_CONF_IMODULE_push(initialized_modules, imod)) { + CRYPTO_THREAD_unlock(module_list_lock); ERR_raise(ERR_LIB_CONF, ERR_R_MALLOC_FAILURE); goto err; } pmod->links++; + CRYPTO_THREAD_unlock(module_list_lock); return ret; err: @@ -423,7 +476,12 @@ void CONF_modules_unload(int all) { int i; CONF_MODULE *md; - CONF_modules_finish(); + + CONF_modules_finish(); /* also inits module list lock */ + + if (!CRYPTO_THREAD_write_lock(module_list_lock)) + return; + /* unload modules in reverse order */ for (i = sk_CONF_MODULE_num(supported_modules) - 1; i >= 0; i--) { md = sk_CONF_MODULE_value(supported_modules, i); @@ -434,10 +492,13 @@ void CONF_modules_unload(int all) (void)sk_CONF_MODULE_delete(supported_modules, i); module_free(md); } + if (sk_CONF_MODULE_num(supported_modules) == 0) { sk_CONF_MODULE_free(supported_modules); supported_modules = NULL; } + + CRYPTO_THREAD_unlock(module_list_lock); } /* unload a single module */ @@ -453,12 +514,21 @@ static void module_free(CONF_MODULE *md) void CONF_modules_finish(void) { CONF_IMODULE *imod; + + if (!RUN_ONCE(&init_module_list_lock, do_init_module_list_lock)) + return; + + if (!CRYPTO_THREAD_write_lock(module_list_lock)) + return; + while (sk_CONF_IMODULE_num(initialized_modules) > 0) { imod = sk_CONF_IMODULE_pop(initialized_modules); module_finish(imod); } sk_CONF_IMODULE_free(initialized_modules); initialized_modules = NULL; + + CRYPTO_THREAD_unlock(module_list_lock); } /* finish a module instance */ @@ -488,8 +558,8 @@ int CONF_module_add(const char *name, conf_init_func *ifunc, void ossl_config_modules_free(void) { - CONF_modules_finish(); - CONF_modules_unload(1); + CONF_modules_unload(1); /* calls CONF_modules_finish */ + module_lists_free(); } /* Utility functions */ diff --git a/test/threadstest.c b/test/threadstest.c index 537f6cd6957..250b12d037f 100644 --- a/test/threadstest.c +++ b/test/threadstest.c @@ -654,6 +654,19 @@ static int test_obj_add(void) 1, default_provider); } +static void test_lib_ctx_load_config_worker(void) +{ + if (!TEST_int_eq(OSSL_LIB_CTX_load_config(multi_libctx, config_file), 1)) + multi_success = 0; +} + +static int test_lib_ctx_load_config(void) +{ + return thread_run_test(&test_lib_ctx_load_config_worker, + MAXIMUM_THREADS, &test_lib_ctx_load_config_worker, + 1, default_provider); +} + typedef enum OPTION_choice { OPT_ERR = -1, OPT_EOF = 0, @@ -722,6 +735,7 @@ int setup_tests(void) #endif ADD_TEST(test_multi_load_unload_provider); ADD_TEST(test_obj_add); + ADD_TEST(test_lib_ctx_load_config); return 1; } -- 2.47.2