DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 1/3] crypto/ipsec_mb: fix qp setup null pointer dereference
@ 2021-12-10 14:09 Ciara Power
  2021-12-10 14:09 ` [PATCH 2/3] crypto/ipsec_mb: fix qp cleanup " Ciara Power
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Ciara Power @ 2021-12-10 14:09 UTC (permalink / raw)
  To: dev; +Cc: stable, john.mcnamara, roy.fan.zhang, Ciara Power, Pablo de Lara

When setting up a qp in a secondary process, the local qp pointer is set
to the stored device qp, configured by the primary process for that
device, but only if that device qp is not NULL.
If the device qp was not set up correctly by the primary process and has
a NULL value, the local qp variable stays at the default initialised
value, NULL. This causes a NULL pointer dereference later in the
function when using the qp value.

This is fixed by always setting the local qp to the value of the device
qp stored, and then checking if qp is NULL, returning an error if it is.

Coverity issue: 374382
Fixes: 72a169278a56 ("crypto/ipsec_mb: support multi-process")
Cc: stable@dpdk.org

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 drivers/crypto/ipsec_mb/ipsec_mb_ops.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
index 189262c4ad..6efa417d67 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
@@ -221,8 +221,11 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 				IMB_VERSION_STR, IMB_MP_REQ_VER_STR);
 		return -EINVAL;
 #endif
-		if (dev->data->queue_pairs[qp_id] != NULL)
-			qp = dev->data->queue_pairs[qp_id];
+		qp = dev->data->queue_pairs[qp_id];
+		if (qp == NULL) {
+			IPSEC_MB_LOG(ERR, "Primary process hasn't configured device qp.");
+			return -EINVAL;
+		}
 	} else {
 		/* Free memory prior to re-allocation if needed. */
 		if (dev->data->queue_pairs[qp_id] != NULL)
-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] crypto/ipsec_mb: fix qp cleanup null pointer dereference
  2021-12-10 14:09 [PATCH 1/3] crypto/ipsec_mb: fix qp setup null pointer dereference Ciara Power
@ 2021-12-10 14:09 ` Ciara Power
  2021-12-16 15:03   ` Zhang, Roy Fan
  2021-12-10 14:09 ` [PATCH 3/3] crypto/ipsec_mb: fix tainted data for session Ciara Power
  2021-12-16 15:02 ` [PATCH 1/3] crypto/ipsec_mb: fix qp setup null pointer dereference Zhang, Roy Fan
  2 siblings, 1 reply; 7+ messages in thread
From: Ciara Power @ 2021-12-10 14:09 UTC (permalink / raw)
  To: dev; +Cc: stable, john.mcnamara, roy.fan.zhang, Ciara Power, Pablo de Lara

The qp was being used in the cleanup without checking if it was non NULL.
A check is now added to verify qp is non NULL before use.

Coverity issue: 374375
Fixes: c75542ae4200 ("crypto/ipsec_mb: introduce IPsec_mb framework")
Cc: roy.fan.zhang@intel.com
Cc: stable@dpdk.org

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 drivers/crypto/ipsec_mb/ipsec_mb_ops.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
index 6efa417d67..1ebd23e8f0 100644
--- a/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
+++ b/drivers/crypto/ipsec_mb/ipsec_mb_ops.c
@@ -285,6 +285,8 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 	return 0;
 
 qp_setup_cleanup:
+	if (qp == NULL)
+		return ret;
 #if IMB_VERSION(1, 1, 0) > IMB_VERSION_NUM
 	if (qp->mb_mgr)
 		free_mb_mgr(qp->mb_mgr);
@@ -294,8 +296,7 @@ ipsec_mb_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 	if (qp->mb_mgr_mz)
 		rte_memzone_free(qp->mb_mgr_mz);
 #endif
-	if (qp)
-		rte_free(qp);
+	rte_free(qp);
 	return ret;
 }
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] crypto/ipsec_mb: fix tainted data for session
  2021-12-10 14:09 [PATCH 1/3] crypto/ipsec_mb: fix qp setup null pointer dereference Ciara Power
  2021-12-10 14:09 ` [PATCH 2/3] crypto/ipsec_mb: fix qp cleanup " Ciara Power
@ 2021-12-10 14:09 ` Ciara Power
  2021-12-16 15:03   ` Zhang, Roy Fan
  2021-12-24 12:55   ` [EXT] " Akhil Goyal
  2021-12-16 15:02 ` [PATCH 1/3] crypto/ipsec_mb: fix qp setup null pointer dereference Zhang, Roy Fan
  2 siblings, 2 replies; 7+ messages in thread
From: Ciara Power @ 2021-12-10 14:09 UTC (permalink / raw)
  To: dev
  Cc: stable, john.mcnamara, roy.fan.zhang, Ciara Power,
	piotrx.bronowski, Pablo de Lara

Downcasting a void * to struct aesni_gcm_session * caused the session
data to be treated as tainted.
Removing the void * temporary variable and adding a cast avoids this
issue.

Coverity issue: 374377
Fixes: 746825e5c0ea ("crypto/ipsec_mb: move aesni_gcm PMD")
Cc: piotrx.bronowski@intel.com
Cc: stable@dpdk.org

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 drivers/crypto/ipsec_mb/pmd_aesni_gcm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c b/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
index 2c203795ab..e5ad629fe5 100644
--- a/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
+++ b/drivers/crypto/ipsec_mb/pmd_aesni_gcm.c
@@ -713,19 +713,17 @@ aesni_gcm_process_bulk(struct rte_cryptodev *dev,
 			__rte_unused union rte_crypto_sym_ofs ofs,
 			struct rte_crypto_sym_vec *vec)
 {
-	void *sess_priv;
 	struct aesni_gcm_session *s;
 	struct gcm_context_data gdata_ctx;
 	IMB_MGR *mb_mgr;
 
-	sess_priv = get_sym_session_private_data(sess, dev->driver_id);
-	if (unlikely(sess_priv == NULL)) {
+	s = (struct aesni_gcm_session *) get_sym_session_private_data(sess,
+		dev->driver_id);
+	if (unlikely(s == NULL)) {
 		aesni_gcm_fill_error_code(vec, EINVAL);
 		return 0;
 	}
 
-	s = sess_priv;
-
 	/* get per-thread MB MGR, create one if needed */
 	mb_mgr = get_per_thread_mb_mgr();
 	if (unlikely(mb_mgr == NULL))
-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 1/3] crypto/ipsec_mb: fix qp setup null pointer dereference
  2021-12-10 14:09 [PATCH 1/3] crypto/ipsec_mb: fix qp setup null pointer dereference Ciara Power
  2021-12-10 14:09 ` [PATCH 2/3] crypto/ipsec_mb: fix qp cleanup " Ciara Power
  2021-12-10 14:09 ` [PATCH 3/3] crypto/ipsec_mb: fix tainted data for session Ciara Power
@ 2021-12-16 15:02 ` Zhang, Roy Fan
  2 siblings, 0 replies; 7+ messages in thread
From: Zhang, Roy Fan @ 2021-12-16 15:02 UTC (permalink / raw)
  To: Power, Ciara, dev; +Cc: stable, Mcnamara, John, De Lara Guarch, Pablo

> -----Original Message-----
> From: Power, Ciara <ciara.power@intel.com>
> Sent: Friday, December 10, 2021 2:10 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Mcnamara, John <john.mcnamara@intel.com>; Zhang,
> Roy Fan <roy.fan.zhang@intel.com>; Power, Ciara <ciara.power@intel.com>;
> De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Subject: [PATCH 1/3] crypto/ipsec_mb: fix qp setup null pointer dereference
> 
> When setting up a qp in a secondary process, the local qp pointer is set
> to the stored device qp, configured by the primary process for that
> device, but only if that device qp is not NULL.
> If the device qp was not set up correctly by the primary process and has
> a NULL value, the local qp variable stays at the default initialised
> value, NULL. This causes a NULL pointer dereference later in the
> function when using the qp value.
> 
> This is fixed by always setting the local qp to the value of the device
> qp stored, and then checking if qp is NULL, returning an error if it is.
> 
> Coverity issue: 374382
> Fixes: 72a169278a56 ("crypto/ipsec_mb: support multi-process")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 2/3] crypto/ipsec_mb: fix qp cleanup null pointer dereference
  2021-12-10 14:09 ` [PATCH 2/3] crypto/ipsec_mb: fix qp cleanup " Ciara Power
@ 2021-12-16 15:03   ` Zhang, Roy Fan
  0 siblings, 0 replies; 7+ messages in thread
From: Zhang, Roy Fan @ 2021-12-16 15:03 UTC (permalink / raw)
  To: Power, Ciara, dev; +Cc: stable, Mcnamara, John, De Lara Guarch, Pablo

> -----Original Message-----
> From: Power, Ciara <ciara.power@intel.com>
> Sent: Friday, December 10, 2021 2:10 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Mcnamara, John <john.mcnamara@intel.com>; Zhang,
> Roy Fan <roy.fan.zhang@intel.com>; Power, Ciara <ciara.power@intel.com>;
> De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Subject: [PATCH 2/3] crypto/ipsec_mb: fix qp cleanup null pointer
> dereference
> 
> The qp was being used in the cleanup without checking if it was non NULL.
> A check is now added to verify qp is non NULL before use.
> 
> Coverity issue: 374375
> Fixes: c75542ae4200 ("crypto/ipsec_mb: introduce IPsec_mb framework")
> Cc: roy.fan.zhang@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> ---
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 3/3] crypto/ipsec_mb: fix tainted data for session
  2021-12-10 14:09 ` [PATCH 3/3] crypto/ipsec_mb: fix tainted data for session Ciara Power
@ 2021-12-16 15:03   ` Zhang, Roy Fan
  2021-12-24 12:55   ` [EXT] " Akhil Goyal
  1 sibling, 0 replies; 7+ messages in thread
From: Zhang, Roy Fan @ 2021-12-16 15:03 UTC (permalink / raw)
  To: Power, Ciara, dev
  Cc: stable, Mcnamara, John, Bronowski, PiotrX, De Lara Guarch, Pablo

> -----Original Message-----
> From: Power, Ciara <ciara.power@intel.com>
> Sent: Friday, December 10, 2021 2:10 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Mcnamara, John <john.mcnamara@intel.com>; Zhang,
> Roy Fan <roy.fan.zhang@intel.com>; Power, Ciara <ciara.power@intel.com>;
> Bronowski, PiotrX <piotrx.bronowski@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: [PATCH 3/3] crypto/ipsec_mb: fix tainted data for session
> 
> Downcasting a void * to struct aesni_gcm_session * caused the session
> data to be treated as tainted.
> Removing the void * temporary variable and adding a cast avoids this
> issue.
> 
> Coverity issue: 374377
> Fixes: 746825e5c0ea ("crypto/ipsec_mb: move aesni_gcm PMD")
> Cc: piotrx.bronowski@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> ---
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [EXT] [PATCH 3/3] crypto/ipsec_mb: fix tainted data for session
  2021-12-10 14:09 ` [PATCH 3/3] crypto/ipsec_mb: fix tainted data for session Ciara Power
  2021-12-16 15:03   ` Zhang, Roy Fan
@ 2021-12-24 12:55   ` Akhil Goyal
  1 sibling, 0 replies; 7+ messages in thread
From: Akhil Goyal @ 2021-12-24 12:55 UTC (permalink / raw)
  To: Ciara Power, dev
  Cc: stable, john.mcnamara, roy.fan.zhang, piotrx.bronowski, Pablo de Lara

> Downcasting a void * to struct aesni_gcm_session * caused the session
> data to be treated as tainted.
> Removing the void * temporary variable and adding a cast avoids this
> issue.
> 
> Coverity issue: 374377
> Fixes: 746825e5c0ea ("crypto/ipsec_mb: move aesni_gcm PMD")
> Cc: piotrx.bronowski@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>
Series Applied to dpdk-next-crypto

Thanks.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-12-24 12:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-10 14:09 [PATCH 1/3] crypto/ipsec_mb: fix qp setup null pointer dereference Ciara Power
2021-12-10 14:09 ` [PATCH 2/3] crypto/ipsec_mb: fix qp cleanup " Ciara Power
2021-12-16 15:03   ` Zhang, Roy Fan
2021-12-10 14:09 ` [PATCH 3/3] crypto/ipsec_mb: fix tainted data for session Ciara Power
2021-12-16 15:03   ` Zhang, Roy Fan
2021-12-24 12:55   ` [EXT] " Akhil Goyal
2021-12-16 15:02 ` [PATCH 1/3] crypto/ipsec_mb: fix qp setup null pointer dereference Zhang, Roy Fan

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).