From: Matt Caswell <matt@openssl.org>
Date: Mon, 25 Sep 2023 15:44:47 +0000 (+0100)
Subject: Fix a mem leak when the FIPS provider is used in a different thread
X-Git-Tag: openssl-3.1.4~56
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6b3f7f0b28afc3e25ce505e5fde2d9f2b50cdb9e;p=thirdparty%2Fopenssl.git

Fix a mem leak when the FIPS provider is used in a different thread

We were neglecting to register the main thread to receive thread stop
notifications. This is important if the thread that starts the FIPS
provider is not the same one that is used when OPENSSL_cleanup() is
called.

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

diff --git a/crypto/initthread.c b/crypto/initthread.c
index 54a33c32865..23ad0a07391 100644
--- a/crypto/initthread.c
+++ b/crypto/initthread.c
@@ -249,6 +249,15 @@ void ossl_ctx_thread_stop(OSSL_LIB_CTX *ctx)
 
 #else
 
+static void ossl_arg_thread_stop(void *arg);
+
+/* Register the current thread so that we are informed if it gets stopped */
+int ossl_thread_register_fips(OSSL_LIB_CTX *libctx)
+{
+    return c_thread_start(FIPS_get_core_handle(libctx), ossl_arg_thread_stop,
+                          libctx);
+}
+
 void *ossl_thread_event_ctx_new(OSSL_LIB_CTX *libctx)
 {
     THREAD_EVENT_HANDLER **hands = NULL;
@@ -268,6 +277,16 @@ void *ossl_thread_event_ctx_new(OSSL_LIB_CTX *libctx)
     if (!CRYPTO_THREAD_set_local(tlocal, hands))
         goto err;
 
+    /*
+     * We should ideally call ossl_thread_register_fips() here. This function
+     * is called during the startup of the FIPS provider and we need to ensure
+     * that the main thread is registered to receive thread callbacks in order
+     * to free |hands| that we allocated above. However we are too early in
+     * the FIPS provider initialisation that FIPS_get_core_handle() doesn't work
+     * yet. So we defer this to the main provider OSSL_provider_init_int()
+     * function.
+     */
+
     return tlocal;
  err:
     OPENSSL_free(hands);
@@ -379,8 +398,7 @@ int ossl_init_thread_start(const void *index, void *arg,
          * libcrypto to tell us about later thread stop events. c_thread_start
          * is a callback to libcrypto defined in fipsprov.c
          */
-        if (!c_thread_start(FIPS_get_core_handle(ctx), ossl_arg_thread_stop,
-                            ctx))
+        if (!ossl_thread_register_fips(ctx))
             return 0;
     }
 #endif
diff --git a/include/crypto/context.h b/include/crypto/context.h
index cc06c71be80..213e3686480 100644
--- a/include/crypto/context.h
+++ b/include/crypto/context.h
@@ -21,6 +21,7 @@ void *ossl_child_prov_ctx_new(OSSL_LIB_CTX *);
 void *ossl_prov_drbg_nonce_ctx_new(OSSL_LIB_CTX *);
 void *ossl_self_test_set_callback_new(OSSL_LIB_CTX *);
 void *ossl_rand_crng_ctx_new(OSSL_LIB_CTX *);
+int ossl_thread_register_fips(OSSL_LIB_CTX *);
 void *ossl_thread_event_ctx_new(OSSL_LIB_CTX *);
 void *ossl_fips_prov_ossl_ctx_new(OSSL_LIB_CTX *);
 
diff --git a/providers/fips/fipsprov.c b/providers/fips/fipsprov.c
index 0cd5e1bba26..bf22e907bc7 100644
--- a/providers/fips/fipsprov.c
+++ b/providers/fips/fipsprov.c
@@ -699,6 +699,15 @@ int OSSL_provider_init_int(const OSSL_CORE_HANDLE *handle,
 
     fgbl->handle = handle;
 
+    /*
+     * We need to register this thread to receive thread lifecycle callbacks.
+     * This wouldn't matter if the current thread is also the same thread that
+     * closes the FIPS provider down. But if that happens on a different thread
+     * then memory leaks could otherwise occur.
+     */
+    if (!ossl_thread_register_fips(libctx))
+        goto err;
+
     /*
      * We did initial set up of selftest_params in a local copy, because we
      * could not create fgbl until c_CRYPTO_zalloc was defined in the loop