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

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

[Jesper Juhl]

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 <jesper.juhl@gmail.com>
---

 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;
 	}

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

[Thomas Dahlmann]

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 <jesper.juhl@gmail.com>
> ---
>
>  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: Thomas.Dahlmann@amd.com
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