Patchwork [1/6] usb/fsl_qe_udc: Fix oops on QE UDC probe failure

login
register
mail settings
Submitter Anton Vorontsov
Date Nov. 11, 2008, 4:03 p.m.
Message ID <20081111160320.GA24699@oksana.dev.rtsoft.ru>
Download mbox | patch
Permalink /patch/8159/
State Superseded
Delegated to: Kumar Gala
Headers show

Comments

Anton Vorontsov - Nov. 11, 2008, 4:03 p.m.
In case of probing errors the driver kfrees the udc_controller, but it
doesn't set the pointer to NULL.

When usb_gadget_register_driver is called, it checks for udc_controller
!= NULL, the check passes and the driver accesses nonexistent memory.
Fix this by setting udc_controller to NULL in case of errors.

While at it, also implement irq_of_parse_and_map()'s failure and cleanup
cases.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/usb/gadget/fsl_qe_udc.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
David Brownell - Nov. 18, 2008, 1:55 a.m.
On Tuesday 11 November 2008, Anton Vorontsov wrote:
> In case of probing errors the driver kfrees the udc_controller, but it
> doesn't set the pointer to NULL.
> 
> When usb_gadget_register_driver is called, it checks for udc_controller
> != NULL, the check passes and the driver accesses nonexistent memory.
> Fix this by setting udc_controller to NULL in case of errors.
> 
> While at it, also implement irq_of_parse_and_map()'s failure and cleanup
> cases.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>

Acked-by: David Brownell <dbrownell@users.sourceforge.net>

I seem to detect a lot of bugfix activity here, which
tends to reflect usage ... good!  :)


> ---
>  drivers/usb/gadget/fsl_qe_udc.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/gadget/fsl_qe_udc.c b/drivers/usb/gadget/fsl_qe_udc.c
> index 94c38e4..60b9279 100644
> --- a/drivers/usb/gadget/fsl_qe_udc.c
> +++ b/drivers/usb/gadget/fsl_qe_udc.c
> @@ -2601,6 +2601,10 @@ static int __devinit qe_udc_probe(struct of_device *ofdev,
>  			(unsigned long)udc_controller);
>  	/* request irq and disable DR  */
>  	udc_controller->usb_irq = irq_of_parse_and_map(np, 0);
> +	if (!udc_controller->usb_irq) {
> +		ret = -EINVAL;
> +		goto err_noirq;
> +	}
>  
>  	ret = request_irq(udc_controller->usb_irq, qe_udc_irq, 0,
>  				driver_name, udc_controller);
> @@ -2622,6 +2626,8 @@ static int __devinit qe_udc_probe(struct of_device *ofdev,
>  err6:
>  	free_irq(udc_controller->usb_irq, udc_controller);
>  err5:
> +	irq_dispose_mapping(udc_controller->usb_irq);
> +err_noirq:
>  	if (udc_controller->nullmap) {
>  		dma_unmap_single(udc_controller->gadget.dev.parent,
>  			udc_controller->nullp, 256,
> @@ -2645,7 +2651,7 @@ err2:
>  	iounmap(udc_controller->usb_regs);
>  err1:
>  	kfree(udc_controller);
> -
> +	udc_controller = NULL;
>  	return ret;
>  }
>  
> @@ -2707,6 +2713,7 @@ static int __devexit qe_udc_remove(struct of_device *ofdev)
>  	kfree(ep->txframe);
>  
>  	free_irq(udc_controller->usb_irq, udc_controller);
> +	irq_dispose_mapping(udc_controller->usb_irq);
>  
>  	tasklet_kill(&udc_controller->rx_tasklet);
>  
> -- 
> 1.5.6.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
>

Patch

diff --git a/drivers/usb/gadget/fsl_qe_udc.c b/drivers/usb/gadget/fsl_qe_udc.c
index 94c38e4..60b9279 100644
--- a/drivers/usb/gadget/fsl_qe_udc.c
+++ b/drivers/usb/gadget/fsl_qe_udc.c
@@ -2601,6 +2601,10 @@  static int __devinit qe_udc_probe(struct of_device *ofdev,
 			(unsigned long)udc_controller);
 	/* request irq and disable DR  */
 	udc_controller->usb_irq = irq_of_parse_and_map(np, 0);
+	if (!udc_controller->usb_irq) {
+		ret = -EINVAL;
+		goto err_noirq;
+	}
 
 	ret = request_irq(udc_controller->usb_irq, qe_udc_irq, 0,
 				driver_name, udc_controller);
@@ -2622,6 +2626,8 @@  static int __devinit qe_udc_probe(struct of_device *ofdev,
 err6:
 	free_irq(udc_controller->usb_irq, udc_controller);
 err5:
+	irq_dispose_mapping(udc_controller->usb_irq);
+err_noirq:
 	if (udc_controller->nullmap) {
 		dma_unmap_single(udc_controller->gadget.dev.parent,
 			udc_controller->nullp, 256,
@@ -2645,7 +2651,7 @@  err2:
 	iounmap(udc_controller->usb_regs);
 err1:
 	kfree(udc_controller);
-
+	udc_controller = NULL;
 	return ret;
 }
 
@@ -2707,6 +2713,7 @@  static int __devexit qe_udc_remove(struct of_device *ofdev)
 	kfree(ep->txframe);
 
 	free_irq(udc_controller->usb_irq, udc_controller);
+	irq_dispose_mapping(udc_controller->usb_irq);
 
 	tasklet_kill(&udc_controller->rx_tasklet);