From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Kulasek Subject: [PATCH v2] ixgbe: fix bad shift operation in ixgbe_set_pool_rx/tx Date: Fri, 22 Apr 2016 17:35:57 +0200 Message-ID: <1461339357-8048-1-git-send-email-tomaszx.kulasek@intel.com> References: <1460727549-4380-1-git-send-email-tomaszx.kulasek@intel.com> Cc: dev@dpdk.org To: wenzhuo.lu@intel.com, helin.zhang@intel.com, konstantin.ananyev@intel.com, bruce.richardson@intel.com Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 51BBE2949 for ; Fri, 22 Apr 2016 17:36:26 +0200 (CEST) In-Reply-To: <1460727549-4380-1-git-send-email-tomaszx.kulasek@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Fix issue reported by Coverity. Coverity ID 13193: Bad bit shift operation (BAD_SHIFT) large_shift: In expression 1 << pool, left shifting by more than 31 bits has undefined behavior. The shift amount, pool, is at least 32. This patch is a rework of register addr selection logic and mask computation to made it more readable and avoid bit overflow when 32 bit value is shifted over its size for pool > 31. Fixes: fe3a45fd4104 ("ixgbe: add VMDq support") Signed-off-by: Tomasz Kulasek --- v2: - joined two patches for same issue in tx/rx - added pool parameter checking for invalid value - reworked register selection logic and mask shift in more explicit way drivers/net/ixgbe/ixgbe_ethdev.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index 3f1ebc1..eed2662 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -4399,9 +4399,19 @@ ixgbe_set_pool_rx(struct rte_eth_dev *dev, uint16_t pool, uint8_t on) if (ixgbe_vmdq_mode_check(hw) < 0) return -ENOTSUP; - addr = IXGBE_VFRE(pool >= ETH_64_POOLS/2); + if (pool >= ETH_64_POOLS) + return -EINVAL; + + /* for pool >= 32, set bit in PFVFRE[1], otherwise PFVFRE[0] */ + if (pool >= 32) { + addr = IXGBE_VFRE(1); + val = bit1 << (pool - 32); + } else { + addr = IXGBE_VFRE(0); + val = bit1 << pool; + } + reg = IXGBE_READ_REG(hw, addr); - val = bit1 << pool; if (on) reg |= val; @@ -4426,9 +4436,19 @@ ixgbe_set_pool_tx(struct rte_eth_dev *dev, uint16_t pool, uint8_t on) if (ixgbe_vmdq_mode_check(hw) < 0) return -ENOTSUP; - addr = IXGBE_VFTE(pool >= ETH_64_POOLS/2); + if (pool >= ETH_64_POOLS) + return -EINVAL; + + /* for pool >= 32, set bit in PFVFTE[1], otherwise PFVFTE[0] */ + if (pool >= 32) { + addr = IXGBE_VFTE(1); + val = bit1 << (pool - 32); + } else { + addr = IXGBE_VFTE(0); + val = bit1 << pool; + } + reg = IXGBE_READ_REG(hw, addr); - val = bit1 << pool; if (on) reg |= val; -- 1.7.9.5