[PATCH v2] crypto/openssl: fix use-after-free bug and cleanup

[Pratik Senapati]

params is freed before it is used by
EVP_PKEY_decapsulate_init()
causing a use-after-free issue.

Pass NULL to EVP_PKEY_decapsulate_init()
instead of params to avoid it.

Add resource cleanup for all error paths in the
ML-KEM decapsulate and encapsulate handlers.

Consolidate cleanup into multiple goto labels;
err_decap, err_pkey, err_params for decap and
err_encap, err_pkey, err_params for encap.

Fixes: 5f761d7b605e ("crypto/openssl: support ML-KEM and ML-DSA")
Cc: stable at dpdk.org

Signed-off-by: Pratik Senapati <psenapati at marvell.com>
---
 .mailmap                                 |   1 +
 drivers/crypto/openssl/rte_openssl_pmd.c | 124 +++++++++--------------
 2 files changed, 47 insertions(+), 78 deletions(-)

diff --git a/.mailmap b/.mailmap
index 4f93307aed..b6f47c10b9 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1287,6 +1287,7 @@ Prashant Gupta <prashant.gupta_3 at nxp.com>
 Prashant Upadhyaya <prashant.upadhyaya at aricent.com> <praupadhyaya at gmail.com>
 Prateek Agarwal <prateekag at cse.iitb.ac.in>
 Prathisna Padmasanan <prathisna.padmasanan at intel.com>
+Pratik Senapati <psenapati at marvell.com> 
 Praveen Kaligineedi <pkaligineedi at google.com>
 Praveen Shetty <praveen.shetty at intel.com>
 Pravin Pathak <pravin.pathak.dev at gmail.com> <pravin.pathak at intel.com>
diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
index 4f171f48cc..7464884fb2 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd.c
@@ -3537,35 +3537,24 @@ mlkem_encap_op_evp(struct rte_crypto_op *cop,
 		return -1;
 	}
 
-	if (EVP_PKEY_fromdata_init(pctx) != 1) {
-		OSSL_PARAM_free(params);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
-	}
+	cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
 
-	if (EVP_PKEY_fromdata(pctx, &pkey, EVP_PKEY_PUBLIC_KEY, params) != 1) {
-		OSSL_PARAM_free(params);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
-	}
-	OSSL_PARAM_free(params);
+	if (EVP_PKEY_fromdata_init(pctx) != 1)
+		goto err_params;
 
-	if (pkey == NULL) {
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
-	}
+	if (EVP_PKEY_fromdata(pctx, &pkey, EVP_PKEY_PUBLIC_KEY, params) != 1)
+		goto err_params;
+
+	if (pkey == NULL)
+		goto err_params;
 
 	cctx = EVP_PKEY_CTX_new_from_pkey(NULL, pkey, NULL);
-	if (cctx == NULL) {
-		EVP_PKEY_free(pkey);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
-	}
+	if (cctx == NULL)
+		goto err_pkey;
 
-	if (EVP_PKEY_encapsulate_init(cctx, NULL) != 1) {
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
-	}
+
+	if (EVP_PKEY_encapsulate_init(cctx, NULL) != 1)
+		goto err_encap;
 
 	if (op->encap.message.length) {
 		const OSSL_PARAM kem_params[] = {
@@ -3574,49 +3563,42 @@ mlkem_encap_op_evp(struct rte_crypto_op *cop,
 			OSSL_PARAM_END
 		};
 
-		if (EVP_PKEY_encapsulate_init(cctx, kem_params) != 1) {
-			EVP_PKEY_free(pkey);
-			cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-			return -1;
-		}
+		if (EVP_PKEY_encapsulate_init(cctx, kem_params) != 1)
+			goto err_encap;
 	}
 
 	if (EVP_PKEY_encapsulate(cctx, NULL, &outlen, NULL, &keylen) != 1) {
 		OPENSSL_LOG(ERR, "Failed to determine output length");
-		EVP_PKEY_free(pkey);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
+		goto err_encap;
 	}
 
 	if (outlen > op->encap.cipher.length) {
 		OPENSSL_LOG(ERR, "Insufficient buffer for cipher text");
-		EVP_PKEY_free(pkey);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
+		goto err_encap;
 	}
 
 	if (keylen > op->encap.sk.length) {
 		OPENSSL_LOG(ERR, "Insufficient buffer for shared key");
-		EVP_PKEY_free(pkey);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
+		goto err_encap;
 	}
 
 	if (EVP_PKEY_encapsulate(cctx, op->encap.cipher.data, &outlen,
 			op->encap.sk.data, &keylen) != 1) {
 		OPENSSL_LOG(ERR, "Failed to encapculate");
-		EVP_PKEY_free(pkey);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
+		goto err_encap;
 	}
 
 	op->encap.cipher.length = outlen;
 	op->encap.sk.length = keylen;
+	ret = 0;
+	cop->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
 
+err_encap:
 	EVP_PKEY_CTX_free(cctx);
+err_pkey:
 	EVP_PKEY_free(pkey);
-	ret = 0;
-	cop->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
+err_params:
+	OSSL_PARAM_free(params);
 	return ret;
 }
 
@@ -3664,65 +3646,51 @@ mlkem_decap_op_evp(struct rte_crypto_op *cop,
 		return -1;
 	}
 
-	if (EVP_PKEY_fromdata_init(pctx) != 1) {
-		OSSL_PARAM_free(params);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
-	}
+	cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
 
-	if (EVP_PKEY_fromdata(pctx, &pkey, EVP_PKEY_PRIVATE_KEY, params) != 1) {
-		OSSL_PARAM_free(params);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
-	}
-	OSSL_PARAM_free(params);
+	if (EVP_PKEY_fromdata_init(pctx) != 1)
+		goto err_params;
 
-	if (pkey == NULL) {
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
-	}
+	if (EVP_PKEY_fromdata(pctx, &pkey, EVP_PKEY_PRIVATE_KEY, params) != 1)
+		goto err_params;
+
+	if (pkey == NULL)
+		goto err_params;
 
 	cctx = EVP_PKEY_CTX_new_from_pkey(NULL, pkey, NULL);
-	if (cctx == NULL) {
-		EVP_PKEY_free(pkey);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
-	}
+	if (cctx == NULL)
+		goto err_pkey;
 
-	if (EVP_PKEY_decapsulate_init(cctx, params) != 1) {
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
-	}
+	if (EVP_PKEY_decapsulate_init(cctx, NULL) != 1)
+		goto err_decap;
 
 	if (EVP_PKEY_decapsulate(cctx, NULL, &keylen,
 		op->decap.cipher.data, op->decap.cipher.length) != 1) {
 		OPENSSL_LOG(ERR, "Failed to determine output length");
-		EVP_PKEY_free(pkey);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
+		goto err_decap;
 	}
 
 	if (keylen > op->decap.sk.length) {
 		OPENSSL_LOG(ERR, "Insufficient buffer for shared key");
-		EVP_PKEY_free(pkey);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
+		goto err_decap;
 	}
 
 	if (EVP_PKEY_decapsulate(cctx, op->decap.sk.data, &keylen,
 			op->decap.cipher.data, op->decap.cipher.length) != 1) {
 		OPENSSL_LOG(ERR, "Failed to decapsulate");
-		EVP_PKEY_free(pkey);
-		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
-		return -1;
+		goto err_decap;
 	}
 
 	op->decap.sk.length = keylen;
+	ret = 0;
+	cop->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
 
+err_decap:
 	EVP_PKEY_CTX_free(cctx);
+err_pkey:
 	EVP_PKEY_free(pkey);
-	ret = 0;
-	cop->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
+err_params:
+	OSSL_PARAM_free(params);
 	return ret;
 }
 
-- 
2.43.0