| Message ID | 1455798876-32186-1-git-send-email-sudipm.mukherjee@gmail.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
On 18 February 2016 at 13:34, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > From: Sudip Mukherjee <sudip@vectorindia.org> > > On error we jumped to the label bcma_out and returned the error code but > we missed freeing dev. What if b43_one_core_attach fails? Won't we miss kfree then as well? Shouldn't you simply improve error path and maybe use an additional label? Also may we be missing a proper free in b43_bcma_remove as well? Your patch subject could also be a bit more descriptive (e.g. what memory leak exactly is it).
On Thu, 18 Feb 2016 18:04:36 +0530 Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > From: Sudip Mukherjee <sudip@vectorindia.org> > > On error we jumped to the label bcma_out and returned the error code but > we missed freeing dev. > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> > --- > drivers/net/wireless/broadcom/b43/main.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/wireless/broadcom/b43/main.c b/drivers/net/wireless/broadcom/b43/main.c > index c279211..78f670a 100644 > --- a/drivers/net/wireless/broadcom/b43/main.c > +++ b/drivers/net/wireless/broadcom/b43/main.c > @@ -5671,6 +5671,7 @@ static int b43_bcma_probe(struct bcma_device *core) > wl = b43_wireless_init(dev); > if (IS_ERR(wl)) { > err = PTR_ERR(wl); > + kfree(dev); > goto bcma_out; > } > We recently had a patch that fixes this, among more leaks. Subject: [PATCH v2 resend] b43: Fix memory leaks in b43_bus_dev_ssb_init and b43_bus_dev_bcma_init Please test that patch instead, so we can finally apply it. It needs to be tested on both ssb and bcma. Come on. This isn't too hard. :) Please somebody with any hardware test it. (I currently don't have any b43 hardware)
Michael Büsch <m@bues.ch> writes: > On Thu, 18 Feb 2016 18:04:36 +0530 > Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > >> From: Sudip Mukherjee <sudip@vectorindia.org> >> >> On error we jumped to the label bcma_out and returned the error code but >> we missed freeing dev. >> >> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> >> --- >> drivers/net/wireless/broadcom/b43/main.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/net/wireless/broadcom/b43/main.c b/drivers/net/wireless/broadcom/b43/main.c >> index c279211..78f670a 100644 >> --- a/drivers/net/wireless/broadcom/b43/main.c >> +++ b/drivers/net/wireless/broadcom/b43/main.c >> @@ -5671,6 +5671,7 @@ static int b43_bcma_probe(struct bcma_device *core) >> wl = b43_wireless_init(dev); >> if (IS_ERR(wl)) { >> err = PTR_ERR(wl); >> + kfree(dev); >> goto bcma_out; >> } >> > > We recently had a patch that fixes this, among more leaks. Subject: > [PATCH v2 resend] b43: Fix memory leaks in b43_bus_dev_ssb_init and > b43_bus_dev_bcma_init > > Please test that patch instead, so we can finally apply it. > > It needs to be tested on both ssb and bcma. Come on. This isn't too > hard. :) Please somebody with any hardware test it. (I currently don't > have any b43 hardware) And the patch can be downloaded from patchwork: https://patchwork.kernel.org/patch/8049041/
On Friday 19 February 2016 01:14 PM, Kalle Valo wrote: > Michael Büsch <m@bues.ch> writes: > >> On Thu, 18 Feb 2016 18:04:36 +0530 >> Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: >> >>> From: Sudip Mukherjee <sudip@vectorindia.org> >>> >>> On error we jumped to the label bcma_out and returned the error code but >>> we missed freeing dev. >>> >>> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> >>> --- >>> drivers/net/wireless/broadcom/b43/main.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/net/wireless/broadcom/b43/main.c b/drivers/net/wireless/broadcom/b43/main.c >>> index c279211..78f670a 100644 >>> --- a/drivers/net/wireless/broadcom/b43/main.c >>> +++ b/drivers/net/wireless/broadcom/b43/main.c >>> @@ -5671,6 +5671,7 @@ static int b43_bcma_probe(struct bcma_device *core) >>> wl = b43_wireless_init(dev); >>> if (IS_ERR(wl)) { >>> err = PTR_ERR(wl); >>> + kfree(dev); >>> goto bcma_out; >>> } >>> >> >> We recently had a patch that fixes this, among more leaks. Subject: >> [PATCH v2 resend] b43: Fix memory leaks in b43_bus_dev_ssb_init and >> b43_bus_dev_bcma_init >> >> Please test that patch instead, so we can finally apply it. >> >> It needs to be tested on both ssb and bcma. Come on. This isn't too >> hard. :) Please somebody with any hardware test it. (I currently don't >> have any b43 hardware) > > And the patch can be downloaded from patchwork: > > https://patchwork.kernel.org/patch/8049041/ I have an old laptop running on 800Mhz CPU. It has "Broadcom BCM4311 [14e4:4311] (rev 01)". I will try to test it on this weekend. regards sudip
On Fri, 19 Feb 2016 20:37:18 +0530 Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > > https://patchwork.kernel.org/patch/8049041/ > > I have an old laptop running on 800Mhz CPU. It has "Broadcom BCM4311 > [14e4:4311] (rev 01)". > I will try to test it on this weekend. Any news on this one?
On Thursday 10 March 2016 11:13 PM, Michael Büsch wrote: > On Fri, 19 Feb 2016 20:37:18 +0530 > Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote: > >>> https://patchwork.kernel.org/patch/8049041/ >> >> I have an old laptop running on 800Mhz CPU. It has "Broadcom BCM4311 >> [14e4:4311] (rev 01)". >> I will try to test it on this weekend. > > Any news on this one? No. Sorry. I was trying to install ubuntu 14.04 in it, but for some reason the usb stick is not moving past the boot screen. Give me two more days and I will let you all know by this Saturday. regards sudip
diff --git a/drivers/net/wireless/broadcom/b43/main.c b/drivers/net/wireless/broadcom/b43/main.c index c279211..78f670a 100644 --- a/drivers/net/wireless/broadcom/b43/main.c +++ b/drivers/net/wireless/broadcom/b43/main.c @@ -5671,6 +5671,7 @@ static int b43_bcma_probe(struct bcma_device *core) wl = b43_wireless_init(dev); if (IS_ERR(wl)) { err = PTR_ERR(wl); + kfree(dev); goto bcma_out; }