From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 415FD2E8A for ; Fri, 12 Dec 2014 19:31:12 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP; 12 Dec 2014 10:28:39 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,565,1413270000"; d="scan'208";a="622891846" Received: from irsmsx108.ger.corp.intel.com ([163.33.3.3]) by orsmga001.jf.intel.com with ESMTP; 12 Dec 2014 10:30:42 -0800 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.213]) by IRSMSX108.ger.corp.intel.com ([169.254.11.150]) with mapi id 14.03.0195.001; Fri, 12 Dec 2014 18:30:42 +0000 From: "Wodkowski, PawelX" To: "Doherty, Declan" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] bond: static analysis issues fix Thread-Index: AQHQFjK82a2DJa4jo0m7Coca1/ieTZyMRJdQ Date: Fri, 12 Dec 2014 18:30:42 +0000 Message-ID: References: <1418405982-21407-1-git-send-email-declan.doherty@intel.com> In-Reply-To: <1418405982-21407-1-git-send-email-declan.doherty@intel.com> Accept-Language: pl-PL, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] bond: static analysis issues fix X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Dec 2014 18:31:12 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Declan Doherty > Sent: Friday, December 12, 2014 6:40 PM > To: dev@dpdk.org > Subject: [dpdk-dev] [PATCH] bond: static analysis issues fix >=20 > Fixes for link bonding library identified by static analysis tool >=20 > - Overflow check for active_slaves array in activate_slave function > - Allocation check of pci_id_table in rte_eth_bond_create > - Use of eth_dev pointer in mac_address_get/set before NULL check >=20 > Signed-off-by: Declan Doherty > --- > lib/librte_pmd_bond/rte_eth_bond_api.c | 12 ++++++++---- > lib/librte_pmd_bond/rte_eth_bond_pmd.c | 8 ++++---- > 2 files changed, 12 insertions(+), 8 deletions(-) >=20 > diff --git a/lib/librte_pmd_bond/rte_eth_bond_api.c > b/lib/librte_pmd_bond/rte_eth_bond_api.c > index ef5ddf4..9cb1c1f 100644 > --- a/lib/librte_pmd_bond/rte_eth_bond_api.c > +++ b/lib/librte_pmd_bond/rte_eth_bond_api.c > @@ -115,8 +115,11 @@ activate_slave(struct rte_eth_dev *eth_dev, uint8_t > port_id) > if (internals->mode =3D=3D BONDING_MODE_8023AD) > bond_mode_8023ad_activate_slave(eth_dev, port_id); >=20 > - internals->active_slaves[internals->active_slave_count] =3D port_id; > - internals->active_slave_count++; > + if (internals->active_slave_count < > + RTE_DIM(internals->active_slaves) - 1) { > + internals->active_slaves[internals->active_slave_count] =3D > port_id; > + internals->active_slave_count++; > + } > } >=20 > void > @@ -144,7 +147,8 @@ deactivate_slave(struct rte_eth_dev *eth_dev, uint8_t > port_id) > sizeof(internals->active_slaves[0])); > } >=20 > - internals->active_slave_count =3D active_count; > + internals->active_slave_count =3D active_count < RTE_MAX_ETHPORTS ? > + active_count : RTE_MAX_ETHPORTS - 1; Since port might not be added twice and active_slaves array is (should be) proper size to contain every port you can add to bonding and in fact is one element bigger and active_slave_count should newer overflow, those changes might only mask real problems in user application and/or library it= self. I think if you want to make this static analysis tool happy it should be ch= anged to RTE_VERIFY(), assert(), rte_panic() or something like that to indicate undefined state. Pawel