Ensure all the thread-specific destructors run before the global destructors

Rework the OpenSSL cleanup logic

diff --git a/src/lib/tls/base.c b/src/lib/tls/base.c
index cd188b578c6ee7efc47f977cb93d371c6b1621bf..4d82af28f6a442a4392502413aae8711a9f1d544 100644 (file)
--- a/src/lib/tls/base.c
+++ b/src/lib/tls/base.c
@@ -404,10 +404,16 @@ void fr_openssl_free(void)
 {
        if (--instance_count > 0) return;
 
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
-       fr_tls_engine_free_all();
+       fr_dict_autofree(tls_dict);
 
-#else
+       fr_tls_log_free();
+
+       fr_tls_bio_free();
+}
+
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+static void _openssl_provider_free(void)
+{
        if (openssl_default_provider && !OSSL_PROVIDER_unload(openssl_default_provider)) {
                fr_tls_log_error(NULL, "Failed unloading default provider");
        }
@@ -417,15 +423,19 @@ void fr_openssl_free(void)
                fr_tls_log_error(NULL, "Failed unloading legacy provider");
        }
        openssl_legacy_provider = NULL;
+}
 #endif
 
-       OPENSSL_cleanup();
-
-       fr_dict_autofree(tls_dict);
-
-       fr_tls_log_free();
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
+static void _openssl_engine_free(void)
+{
+       fr_tls_engine_free_all();
+}
+#endif
 
-       fr_tls_bio_free();
+static void fr_openssl_cleanup(UNUSED void *uctx)
+{
+       OPENSSL_cleanup();
 }
 
 /** Add all the default ciphers and message digests to our context.
@@ -449,6 +459,16 @@ int fr_openssl_init(void)
                return -1;
        }
 
+       /*
+        *      NO_ATEXIT has no effect if init is done after
+        *      loading providers, and we need to control the
+        *      exit handler as it needs to be executed last
+        *      after all the EVP_MD ctx have been called, as
+        *      they may unload elements of providers once all
+        *      the contexts have been cleaned up.
+        */
+       OPENSSL_init_crypto(OPENSSL_INIT_NO_ATEXIT | OPENSSL_INIT_LOAD_CONFIG, NULL);
+
 #if OPENSSL_VERSION_NUMBER >= 0x30000000L
        /*
         *      Load the default provider for most algorithms
@@ -471,7 +491,18 @@ int fr_openssl_init(void)
        }
 #endif
 
-       OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL);
+       /*
+        *      It's best to use OpenSSL's cleanup stack
+        *      as then everything is cleaned up relative
+        *      to the OPENSSL_cleanup() call.
+        */
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+       OPENSSL_atexit(_openssl_provider_free);
+#endif
+
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
+       OPENSSL_atexit(_openssl_engine_free);
+#endif
 
        /*
         *      SHA256 is in all versions of OpenSSL, but isn't
@@ -492,6 +523,16 @@ int fr_openssl_init(void)
 
        fr_tls_bio_init();
 
+       /*
+        *      Use an atexit handler to try and ensure
+        *      that OpenSSL gets freed last.
+        *
+        *      All EVP_*ctxs need to be freed before we
+        *      de-initialise the libraries else we get
+        *      crashes (at least with OpenSSL 3.0.1).
+        */
+       fr_atexit_global(fr_openssl_cleanup, NULL);
+
        instance_count++;
 
        return 0;

diff --git a/src/lib/tls/ctx.c b/src/lib/tls/ctx.c
index ea1d44dd691f22b7741236d12f899da7ddec73b1..13eea42e88f3bc0802e6a6d0931f3628e97860eb 100644 (file)
--- a/src/lib/tls/ctx.c
+++ b/src/lib/tls/ctx.c
@@ -946,7 +946,7 @@ post_ca:
         *      message.  For every new session, there can be a
         *      different callback argument.
         */
-        SSL_CTX_set_msg_callback(ctx, fr_tls_session_msg_cb);
+       SSL_CTX_set_msg_callback(ctx, fr_tls_session_msg_cb);
        /* Set Info callback */
        SSL_CTX_set_info_callback(ctx, fr_tls_session_info_cb);
 

diff --git a/src/lib/util/atexit.c b/src/lib/util/atexit.c
index 74976e87046973a498e53fe7f3a9430869f7e346..9d2361f76146c2be11582852143dd4a6e664ce32 100644 (file)
--- a/src/lib/util/atexit.c
+++ b/src/lib/util/atexit.c
@@ -69,6 +69,7 @@ struct fr_exit_handler_list_s {
 };
 
 static _Thread_local fr_atexit_list_t  *fr_atexit_thread_local = NULL;
+static fr_atexit_list_t                        *fr_atexit_threads = NULL;
 static fr_atexit_list_t                        *fr_atexit_global = NULL;
 static pthread_mutex_t                 fr_atexit_global_mutex = PTHREAD_MUTEX_INITIALIZER;
 static bool                            is_exiting;
@@ -168,6 +169,7 @@ static void _global_free(void)
        is_exiting = true;
        pthread_mutex_unlock(&fr_atexit_global_mutex);
 
+       TALLOC_FREE(fr_atexit_threads); /* Forcefully cleanup any thread-specific memory */
        TALLOC_FREE(fr_atexit_global);
 }
 
@@ -185,6 +187,15 @@ int fr_atexit_global_setup(void)
 
        fr_dlist_talloc_init(&fr_atexit_global->head, fr_atexit_entry_t, entry);
        talloc_set_destructor(fr_atexit_global, _global_list_free);
+
+       fr_atexit_threads = talloc_zero(NULL, fr_atexit_list_t);
+       if (unlikely(!fr_atexit_threads)) return -1;
+
+       THREAD_LOCAL_DEBUG("%s - Alloced threads destructor list %p", __FUNCTION__, fr_atexit_threads);
+
+       fr_dlist_talloc_init(&fr_atexit_threads->head, fr_atexit_entry_t, entry);
+       talloc_set_destructor(fr_atexit_threads, _global_list_free);
+
        atexit(_global_free);   /* Call all remaining destructors at process exit */
 
        return 0;
@@ -262,7 +273,7 @@ int _fr_atexit_thread_local(NDEBUG_LOCATION_ARGS
                 */
                pthread_mutex_lock(&fr_atexit_global_mutex);
                list->e = atexit_entry_alloc(NDEBUG_LOCATION_VALS
-                                            fr_atexit_global,
+                                            fr_atexit_threads,
                                             _thread_local_free,
                                             list);