[PATCH 0/5] fix coverity issues

[PATCH 0/5] fix coverity issues

[Chaoyong He]

This patch series aims to fix some coverity issues:
- Coverity issue 385406
- Coverity issue 385412
- Coverity issue 405351
- Coverity issue 405352
- Coverity issue 405356

Chaoyong He (5):
  vdpa/nfp: fix checking return value
  vdpa/nfp: fix integer handling issues
  common/nfp: fix integer handling issues
  net/nfp: fix integer shift problem
  net/nfp: fix null pointer dereferences

 drivers/common/nfp/nfp_common.c      | 4 ++--
 drivers/net/nfp/nfdk/nfp_nfdk_dp.c   | 3 +++
 drivers/net/nfp/nfpcore/nfp_target.c | 2 +-
 drivers/vdpa/nfp/nfp_vdpa.c          | 4 +++-
 drivers/vdpa/nfp/nfp_vdpa_core.c     | 2 +-
 5 files changed, 10 insertions(+), 5 deletions(-)

-- 
2.39.1

[PATCH 1/5] vdpa/nfp: fix checking return value

[Chaoyong He]

CI found calling 'rte_vfio_get_group_num()' without checking return
value.

Coverity issue: 405356
Fixes: 7b2a1228c59d ("vdpa/nfp: add remap PCI memory")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/vdpa/nfp/nfp_vdpa.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/nfp/nfp_vdpa.c b/drivers/vdpa/nfp/nfp_vdpa.c
index a1d1dc4f73..cef80b5476 100644
--- a/drivers/vdpa/nfp/nfp_vdpa.c
+++ b/drivers/vdpa/nfp/nfp_vdpa.c
@@ -113,8 +113,10 @@ nfp_vdpa_vfio_setup(struct nfp_vdpa_dev *device)
 	rte_pci_unmap_device(pci_dev);
 
 	rte_pci_device_name(&pci_dev->addr, dev_name, RTE_DEV_NAME_MAX_LEN);
-	rte_vfio_get_group_num(rte_pci_get_sysfs_path(), dev_name,
+	ret = rte_vfio_get_group_num(rte_pci_get_sysfs_path(), dev_name,
 			&device->iommu_group);
+	if (ret <= 0)
+		return -1;
 
 	device->vfio_container_fd = rte_vfio_container_create();
 	if (device->vfio_container_fd < 0)
-- 
2.39.1

[PATCH 2/5] vdpa/nfp: fix integer handling issues

[Chaoyong He]

CI found integer handling issues, overflow before widen.

Coverity issue: 405352
Fixes: 76ea5ebef08e ("vdpa/nfp: add notify related logic")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/vdpa/nfp/nfp_vdpa_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vdpa/nfp/nfp_vdpa_core.c b/drivers/vdpa/nfp/nfp_vdpa_core.c
index e2a6253ae5..7b877605e4 100644
--- a/drivers/vdpa/nfp/nfp_vdpa_core.c
+++ b/drivers/vdpa/nfp/nfp_vdpa_core.c
@@ -176,7 +176,7 @@ uint64_t
 nfp_vdpa_get_queue_notify_offset(struct nfp_vdpa_hw *vdpa_hw __rte_unused,
 		int qid)
 {
-	return NFP_VDPA_NOTIFY_ADDR_BASE + (qid * NFP_VDPA_NOTIFY_ADDR_INTERVAL);
+	return NFP_VDPA_NOTIFY_ADDR_BASE + ((uint64_t)qid * NFP_VDPA_NOTIFY_ADDR_INTERVAL);
 }
 
 /*
-- 
2.39.1

[PATCH 3/5] common/nfp: fix integer handling issues

[Chaoyong He]

CI found integer handling issues, overflow before widen.

Coverity issue: 405351
Fixes: 87f5b35ba4e8 ("common/nfp: move queue logic")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/common/nfp/nfp_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/common/nfp/nfp_common.c b/drivers/common/nfp/nfp_common.c
index 4c94c9d59a..40e1620c2e 100644
--- a/drivers/common/nfp/nfp_common.c
+++ b/drivers/common/nfp/nfp_common.c
@@ -187,14 +187,14 @@ nfp_enable_queues(struct nfp_hw *hw,
 	/* Enabling the required TX queues in the device */
 	enabled_queues = 0;
 	for (i = 0; i < nb_tx_queues; i++)
-		enabled_queues |= (1 << i);
+		enabled_queues |= (1ULL << i);
 
 	nn_cfg_writeq(hw, NFP_NET_CFG_TXRS_ENABLE, enabled_queues);
 
 	/* Enabling the required RX queues in the device */
 	enabled_queues = 0;
 	for (i = 0; i < nb_rx_queues; i++)
-		enabled_queues |= (1 << i);
+		enabled_queues |= (1ULL << i);
 
 	nn_cfg_writeq(hw, NFP_NET_CFG_RXRS_ENABLE, enabled_queues);
 }
-- 
2.39.1

[PATCH 4/5] net/nfp: fix integer shift problem

[Chaoyong He]

CI found integer handling issues, bad shift.

Coverity issue: 385412
Fixes: e6858e7e7cd3 ("net/nfp: move shared target logic to own file")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfpcore/nfp_target.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfpcore/nfp_target.c b/drivers/net/nfp/nfpcore/nfp_target.c
index ecb45f7928..ea5b39a4b9 100644
--- a/drivers/net/nfp/nfpcore/nfp_target.c
+++ b/drivers/net/nfp/nfpcore/nfp_target.c
@@ -528,7 +528,7 @@ nfp_mask64(int msb,
 		return 0;
 
 	width = msb - lsb + 1;
-	if (width < 0)
+	if (width <= 0)
 		return 0;
 
 	if (width == 64)
-- 
2.39.1

[PATCH 5/5] net/nfp: fix null pointer dereferences

[Chaoyong He]

CI found null pointer dereferences problem.

Coverity issue: 385406
Fixes: d7f6d9b21ffa ("net/nfp: adjust coding style for NFDk")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Peng Zhang <peng.zhang@corigine.com>
---
 drivers/net/nfp/nfdk/nfp_nfdk_dp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/nfp/nfdk/nfp_nfdk_dp.c b/drivers/net/nfp/nfdk/nfp_nfdk_dp.c
index 1f330b7bb6..0141fbcc8f 100644
--- a/drivers/net/nfp/nfdk/nfp_nfdk_dp.c
+++ b/drivers/net/nfp/nfdk/nfp_nfdk_dp.c
@@ -301,6 +301,9 @@ nfp_net_nfdk_xmit_pkts_common(void *tx_queue,
 		uint64_t metadata = 0;
 
 		pkt = *(tx_pkts + npkts);
+		if (pkt == NULL)
+			goto xmit_end;
+
 		nop_descs = nfp_net_nfdk_tx_maybe_close_block(txq, pkt);
 		if (nop_descs < 0)
 			goto xmit_end;
-- 
2.39.1

Re: [PATCH 3/5] common/nfp: fix integer handling issues

[Thomas Monjalon]

15/11/2023 04:23, Chaoyong He:
> CI found integer handling issues, overflow before widen.
> 
> Coverity issue: 405351
> Fixes: 87f5b35ba4e8 ("common/nfp: move queue logic")
> Cc: stable@dpdk.org
[...]
> -		enabled_queues |= (1 << i);
> +		enabled_queues |= (1ULL << i);

That's a very bad fix.
You should use RTE_BIT64() which is more explicit.

Please read rte_bitops.h, that's a nice set of macros and functions.

RE: [PATCH 3/5] common/nfp: fix integer handling issues

[Chaoyong He]

> 15/11/2023 04:23, Chaoyong He:
> > CI found integer handling issues, overflow before widen.
> >
> > Coverity issue: 405351
> > Fixes: 87f5b35ba4e8 ("common/nfp: move queue logic")
> > Cc: stable@dpdk.org
> [...]
> > -		enabled_queues |= (1 << i);
> > +		enabled_queues |= (1ULL << i);
> 
> That's a very bad fix.
> You should use RTE_BIT64() which is more explicit.
> 
> Please read rte_bitops.h, that's a nice set of macros and functions.
> 

Okay, I got it now.
So, what should I do about it? Send out another patch to fix or something else?

Re: [PATCH 3/5] common/nfp: fix integer handling issues

[Thomas Monjalon]

23/11/2023 02:23, Chaoyong He:
> > 15/11/2023 04:23, Chaoyong He:
> > > CI found integer handling issues, overflow before widen.
> > >
> > > Coverity issue: 405351
> > > Fixes: 87f5b35ba4e8 ("common/nfp: move queue logic")
> > > Cc: stable@dpdk.org
> > [...]
> > > -		enabled_queues |= (1 << i);
> > > +		enabled_queues |= (1ULL << i);
> > 
> > That's a very bad fix.
> > You should use RTE_BIT64() which is more explicit.
> > 
> > Please read rte_bitops.h, that's a nice set of macros and functions.
> 
> Okay, I got it now.
> So, what should I do about it? Send out another patch to fix or something else?

You may convert your driver to use it where appropriate
if you feel it's worth.