[v4] crypto/openssl: fix memory leaks in asym ops

Message ID 0b2e2f052c59d383b4c9daca9927a8b67e7ac801.1699853968.git.gmuthukrishn@marvell.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series [v4] crypto/openssl: fix memory leaks in asym ops |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

Gowrishankar Muthukrishnan Nov. 13, 2023, 5:41 a.m. UTC
Fix memory leaks in Asymmetric ops, as reported by valgrind.

Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
---
v4:
 - patch rebased.
---
 drivers/crypto/openssl/rte_openssl_pmd.c     | 28 ++++++++++++--------
 drivers/crypto/openssl/rte_openssl_pmd_ops.c | 16 +++++++----
 2 files changed, 28 insertions(+), 16 deletions(-)
  

Comments

Akhil Goyal Nov. 13, 2023, 6:23 a.m. UTC | #1
> Subject: [PATCH v4] crypto/openssl: fix memory leaks in asym ops
> 
> Fix memory leaks in Asymmetric ops, as reported by valgrind.
> 
> Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
> ---
> v4:
>  - patch rebased.
Applied to dpdk-next-crypto
Added-fixes tags and Ciara's ack from v3.

Thanks.
  

Patch

diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
index 090320602d..9d463520ff 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd.c
@@ -1897,6 +1897,7 @@  process_openssl_dsa_sign_op_evp(struct rte_crypto_op *cop,
 	size_t outlen;
 	unsigned char *dsa_sign_data;
 	const unsigned char *dsa_sign_data_p;
+	int ret = -1;
 
 	cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
 	params = OSSL_PARAM_BLD_to_param(param_bld);
@@ -1950,16 +1951,16 @@  process_openssl_dsa_sign_op_evp(struct rte_crypto_op *cop,
 		cop->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
 	}
 
+	ret = 0;
 	DSA_SIG_free(sign);
 	OPENSSL_free(dsa_sign_data);
-	return 0;
 
 err_dsa_sign:
 	if (params)
 		OSSL_PARAM_free(params);
 	EVP_PKEY_CTX_free(key_ctx);
 	EVP_PKEY_CTX_free(dsa_ctx);
-	return -1;
+	return ret;
 }
 
 /* process dsa verify operation */
@@ -2032,6 +2033,7 @@  process_openssl_dsa_verify_op_evp(struct rte_crypto_op *cop,
 		ret = 0;
 	}
 
+	OPENSSL_free(dsa_sig);
 err_dsa_verify:
 	if (sign)
 		DSA_SIG_free(sign);
@@ -2040,6 +2042,9 @@  process_openssl_dsa_verify_op_evp(struct rte_crypto_op *cop,
 	EVP_PKEY_CTX_free(key_ctx);
 	EVP_PKEY_CTX_free(dsa_ctx);
 
+	BN_free(pub_key);
+	EVP_PKEY_free(pkey);
+
 	return ret;
 }
 #else
@@ -2666,6 +2671,9 @@  process_openssl_sm2_op_evp(struct rte_crypto_op *cop,
 	EVP_PKEY_CTX *kctx = NULL, *sctx = NULL, *cctx = NULL;
 	struct rte_crypto_asym_op *op = cop->asym;
 	OSSL_PARAM *params = sess->u.sm2.params;
+	EVP_MD_CTX *md_ctx = NULL;
+	ECDSA_SIG *ec_sign = NULL;
+	EVP_MD *check_md = NULL;
 	EVP_PKEY *pkey = NULL;
 	int ret = -1;
 
@@ -2731,10 +2739,7 @@  process_openssl_sm2_op_evp(struct rte_crypto_op *cop,
 		{
 			unsigned char signbuf[128] = {0};
 			const unsigned char *signptr;
-			EVP_MD_CTX *md_ctx = NULL;
 			const BIGNUM *r, *s;
-			ECDSA_SIG *ec_sign;
-			EVP_MD *check_md;
 			size_t signlen;
 
 			kctx = EVP_PKEY_CTX_new_from_name(NULL, "SM2", NULL);
@@ -2792,11 +2797,8 @@  process_openssl_sm2_op_evp(struct rte_crypto_op *cop,
 		break;
 	case RTE_CRYPTO_ASYM_OP_VERIFY:
 		{
-			unsigned char signbuf[128] = {0};
+			unsigned char signbuf[128] = {0}, *signbuf_new = NULL;
 			BIGNUM *r = NULL, *s = NULL;
-			EVP_MD_CTX *md_ctx = NULL;
-			ECDSA_SIG *ec_sign;
-			EVP_MD *check_md;
 			size_t signlen;
 
 			kctx = EVP_PKEY_CTX_new_from_name(NULL, "SM2", NULL);
@@ -2849,11 +2851,12 @@  process_openssl_sm2_op_evp(struct rte_crypto_op *cop,
 			r = NULL;
 			s = NULL;
 
-			signlen = i2d_ECDSA_SIG(ec_sign, (unsigned char **)&signbuf);
+			signbuf_new = signbuf;
+			signlen = i2d_ECDSA_SIG(ec_sign, (unsigned char **)&signbuf_new);
 			if (signlen <= 0)
 				goto err_sm2;
 
-			if (!EVP_DigestVerifyFinal(md_ctx, signbuf, signlen))
+			if (!EVP_DigestVerifyFinal(md_ctx, signbuf_new, signlen))
 				goto err_sm2;
 
 			BN_free(r);
@@ -2872,6 +2875,9 @@  process_openssl_sm2_op_evp(struct rte_crypto_op *cop,
 	ret = 0;
 	cop->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
 err_sm2:
+	EVP_MD_free(check_md);
+	EVP_MD_CTX_free(md_ctx);
+
 	EVP_PKEY_CTX_free(kctx);
 
 	EVP_PKEY_CTX_free(sctx);
diff --git a/drivers/crypto/openssl/rte_openssl_pmd_ops.c b/drivers/crypto/openssl/rte_openssl_pmd_ops.c
index 419a767817..db5579bdb1 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd_ops.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd_ops.c
@@ -958,9 +958,11 @@  static int openssl_set_asym_session_parameters(
 		rsa_ctx = EVP_PKEY_CTX_new(pkey, NULL);
 		asym_session->xfrm_type = RTE_CRYPTO_ASYM_XFORM_RSA;
 		asym_session->u.r.ctx = rsa_ctx;
+		EVP_PKEY_free(pkey);
 		EVP_PKEY_CTX_free(key_ctx);
+		OSSL_PARAM_BLD_free(param_bld);
 		OSSL_PARAM_free(params);
-		break;
+		ret = 0;
 #else
 		RSA *rsa = RSA_new();
 		if (rsa == NULL)
@@ -1030,7 +1032,7 @@  static int openssl_set_asym_session_parameters(
 		}
 		asym_session->u.r.rsa = rsa;
 		asym_session->xfrm_type = RTE_CRYPTO_ASYM_XFORM_RSA;
-		break;
+		ret = 0;
 #endif
 err_rsa:
 		BN_clear_free(n);
@@ -1042,7 +1044,7 @@  static int openssl_set_asym_session_parameters(
 		BN_clear_free(dmq1);
 		BN_clear_free(iqmp);
 
-		return -1;
+		return ret;
 	}
 	case RTE_CRYPTO_ASYM_XFORM_MODEX:
 	{
@@ -1184,8 +1186,7 @@  static int openssl_set_asym_session_parameters(
 #if (OPENSSL_VERSION_NUMBER >= 0x30000000L)
 		BIGNUM *p = NULL, *g = NULL;
 		BIGNUM *q = NULL, *priv_key = NULL;
-		BIGNUM *pub_key = BN_new();
-		BN_zero(pub_key);
+		BIGNUM *pub_key = NULL;
 		OSSL_PARAM_BLD *param_bld = NULL;
 
 		p = BN_bin2bn((const unsigned char *)
@@ -1363,6 +1364,7 @@  static int openssl_set_asym_session_parameters(
 
 		asym_session->u.sm2.params = params;
 		OSSL_PARAM_BLD_free(param_bld);
+		BN_free(pkey_bn);
 
 		asym_session->xfrm_type = RTE_CRYPTO_ASYM_XFORM_SM2;
 		break;
@@ -1373,6 +1375,7 @@  static int openssl_set_asym_session_parameters(
 		if (asym_session->u.sm2.params)
 			OSSL_PARAM_free(asym_session->u.sm2.params);
 
+		BN_free(pkey_bn);
 		return -1;
 #else
 		OPENSSL_LOG(WARNING, "SM2 unsupported in current OpenSSL Version");
@@ -1451,6 +1454,8 @@  static void openssl_reset_asym_session(struct openssl_asym_session *sess)
 		break;
 	case RTE_CRYPTO_ASYM_XFORM_DH:
 #if (OPENSSL_VERSION_NUMBER >= 0x30000000L)
+		OSSL_PARAM_BLD_free(sess->u.dh.param_bld);
+		OSSL_PARAM_BLD_free(sess->u.dh.param_bld_peer);
 		sess->u.dh.param_bld = NULL;
 		sess->u.dh.param_bld_peer = NULL;
 #else
@@ -1460,6 +1465,7 @@  static void openssl_reset_asym_session(struct openssl_asym_session *sess)
 		break;
 	case RTE_CRYPTO_ASYM_XFORM_DSA:
 #if (OPENSSL_VERSION_NUMBER >= 0x30000000L)
+		OSSL_PARAM_BLD_free(sess->u.s.param_bld);
 		sess->u.s.param_bld = NULL;
 #else
 		if (sess->u.s.dsa)