common/qat: fix null dereference in release function

Message ID 20240308094411.5467-1-arkadiuszx.kusztal@intel.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series common/qat: fix null dereference in release function |

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/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

Arkadiusz Kusztal March 8, 2024, 9:44 a.m. UTC
  This commit fixes NULL dereference in the release function,
and three additional coverity issues related to NULL check.

Coverity issue: 415038
Coverity issue: 415050
Coverity issue: 415052
Coverity issue: 415053
Fixes: 477d7d051211 ("common/qat: decouple drivers from common code")

Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
---
 drivers/common/qat/qat_device.c     | 14 +++++++-------
 drivers/compress/qat/qat_comp_pmd.c |  6 ++++--
 drivers/crypto/qat/qat_asym.c       |  5 ++---
 drivers/crypto/qat/qat_sym.c        |  4 ++--
 4 files changed, 15 insertions(+), 14 deletions(-)
  

Comments

Power, Ciara March 11, 2024, 2:39 p.m. UTC | #1
> -----Original Message-----
> From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Sent: Friday, March 8, 2024 9:44 AM
> To: dev@dpdk.org
> Cc: gakhil@marvell.com; Power, Ciara <ciara.power@intel.com>; Kusztal,
> ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Subject: [PATCH] common/qat: fix null dereference in release function
> 
> This commit fixes NULL dereference in the release function, and three additional
> coverity issues related to NULL check.
> 
> Coverity issue: 415038
> Coverity issue: 415050
> Coverity issue: 415052
> Coverity issue: 415053
> Fixes: 477d7d051211 ("common/qat: decouple drivers from common code")
> 
> Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  drivers/common/qat/qat_device.c     | 14 +++++++-------
>  drivers/compress/qat/qat_comp_pmd.c |  6 ++++--
>  drivers/crypto/qat/qat_asym.c       |  5 ++---
>  drivers/crypto/qat/qat_sym.c        |  4 ++--
>  4 files changed, 15 insertions(+), 14 deletions(-)
> 

Acked-by: Ciara Power <ciara.power@intel.com>
  
Akhil Goyal March 13, 2024, 2:51 p.m. UTC | #2
> > Subject: [PATCH] common/qat: fix null dereference in release function
> >
> > This commit fixes NULL dereference in the release function, and three
> additional
> > coverity issues related to NULL check.
> >
> > Coverity issue: 415038
> > Coverity issue: 415050
> > Coverity issue: 415052
> > Coverity issue: 415053
> > Fixes: 477d7d051211 ("common/qat: decouple drivers from common code")
> >
> > Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> >  drivers/common/qat/qat_device.c     | 14 +++++++-------
> >  drivers/compress/qat/qat_comp_pmd.c |  6 ++++--
> >  drivers/crypto/qat/qat_asym.c       |  5 ++---
> >  drivers/crypto/qat/qat_sym.c        |  4 ++--
> >  4 files changed, 15 insertions(+), 14 deletions(-)
> >
> 
> Acked-by: Ciara Power <ciara.power@intel.com>
Applied to dpdk-next-crypto
Thanks.
  

Patch

diff --git a/drivers/common/qat/qat_device.c b/drivers/common/qat/qat_device.c
index 500ca0f308..666e2bb995 100644
--- a/drivers/common/qat/qat_device.c
+++ b/drivers/common/qat/qat_device.c
@@ -420,15 +420,15 @@  qat_pci_device_release(struct rte_pci_device *pci_dev)
 
 		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 			for (i = 0; i < QAT_MAX_SERVICES; i++) {
-				if (qat_dev->pmd[i] == NULL)
-					continue;
-				QAT_LOG(DEBUG, "QAT %s device %s is busy",
-					qat_service[i].name, name);
-				busy = 1;
+				if (qat_dev->pmd[i] != NULL) {
+					QAT_LOG(DEBUG, "QAT %s device %s is busy",
+						qat_service[i].name, name);
+					busy = 1;
+				}
+			}
 			if (busy)
 				return -EBUSY;
 			rte_memzone_free(inst->mz);
-			}
 		}
 		memset(inst, 0, sizeof(struct qat_device_info));
 		qat_nb_pci_devices--;
@@ -445,7 +445,7 @@  qat_pci_dev_destroy(struct qat_pci_device *qat_pci_dev,
 	int i;
 
 	for (i = 0; i < QAT_MAX_SERVICES; i++) {
-		if (!qat_service[i].dev_create)
+		if (!qat_service[i].dev_destroy)
 			continue;
 		qat_service[i].dev_destroy(qat_pci_dev);
 	}
diff --git a/drivers/compress/qat/qat_comp_pmd.c b/drivers/compress/qat/qat_comp_pmd.c
index c5f7c7dc67..55e510c91f 100644
--- a/drivers/compress/qat/qat_comp_pmd.c
+++ b/drivers/compress/qat/qat_comp_pmd.c
@@ -792,12 +792,14 @@  qat_comp_dev_create(struct qat_pci_device *qat_pci_dev)
 static int
 qat_comp_dev_destroy(struct qat_pci_device *qat_pci_dev)
 {
-	struct qat_comp_dev_private *dev =
-		qat_pci_dev->pmd[QAT_SERVICE_COMPRESSION];
+	struct qat_comp_dev_private *dev;
 
 	if (qat_pci_dev == NULL)
 		return -ENODEV;
 
+	dev = qat_pci_dev->pmd[QAT_SERVICE_COMPRESSION];
+	if (dev == NULL)
+		return 0;
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memzone_free(dev->capa_mz);
 
diff --git a/drivers/crypto/qat/qat_asym.c b/drivers/crypto/qat/qat_asym.c
index 8ec9f65156..14d6ec358c 100644
--- a/drivers/crypto/qat/qat_asym.c
+++ b/drivers/crypto/qat/qat_asym.c
@@ -1625,16 +1625,15 @@  static int
 qat_asym_dev_destroy(struct qat_pci_device *qat_pci_dev)
 {
 	struct rte_cryptodev *cryptodev;
-	struct qat_cryptodev_private *dev =
-		qat_pci_dev->pmd[QAT_SERVICE_ASYMMETRIC];
+	struct qat_cryptodev_private *dev;
 
 	if (qat_pci_dev == NULL)
 		return -ENODEV;
+	dev = qat_pci_dev->pmd[QAT_SERVICE_ASYMMETRIC];
 	if (dev == NULL)
 		return 0;
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memzone_free(dev->capa_mz);
-
 	/* free crypto device */
 	cryptodev = rte_cryptodev_pmd_get_dev(dev->dev_id);
 	rte_cryptodev_pmd_destroy(cryptodev);
diff --git a/drivers/crypto/qat/qat_sym.c b/drivers/crypto/qat/qat_sym.c
index 6c7b1724ef..c530496786 100644
--- a/drivers/crypto/qat/qat_sym.c
+++ b/drivers/crypto/qat/qat_sym.c
@@ -345,11 +345,11 @@  static int
 qat_sym_dev_destroy(struct qat_pci_device *qat_pci_dev)
 {
 	struct rte_cryptodev *cryptodev;
-	struct qat_cryptodev_private *dev =
-		qat_pci_dev->pmd[QAT_SERVICE_SYMMETRIC];
+	struct qat_cryptodev_private *dev;
 
 	if (qat_pci_dev == NULL)
 		return -ENODEV;
+	dev = qat_pci_dev->pmd[QAT_SERVICE_SYMMETRIC];
 	if (dev == NULL)
 		return 0;
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)