DPDK patches and discussions
 help / color / mirror / Atom feed
From: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
To: <dev@dpdk.org>
Cc: <anoobj@marvell.com>, Akhil Goyal <gakhil@marvell.com>,
	Kai Ji <kai.ji@intel.com>, Ciara Power <ciara.power@intel.com>,
	"Gowrishankar Muthukrishnan" <gmuthukrishn@marvell.com>
Subject: [PATCH v2] crypto/openssl: fix memory leaks in asym ops
Date: Thu, 2 Nov 2023 15:33:55 +0530	[thread overview]
Message-ID: <83522013646bcd96b2420b3f69b74255981b3a20.1698913776.git.gmuthukrishn@marvell.com> (raw)
Message-ID: <20231102100355.dGwLwA-MJwQgQpHUFC6s_x6Sc3pS37p93t22nG71Y9I@z> (raw)
In-Reply-To: <20230919130412.284-1-gmuthukrishn@marvell.com>

Fix memory leaks in Asymmetric ops, as reported by valgrind.

Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
---
v2:
 - added more fixes.
---
 drivers/crypto/openssl/rte_openssl_pmd.c     | 38 ++++++++++++++------
 drivers/crypto/openssl/rte_openssl_pmd_ops.c | 15 ++++++--
 2 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
index c234882417..5961457279 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,9 +1951,9 @@ 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)
@@ -1961,7 +1962,9 @@ process_openssl_dsa_sign_op_evp(struct rte_crypto_op *cop,
 		EVP_PKEY_CTX_free(key_ctx);
 	if (dsa_ctx)
 		EVP_PKEY_CTX_free(dsa_ctx);
-	return -1;
+	if (pkey)
+		EVP_PKEY_free(pkey);
+	return ret;
 }
 
 /* process dsa verify operation */
@@ -2034,6 +2037,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);
@@ -2043,6 +2047,10 @@ process_openssl_dsa_verify_op_evp(struct rte_crypto_op *cop,
 		EVP_PKEY_CTX_free(key_ctx);
 	if (dsa_ctx)
 		EVP_PKEY_CTX_free(dsa_ctx);
+	if (pub_key)
+		BN_free(pub_key);
+	if (pkey)
+		EVP_PKEY_free(pkey);
 
 	return ret;
 }
@@ -2674,6 +2682,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;
 
@@ -2739,10 +2750,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);
@@ -2800,11 +2808,8 @@ process_openssl_sm2_op_evp(struct rte_crypto_op *cop,
 		break;
 	case RTE_CRYPTO_ASYM_OP_VERIFY:
 		{
-			unsigned char signbuf[128] = {0};
 			BIGNUM *r = NULL, *s = NULL;
-			EVP_MD_CTX *md_ctx = NULL;
-			ECDSA_SIG *ec_sign;
-			EVP_MD *check_md;
+			unsigned char *signbuf;
 			size_t signlen;
 
 			kctx = EVP_PKEY_CTX_new_from_name(NULL, "SM2", NULL);
@@ -2857,13 +2862,18 @@ process_openssl_sm2_op_evp(struct rte_crypto_op *cop,
 			r = NULL;
 			s = NULL;
 
-			signlen = i2d_ECDSA_SIG(ec_sign, (unsigned char **)&signbuf);
-			if (signlen <= 0)
+			signlen = i2d_ECDSA_SIG(ec_sign, 0);
+			signbuf = rte_malloc(NULL, signlen, 0);
+			signlen = i2d_ECDSA_SIG(ec_sign, &signbuf);
+			if (signlen <= 0) {
+				rte_free(signbuf);
 				goto err_sm2;
+			}
 
 			if (!EVP_DigestVerifyFinal(md_ctx, signbuf, signlen))
 				goto err_sm2;
 
+			rte_free(signbuf);
 			BN_free(r);
 			BN_free(s);
 			ECDSA_SIG_free(ec_sign);
@@ -2880,6 +2890,12 @@ process_openssl_sm2_op_evp(struct rte_crypto_op *cop,
 	ret = 0;
 	cop->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
 err_sm2:
+	if (check_md)
+		EVP_MD_free(check_md);
+
+	if (md_ctx)
+		EVP_MD_CTX_free(md_ctx);
+
 	if (kctx)
 		EVP_PKEY_CTX_free(kctx);
 
diff --git a/drivers/crypto/openssl/rte_openssl_pmd_ops.c b/drivers/crypto/openssl/rte_openssl_pmd_ops.c
index 2862c294a9..98450f36cf 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:
 	{
@@ -1228,6 +1230,7 @@ static int openssl_set_asym_session_parameters(
 		}
 		asym_session->xfrm_type = RTE_CRYPTO_ASYM_XFORM_DSA;
 		asym_session->u.s.param_bld = param_bld;
+		BN_free(pub_key);
 
 		break;
 #else
@@ -1363,6 +1366,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 +1377,8 @@ static int openssl_set_asym_session_parameters(
 		if (asym_session->u.sm2.params)
 			OSSL_PARAM_free(asym_session->u.sm2.params);
 
+		if (pkey_bn)
+			BN_free(pkey_bn);
 		return -1;
 #else
 		OPENSSL_LOG(WARNING, "SM2 unsupported in current OpenSSL Version");
@@ -1452,6 +1458,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
@@ -1461,6 +1469,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)
-- 
2.25.1


  parent reply	other threads:[~2023-11-02 23:50 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-19 13:04 [PATCH] crypto/openssl: fix memory leaks in SM2 ops Gowrishankar Muthukrishnan
2023-10-23 13:33 ` Akhil Goyal
2023-11-02  8:38 ` Gowrishankar Muthukrishnan [this message]
2023-11-02 10:03   ` [PATCH v2] crypto/openssl: fix memory leaks in asym ops Gowrishankar Muthukrishnan
2023-11-02 22:47   ` Stephen Hemminger
2023-11-03 15:19     ` [EXT] " Gowrishankar Muthukrishnan
2023-11-03 10:18   ` Power, Ciara
2023-11-03 11:38   ` Power, Ciara
2023-11-03 15:15   ` [PATCH v3] " Gowrishankar Muthukrishnan
2023-11-03 15:39     ` Power, Ciara
2023-11-09 20:18     ` Akhil Goyal
2023-11-13  5:41     ` [PATCH v4] " Gowrishankar Muthukrishnan
2023-11-13  6:23       ` Akhil Goyal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83522013646bcd96b2420b3f69b74255981b3a20.1698913776.git.gmuthukrishn@marvell.com \
    --to=gmuthukrishn@marvell.com \
    --cc=anoobj@marvell.com \
    --cc=ciara.power@intel.com \
    --cc=dev@dpdk.org \
    --cc=gakhil@marvell.com \
    --cc=kai.ji@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).