> -----Original Message----- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Declan Doherty > Sent: Friday, December 12, 2014 6:40 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH] bond: static analysis issues fix > > Fixes for link bonding library identified by static analysis tool > > - 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 > > Signed-off-by: Declan Doherty <declan.doherty at intel.com> > --- > 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(-) > > 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 == BONDING_MODE_8023AD) > bond_mode_8023ad_activate_slave(eth_dev, port_id); > > - internals->active_slaves[internals->active_slave_count] = port_id; > - internals->active_slave_count++; > + if (internals->active_slave_count < > + RTE_DIM(internals->active_slaves) - 1) { > + internals->active_slaves[internals->active_slave_count] = > port_id; > + internals->active_slave_count++; > + } > } > > void > @@ -144,7 +147,8 @@ deactivate_slave(struct rte_eth_dev *eth_dev, uint8_t > port_id) > sizeof(internals->active_slaves[0])); > } > > - internals->active_slave_count = active_count; > + internals->active_slave_count = 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 itself. I think if you want to make this static analysis tool happy it should be changed to RTE_VERIFY(), assert(), rte_panic() or something like that to indicate undefined state. Pawel
