| Message ID | 1438778204-8896-1-git-send-email-bernard.iremonger@intel.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
Tested-by: Marvin Liu <yong.liu@intel.com> > -----Original Message----- > From: Iremonger, Bernard > Sent: Wednesday, August 05, 2015 8:37 PM > To: dev@dpdk.org > Cc: Jastrzebski, MichalX K; Liu, Yong; Iremonger, Bernard > Subject: [PATCH v2 1/1] bonding: fix error handling in > rte_eth_bond_create() > > if the name parameter to rte_eth_bond_create() was NULL, > there was a segmentation fault because eth_dev was also NULL. > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com> > --- > drivers/net/bonding/rte_eth_bond_api.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/bonding/rte_eth_bond_api.c > b/drivers/net/bonding/rte_eth_bond_api.c > index 4ca26dd..0681d1a 100644 > --- a/drivers/net/bonding/rte_eth_bond_api.c > +++ b/drivers/net/bonding/rte_eth_bond_api.c > @@ -239,6 +239,10 @@ rte_eth_bond_create(const char *name, uint8_t mode, > uint8_t socket_id) > > eth_dev->data->mac_addrs = rte_zmalloc_socket(name, ETHER_ADDR_LEN, > 0, > socket_id); > + if (eth_dev->data->mac_addrs == NULL) { > + RTE_BOND_LOG(ERR, "Unable to malloc mac_addrs"); > + goto err; > + } > > eth_dev->data->dev_started = 0; > eth_dev->data->promiscuous = 0; > @@ -285,8 +289,10 @@ rte_eth_bond_create(const char *name, uint8_t mode, > uint8_t socket_id) > err: > rte_free(pci_dev); > rte_free(internals); > - rte_free(eth_dev->data->mac_addrs); > - > + if (eth_dev != NULL) { > + rte_free(eth_dev->data->mac_addrs); > + rte_eth_dev_release_port(eth_dev); > + } > return -1; > } > > -- > 1.9.1
2015-08-05 13:36, Bernard Iremonger: > if the name parameter to rte_eth_bond_create() was NULL, > there was a segmentation fault because eth_dev was also NULL. You also add error handling of mac_addrs alloc and release_port(). It deserves to be said in this commit message.
Hi Thomas, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Wednesday, August 5, 2015 2:16 PM > To: Iremonger, Bernard > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/1] bonding: fix error handling in > rte_eth_bond_create() > > 2015-08-05 13:36, Bernard Iremonger: > > if the name parameter to rte_eth_bond_create() was NULL, there was a > > segmentation fault because eth_dev was also NULL. > > You also add error handling of mac_addrs alloc and release_port(). > It deserves to be said in this commit message. Will I send a v3 patch ? Regards, Bernard.
2015-08-05 13:19, Iremonger, Bernard: > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > 2015-08-05 13:36, Bernard Iremonger: > > > if the name parameter to rte_eth_bond_create() was NULL, there was a > > > segmentation fault because eth_dev was also NULL. > > > > You also add error handling of mac_addrs alloc and release_port(). > > It deserves to be said in this commit message. > > Will I send a v3 patch ? My comment was mostly to show which kind of information may be useful. Sometimes I write it myself. You are welcome to write it in order I add it or you can send a v3. Other note: the function names are avoided in commit titles as they are redundant with the title prefix and not so easy to read. The title could be bonding: fix crash on initialization error or, to include other fixes, bonding: fix device initialization error handling Thanks
Hi Thomas, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Wednesday, August 5, 2015 2:35 PM > To: Iremonger, Bernard > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/1] bonding: fix error handling in > rte_eth_bond_create() > > 2015-08-05 13:19, Iremonger, Bernard: > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > > 2015-08-05 13:36, Bernard Iremonger: > > > > if the name parameter to rte_eth_bond_create() was NULL, there was > > > > a segmentation fault because eth_dev was also NULL. > > > > > > You also add error handling of mac_addrs alloc and release_port(). > > > It deserves to be said in this commit message. > > > > Will I send a v3 patch ? > > My comment was mostly to show which kind of information may be useful. > Sometimes I write it myself. You are welcome to write it in order I add it or > you can send a v3. > > Other note: the function names are avoided in commit titles as they are > redundant with the title prefix and not so easy to read. > The title could be > bonding: fix crash on initialization error or, to include other fixes, > bonding: fix device initialization error handling > > Thanks I will send a v3. Regards, Bernard.
diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c index 4ca26dd..0681d1a 100644 --- a/drivers/net/bonding/rte_eth_bond_api.c +++ b/drivers/net/bonding/rte_eth_bond_api.c @@ -239,6 +239,10 @@ rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id) eth_dev->data->mac_addrs = rte_zmalloc_socket(name, ETHER_ADDR_LEN, 0, socket_id); + if (eth_dev->data->mac_addrs == NULL) { + RTE_BOND_LOG(ERR, "Unable to malloc mac_addrs"); + goto err; + } eth_dev->data->dev_started = 0; eth_dev->data->promiscuous = 0; @@ -285,8 +289,10 @@ rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id) err: rte_free(pci_dev); rte_free(internals); - rte_free(eth_dev->data->mac_addrs); - + if (eth_dev != NULL) { + rte_free(eth_dev->data->mac_addrs); + rte_eth_dev_release_port(eth_dev); + } return -1; }
if the name parameter to rte_eth_bond_create() was NULL, there was a segmentation fault because eth_dev was also NULL. Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com> --- drivers/net/bonding/rte_eth_bond_api.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)