Make OSSL_LIB_CTX_load_config thread safe

Fixes #18226.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18331)

diff --git a/crypto/conf/conf_mod.c b/crypto/conf/conf_mod.c
index 36b054ca516f47538e7d42abc2733b7cab075f98..6286282108e61982f3d51c7f3362aac6f727d0c6 100644 (file)
--- 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 537f6cd69570b43a5a41278f4030548e5a3cb68b..250b12d037f3cb62f1fd56b8843046ff161691fd 100644 (file)
--- 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;
 }