[3/6] drivers: fix bad bit shift operation

Message ID 20211101175337.83358-3-hkalra@marvell.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series [1/6] interrupts: fix argument cannot be negative |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Harman Kalra Nov. 1, 2021, 5:53 p.m. UTC
  This patch fixes coverity issue by adding a check for
negative value to avoid bad bit shift operation.

Coverity issue: 373717,373697,373685
Fixes: d61138d4f0e2 ("drivers: remove direct access to interrupt handle")

Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
 drivers/net/e1000/igb_ethdev.c | 17 +++++++++++------
 drivers/net/igc/igc_ethdev.c   | 18 +++++++++++++-----
 2 files changed, 24 insertions(+), 11 deletions(-)
  

Comments

Wang, Haiyue Nov. 2, 2021, 1:22 a.m. UTC | #1
> -----Original Message-----
> From: Harman Kalra <hkalra@marvell.com>
> Sent: Tuesday, November 2, 2021 01:54
> To: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>
> Cc: david.marchand@redhat.com; Mcnamara, John <john.mcnamara@intel.com>; Harman Kalra
> <hkalra@marvell.com>
> Subject: [PATCH 3/6] drivers: fix bad bit shift operation
> 
> This patch fixes coverity issue by adding a check for
> negative value to avoid bad bit shift operation.
> 
> Coverity issue: 373717,373697,373685
> Fixes: d61138d4f0e2 ("drivers: remove direct access to interrupt handle")
> 
> Signed-off-by: Harman Kalra <hkalra@marvell.com>
> ---
>  drivers/net/e1000/igb_ethdev.c | 17 +++++++++++------
>  drivers/net/igc/igc_ethdev.c   | 18 +++++++++++++-----

Acked-by: Haiyue Wang <haiyue.wang@intel.com>

> --
> 2.18.0
  

Patch

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index ff06575f03..031a880b6a 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -5194,7 +5194,7 @@  eth_igb_assign_msix_vector(struct e1000_hw *hw, int8_t direction,
 static void
 eth_igb_configure_msix_intr(struct rte_eth_dev *dev)
 {
-	int queue_id;
+	int queue_id, nb_efd;
 	uint32_t tmpval, regval, intr_mask;
 	struct e1000_hw *hw =
 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -5243,9 +5243,11 @@  eth_igb_configure_msix_intr(struct rte_eth_dev *dev)
 		E1000_WRITE_REG(hw, E1000_GPIE, E1000_GPIE_MSIX_MODE |
 					E1000_GPIE_PBA | E1000_GPIE_EIAME |
 					E1000_GPIE_NSICR);
-		intr_mask =
-			RTE_LEN2MASK(rte_intr_nb_efd_get(intr_handle),
-				     uint32_t) << misc_shift;
+		nb_efd = rte_intr_nb_efd_get(intr_handle);
+		if (nb_efd < 0)
+			return;
+
+		intr_mask = RTE_LEN2MASK(nb_efd, uint32_t) << misc_shift;
 
 		if (dev->data->dev_conf.intr_conf.lsc != 0)
 			intr_mask |= (1 << IGB_MSIX_OTHER_INTR_VEC);
@@ -5263,8 +5265,11 @@  eth_igb_configure_msix_intr(struct rte_eth_dev *dev)
 	/* use EIAM to auto-mask when MSI-X interrupt
 	 * is asserted, this saves a register write for every interrupt
 	 */
-	intr_mask = RTE_LEN2MASK(rte_intr_nb_efd_get(intr_handle),
-				 uint32_t) << misc_shift;
+	nb_efd = rte_intr_nb_efd_get(intr_handle);
+	if (nb_efd < 0)
+		return;
+
+	intr_mask = RTE_LEN2MASK(nb_efd, uint32_t) << misc_shift;
 
 	if (dev->data->dev_conf.intr_conf.lsc != 0)
 		intr_mask |= (1 << IGB_MSIX_OTHER_INTR_VEC);
diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c
index 8189ad412a..6652c3c806 100644
--- a/drivers/net/igc/igc_ethdev.c
+++ b/drivers/net/igc/igc_ethdev.c
@@ -727,7 +727,7 @@  igc_configure_msix_intr(struct rte_eth_dev *dev)
 	uint32_t vec = IGC_MISC_VEC_ID;
 	uint32_t base = IGC_MISC_VEC_ID;
 	uint32_t misc_shift = 0;
-	int i;
+	int i, nb_efd;
 
 	/* won't configure msix register if no mapping is done
 	 * between intr vector and event fd
@@ -745,8 +745,12 @@  igc_configure_msix_intr(struct rte_eth_dev *dev)
 	IGC_WRITE_REG(hw, IGC_GPIE, IGC_GPIE_MSIX_MODE |
 				IGC_GPIE_PBA | IGC_GPIE_EIAME |
 				IGC_GPIE_NSICR);
-	intr_mask = RTE_LEN2MASK(rte_intr_nb_efd_get(intr_handle),
-				 uint32_t) << misc_shift;
+
+	nb_efd = rte_intr_nb_efd_get(intr_handle);
+	if (nb_efd < 0)
+		return;
+
+	intr_mask = RTE_LEN2MASK(nb_efd, uint32_t) << misc_shift;
 
 	if (dev->data->dev_conf.intr_conf.lsc)
 		intr_mask |= (1u << IGC_MSIX_OTHER_INTR_VEC);
@@ -802,6 +806,7 @@  igc_rxq_interrupt_setup(struct rte_eth_dev *dev)
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 	int misc_shift = rte_intr_allow_others(intr_handle) ? 1 : 0;
+	int nb_efd;
 
 	/* won't configure msix register if no mapping is done
 	 * between intr vector and event fd
@@ -809,8 +814,11 @@  igc_rxq_interrupt_setup(struct rte_eth_dev *dev)
 	if (!rte_intr_dp_is_en(intr_handle))
 		return;
 
-	mask = RTE_LEN2MASK(rte_intr_nb_efd_get(intr_handle), uint32_t)
-		<< misc_shift;
+	nb_efd = rte_intr_nb_efd_get(intr_handle);
+	if (nb_efd < 0)
+		return;
+
+	mask = RTE_LEN2MASK(nb_efd, uint32_t) << misc_shift;
 	IGC_WRITE_REG(hw, IGC_EIMS, mask);
 }