Fix memory leaks by rewriting engine init/finish logic

Currently, some memory and indices were allocated by the engine
during the binding, but this means the resources would leak if
the engine is unloaded again. Instead, refactor the initialization
code to use the bound init() and finish() functions, so resources
are allocated in init() and can be freed again in finish().

This makes valgrind happy.

Signed-off-by: Jonathan Wernberg <jonathaw@axis.com>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

5 files changed, 57 insertions, 15 deletions

diff --git a/e_tpm2-ecc.c b/e_tpm2-ecc.c
index 6ed7afa..2172feb 100644
--- a/e_tpm2-ecc.c
+++ b/e_tpm2-ecc.c
@@ -332,3 +332,14 @@ int tpm2_setup_ecc_methods(void)
 
 	return 1;
 }
+
+void tpm2_teardown_ecc_methods(void)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000
+	ECDSA_METHOD_free(tpm2_ecdsa);
+	CRYPTO_free_ex_index(CRYPTO_EX_INDEX_ECDSA, ec_app_data);
+#else
+	EC_KEY_METHOD_free(tpm2_eck);
+	CRYPTO_free_ex_index(CRYPTO_EX_INDEX_EC_KEY, ec_app_data);
+#endif
+}
diff --git a/e_tpm2-ecc.h b/e_tpm2-ecc.h
index f523946..f3ac79f 100644
--- a/e_tpm2-ecc.h
+++ b/e_tpm2-ecc.h
@@ -3,5 +3,6 @@
 
 void tpm2_bind_key_to_engine_ecc(EVP_PKEY *pkey, void *data);
 int tpm2_setup_ecc_methods(void);
+void tpm2_teardown_ecc_methods(void);
 
 #endif
diff --git a/e_tpm2-rsa.c b/e_tpm2-rsa.c
index c90b6d2..afd88a3 100644
--- a/e_tpm2-rsa.c
+++ b/e_tpm2-rsa.c
@@ -324,3 +324,12 @@ int tpm2_setup_rsa_methods(void)
 
 	return 1;
 }
+
+void tpm2_teardown_rsa_methods(void)
+{
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+	RSA_meth_free(tpm2_rsa);
+#endif
+
+	CRYPTO_free_ex_index(CRYPTO_EX_INDEX_RSA, ex_app_data);
+}
diff --git a/e_tpm2-rsa.h b/e_tpm2-rsa.h
index 21f1ec4..4a9f21f 100644
--- a/e_tpm2-rsa.h
+++ b/e_tpm2-rsa.h
@@ -3,5 +3,6 @@
 
 void tpm2_bind_key_to_engine_rsa(EVP_PKEY *pkey, void *data);
 int tpm2_setup_rsa_methods(void);
+void tpm2_teardown_rsa_methods(void);
 
 #endif
diff --git a/e_tpm2.c b/e_tpm2.c
index 56a0dc7..389ba41 100644
--- a/e_tpm2.c
+++ b/e_tpm2.c
@@ -20,17 +20,7 @@
 #include "e_tpm2.h"
 
 char *srk_auth = NULL;
-static char *nvprefix;
-
-static int tpm2_engine_init(ENGINE * e)
-{
-	return 1;
-}
-
-static int tpm2_engine_finish(ENGINE * e)
-{
-	return 1;
-}
+static char *nvprefix = NULL;
 
 static int tpm2_set_nvkey_prefix(char *prefix)
 {
@@ -45,6 +35,38 @@ static int tpm2_set_nvkey_prefix(char *prefix)
 	return 1;
 }
 
+static int tpm2_engine_init(ENGINE * e)
+{
+	if (!tpm2_set_nvkey_prefix("//nvkey:"))
+		return 0;
+
+	if (!tpm2_setup_ecc_methods())
+		goto err1;
+
+	if (!tpm2_setup_rsa_methods())
+		goto err2;
+
+	return 1;
+
+err2:
+	tpm2_teardown_ecc_methods();
+err1:
+	OPENSSL_free(nvprefix);
+	nvprefix = NULL;
+
+	return 0;
+}
+
+static int tpm2_engine_finish(ENGINE * e)
+{
+	tpm2_teardown_ecc_methods();
+	tpm2_teardown_rsa_methods();
+	OPENSSL_free(nvprefix);
+	nvprefix = NULL;
+
+	return 1;
+}
+
 static int tpm2_create_srk_policy(char *secret)
 {
 	int len;
@@ -263,9 +285,7 @@ static int tpm2_bind_helper(ENGINE * e)
 	    !ENGINE_set_ctrl_function(e, tpm2_engine_ctrl) ||
 	    !ENGINE_set_load_pubkey_function(e, tpm2_engine_load_pubkey) ||
 	    !ENGINE_set_load_privkey_function(e, tpm2_engine_load_key) ||
-	    !ENGINE_set_cmd_defns(e, tpm2_cmd_defns) ||
-	    !tpm2_setup_ecc_methods() ||
-	    !tpm2_setup_rsa_methods())
+	    !ENGINE_set_cmd_defns(e, tpm2_cmd_defns))
 		return 0;
 
 	return 1;
@@ -281,7 +301,7 @@ static int tpm2_bind_fn(ENGINE * e, const char *id)
 		       id, engine_tpm2_id);
 		return 0;
 	}
-	tpm2_set_nvkey_prefix("//nvkey:");
+
 	if (!tpm2_bind_helper(e)) {
 		fprintf(stderr, "tpm2_bind_helper failed\n");
 		return 0;