2008-03-21 22:38:39

by Jesper Juhl

Subject: [PATCH] mem leak fixes for AMD 5536 UDC high/full speed USB device controller driver

In drivers/usb/gadget/amd5536udc.c::udc_pci_probe(),
sizeof(struct udc) storage is allocated for 'dev'.

There are many exit points from the function where
'dev' is not free'd but has also not yet been used
for anything. The following patch free's 'dev' at the
return points where it has not yet been used.

Please consider applying.


Signed-off-by: Jesper Juhl <[email protected]>
---

amd5536udc.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
index b663f23..fc6f348 100644
--- a/drivers/usb/gadget/amd5536udc.c
+++ b/drivers/usb/gadget/amd5536udc.c
@@ -3248,6 +3248,8 @@ static int udc_pci_probe(

/* pci setup */
if (pci_enable_device(pdev) < 0) {
+ kfree(dev);
+ dev = 0;
retval = -ENODEV;
goto finished;
}
@@ -3259,6 +3261,8 @@ static int udc_pci_probe(

if (!request_mem_region(resource, len, name)) {
dev_dbg(&pdev->dev, "pci device used already\n");
+ kfree(dev);
+ dev = 0;
retval = -EBUSY;
goto finished;
}
@@ -3267,18 +3271,24 @@ static int udc_pci_probe(
dev->virt_addr = ioremap_nocache(resource, len);
if (dev->virt_addr == NULL) {
dev_dbg(&pdev->dev, "start address cannot be mapped\n");
+ kfree(dev);
+ dev = 0;
retval = -EFAULT;
goto finished;
}

if (!pdev->irq) {
dev_err(&dev->pdev->dev, "irq not set\n");
+ kfree(dev);
+ dev = 0;
retval = -ENODEV;
goto finished;
}

if (request_irq(pdev->irq, udc_irq, IRQF_SHARED, name, dev) != 0) {
dev_dbg(&dev->pdev->dev, "request_irq(%d) fail\n", pdev->irq);
+ kfree(dev);
+ dev = 0;
retval = -EBUSY;
goto finished;
}

2008-03-26 13:04:01

by Thomas Dahlmann

Subject: Re: [PATCH] mem leak fixes for AMD 5536 UDC high/full speed USB device controller driver

ACK, thanks.

While testing some of these error cases I found that
there is even some more improvement possible which
I will have to do e.g. changing udc_pci_remove().

Thomas

Jesper Juhl wrote:
> In drivers/usb/gadget/amd5536udc.c::udc_pci_probe(),
> sizeof(struct udc) storage is allocated for 'dev'.
>
> There are many exit points from the function where
> 'dev' is not free'd but has also not yet been used
> for anything. The following patch free's 'dev' at the
> return points where it has not yet been used.
>
> Please consider applying.
>
>
> Signed-off-by: Jesper Juhl <[email protected]>
> ---
>
> amd5536udc.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
> index b663f23..fc6f348 100644
> --- a/drivers/usb/gadget/amd5536udc.c
> +++ b/drivers/usb/gadget/amd5536udc.c
> @@ -3248,6 +3248,8 @@ static int udc_pci_probe(
>
> /* pci setup */
> if (pci_enable_device(pdev) < 0) {
> + kfree(dev);
> + dev = 0;
> retval = -ENODEV;
> goto finished;
> }
> @@ -3259,6 +3261,8 @@ static int udc_pci_probe(
>
> if (!request_mem_region(resource, len, name)) {
> dev_dbg(&pdev->dev, "pci device used already\n");
> + kfree(dev);
> + dev = 0;
> retval = -EBUSY;
> goto finished;
> }
> @@ -3267,18 +3271,24 @@ static int udc_pci_probe(
> dev->virt_addr = ioremap_nocache(resource, len);
> if (dev->virt_addr == NULL) {
> dev_dbg(&pdev->dev, "start address cannot be mapped\n");
> + kfree(dev);
> + dev = 0;
> retval = -EFAULT;
> goto finished;
> }
>
> if (!pdev->irq) {
> dev_err(&dev->pdev->dev, "irq not set\n");
> + kfree(dev);
> + dev = 0;
> retval = -ENODEV;
> goto finished;
> }
>
> if (request_irq(pdev->irq, udc_irq, IRQF_SHARED, name, dev) != 0) {
> dev_dbg(&dev->pdev->dev, "request_irq(%d) fail\n", pdev->irq);
> + kfree(dev);
> + dev = 0;
> retval = -EBUSY;
> goto finished;
> }
>
>
>
>
>


--
Thomas Dahlmann
Software Engineer
AMD, Customer Support Services
voice: (+49 351) 277-6017
fax : (+49 351) 277-96017
email: [email protected]
http://www.amd.com

AMD Saxony Limited Liability Company & Co. KG
Sitz (Gesch?ftsanschrift): Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
Vertretungsberechtigter Komplement?r: AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Gesch?ftsf?hrer der AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy