From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 54766A00C5; Thu, 7 May 2020 08:30:22 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 31C0B1D9B4; Thu, 7 May 2020 08:30:22 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 842411D9B3 for ; Thu, 7 May 2020 08:30:20 +0200 (CEST) IronPort-SDR: s53lBIAyHa9ZDs6ZWdd5XKn63MR4LG9sNFcS/NW8F8qZUKV+ts8qT+6HfS8FxeJE/qSCJqxsep HJnR1wuRbIyQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 May 2020 23:30:20 -0700 IronPort-SDR: HOveSxSplIALlyPDUBZviVfH6WICiQ59O5yDGxSClzc/KyJKyrS25PmhZZnmiq5/VCYJKjPAnZ El0c6R9U/zRQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,362,1583222400"; d="scan'208";a="295618781" Received: from jguo15x-mobl.ccr.corp.intel.com (HELO [10.67.68.173]) ([10.67.68.173]) by fmsmga002.fm.intel.com with ESMTP; 06 May 2020 23:30:18 -0700 To: alvinx.zhang@intel.com, dev@dpdk.org Cc: wei.zhao1@intel.com, xiaolong.ye@intel.com References: <20200507020133.14412-1-alvinx.zhang@intel.com> From: Jeff Guo Message-ID: Date: Thu, 7 May 2020 14:30:17 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: <20200507020133.14412-1-alvinx.zhang@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH] net/igc: fix memory illegal accesses X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" hi, alvin On 5/7/2020 10:01 AM, alvinx.zhang@intel.com wrote: > From: Alvin Zhang > > Add memory access out-of-bounds check. Could you explain why add ...?  If not add, what issue? > > Fixes: bd3fcf0d0fa1 (net/igc: support RSS) > Cc: stable@dpdk.org > > Signed-off-by: Alvin Zhang > --- > drivers/net/igc/igc_ethdev.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c > index 16d98c6..ced8ffd 100644 > --- a/drivers/net/igc/igc_ethdev.c > +++ b/drivers/net/igc/igc_ethdev.c > @@ -2266,6 +2266,8 @@ static int eth_igc_vlan_tpid_set(struct rte_eth_dev *dev, > return -EINVAL; > } > > + RTE_BUILD_BUG_ON(ETH_RSS_RETA_SIZE_128 % IGC_RSS_RDT_REG_SIZE); > + > /* set redirection table */ > for (i = 0; i < ETH_RSS_RETA_SIZE_128; i += IGC_RSS_RDT_REG_SIZE) { > union igc_rss_reta_reg reta, reg; > @@ -2278,7 +2280,8 @@ static int eth_igc_vlan_tpid_set(struct rte_eth_dev *dev, > IGC_RSS_RDT_REG_SIZE_MASK); > > /* if no need to update the register */ > - if (!mask) > + if (!mask || > + shift > (RTE_RETA_GROUP_SIZE - IGC_RSS_RDT_REG_SIZE)) alignment should be match above parentheses. > continue; > > /* check mask whether need to read the register value first */ > @@ -2289,6 +2292,7 @@ static int eth_igc_vlan_tpid_set(struct rte_eth_dev *dev, > IGC_RETA(i / IGC_RSS_RDT_REG_SIZE)); > > /* update the register */ > + RTE_BUILD_BUG_ON(sizeof(reta.bytes) != IGC_RSS_RDT_REG_SIZE); > for (j = 0; j < IGC_RSS_RDT_REG_SIZE; j++) { > if (mask & (1u << j)) > reta.bytes[j] = > @@ -2318,6 +2322,8 @@ static int eth_igc_vlan_tpid_set(struct rte_eth_dev *dev, > return -EINVAL; > } > > + RTE_BUILD_BUG_ON(ETH_RSS_RETA_SIZE_128 % IGC_RSS_RDT_REG_SIZE); > + > /* read redirection table */ > for (i = 0; i < ETH_RSS_RETA_SIZE_128; i += IGC_RSS_RDT_REG_SIZE) { > union igc_rss_reta_reg reta; > @@ -2330,12 +2336,14 @@ static int eth_igc_vlan_tpid_set(struct rte_eth_dev *dev, > IGC_RSS_RDT_REG_SIZE_MASK); > > /* if no need to read register */ > - if (!mask) > + if (!mask || > + shift > (RTE_RETA_GROUP_SIZE - IGC_RSS_RDT_REG_SIZE)) the same as above. > continue; > > /* read register and get the queue index */ > reta.dword = IGC_READ_REG_LE_VALUE(hw, > IGC_RETA(i / IGC_RSS_RDT_REG_SIZE)); > + RTE_BUILD_BUG_ON(sizeof(reta.bytes) != IGC_RSS_RDT_REG_SIZE); need null line or no need, please check it at the place and before. > for (j = 0; j < IGC_RSS_RDT_REG_SIZE; j++) { > if (mask & (1u << j)) > reta_conf[idx].reta[shift + j] = reta.bytes[j];