From: Matt Caswell <matt@openssl.org>
Date: Thu, 2 Jun 2022 12:54:45 +0000 (+0100)
Subject: CONF_modules_unload should fail if CONF_modules_finish fails
X-Git-Tag: openssl-3.2.0-alpha1~2559
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=697d0b5ba146c232f5b2aa87f4e847a5495c1735;p=thirdparty%2Fopenssl.git

CONF_modules_unload should fail if CONF_modules_finish fails

The module_list_lock is used by CONF_modules_unload(). That function relies
on the RUN_ONCE in CONF_modules_finish() to initialise that lock. However
if the RUN_ONCE fails that failure is not propagated to
CONF_modules_unload() and so it erroneously tries to use the lock anyway.

Found due to:
https://github.com/openssl/openssl/pull/18355#issuecomment-1144734604

Reviewed-by: Todd Short <todd.short@me.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18460)
---

diff --git a/crypto/conf/conf_mod.c b/crypto/conf/conf_mod.c
index 6286282108e..c6b8696388c 100644
--- a/crypto/conf/conf_mod.c
+++ b/crypto/conf/conf_mod.c
@@ -82,6 +82,8 @@ 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 int conf_modules_finish_int(void);
+
 static void module_lists_free(void)
 {
     CRYPTO_THREAD_lock_free(module_list_lock);
@@ -477,7 +479,8 @@ void CONF_modules_unload(int all)
     int i;
     CONF_MODULE *md;
 
-    CONF_modules_finish(); /* also inits module list lock */
+    if (!conf_modules_finish_int()) /* also inits module list lock */
+        return;
 
     if (!CRYPTO_THREAD_write_lock(module_list_lock))
         return;
@@ -511,15 +514,15 @@ static void module_free(CONF_MODULE *md)
 
 /* finish and free up all modules instances */
 
-void CONF_modules_finish(void)
+static int conf_modules_finish_int(void)
 {
     CONF_IMODULE *imod;
 
     if (!RUN_ONCE(&init_module_list_lock, do_init_module_list_lock))
-        return;
+        return 0;
 
     if (!CRYPTO_THREAD_write_lock(module_list_lock))
-        return;
+        return 0;
 
     while (sk_CONF_IMODULE_num(initialized_modules) > 0) {
         imod = sk_CONF_IMODULE_pop(initialized_modules);
@@ -529,6 +532,13 @@ void CONF_modules_finish(void)
     initialized_modules = NULL;
 
     CRYPTO_THREAD_unlock(module_list_lock);
+
+    return 1;
+}
+
+void CONF_modules_finish(void)
+{
+    conf_modules_finish_int();
 }
 
 /* finish a module instance */