| Message ID | 20200425134007.15843-1-zhengdejin5@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net,v1] net: acenic: fix an issue about leak related system resources | expand |
On Sat, Apr 25, 2020 at 4:40 PM Dejin Zheng <zhengdejin5@gmail.com> wrote: > > the function ace_allocate_descriptors() and ace_init() can fail in > the acenic_probe_one(), The related system resources were not > released then. so change the error handling to fix it. ... > @@ -568,7 +568,7 @@ static int acenic_probe_one(struct pci_dev *pdev, > #endif > > if (ace_allocate_descriptors(dev)) > - goto fail_free_netdev; > + goto fail_uninit; Not sure. The code is quite old and requires a lot of refactoring. Briefly looking the error path there is quite twisted. > @@ -580,7 +580,7 @@ static int acenic_probe_one(struct pci_dev *pdev, > #endif > > if (ace_init(dev)) > - goto fail_free_netdev; > + goto fail_uninit; This change seems incorrect, the ace_init() calls ace_init_cleanup() on error. So, your change makes it call the cleanup() twice.
On Mon, Apr 27, 2020 at 01:40:02PM +0300, Andy Shevchenko wrote: > On Sat, Apr 25, 2020 at 4:40 PM Dejin Zheng <zhengdejin5@gmail.com> wrote: > > > > the function ace_allocate_descriptors() and ace_init() can fail in > > the acenic_probe_one(), The related system resources were not > > released then. so change the error handling to fix it. > > ... > > > @@ -568,7 +568,7 @@ static int acenic_probe_one(struct pci_dev *pdev, > > #endif > > > > if (ace_allocate_descriptors(dev)) > > - goto fail_free_netdev; > > + goto fail_uninit; > > Not sure. > The code is quite old and requires a lot of refactoring. > > Briefly looking the error path there is quite twisted. > > > @@ -580,7 +580,7 @@ static int acenic_probe_one(struct pci_dev *pdev, > > #endif > > > > if (ace_init(dev)) > > - goto fail_free_netdev; > > + goto fail_uninit; > > This change seems incorrect, the ace_init() calls ace_init_cleanup() on error. > So, your change makes it call the cleanup() twice. > Hi Andy: Yes, this code is quite old, and There are also some mistakes in my patch, abandon this commit first. Thanks! BR, Dejin > -- > With Best Regards, > Andy Shevchenko
diff --git a/drivers/net/ethernet/alteon/acenic.c b/drivers/net/ethernet/alteon/acenic.c index 5d192d551623..32f9e68dd323 100644 --- a/drivers/net/ethernet/alteon/acenic.c +++ b/drivers/net/ethernet/alteon/acenic.c @@ -568,7 +568,7 @@ static int acenic_probe_one(struct pci_dev *pdev, #endif if (ace_allocate_descriptors(dev)) - goto fail_free_netdev; + goto fail_uninit; #ifdef MODULE if (boards_found >= ACE_MAX_MOD_PARMS) @@ -580,7 +580,7 @@ static int acenic_probe_one(struct pci_dev *pdev, #endif if (ace_init(dev)) - goto fail_free_netdev; + goto fail_uninit; if (register_netdev(dev)) { printk(KERN_ERR "acenic: device registration failed\n");
the function ace_allocate_descriptors() and ace_init() can fail in the acenic_probe_one(), The related system resources were not released then. so change the error handling to fix it. Fixes: 1da177e4c3f41524e8 ("Linux-2.6.12-rc2") Cc: Andy Shevchenko <andy.shevchenko@gmail.com> Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com> --- drivers/net/ethernet/alteon/acenic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)