Re: [PATCH] usb: gadget: udc: add null check before pointer dereference

[Gustavo A. R. Silva]

Hi Alan,

Quoting Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>:

> On Mon, 1 May 2017, Gustavo A. R. Silva wrote:
>
> > Add null check before dereferencing dev->regs pointer inside
> > net2280_led_shutdown() function.
> >
> > Addresses-Coverity-ID: 101783
> > Signed-off-by: Gustavo A. R. Silva <garsilva@xxxxxxxxxxxxxx>
> > ---
> >  drivers/usb/gadget/udc/net2280.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/net2280.c  
> > b/drivers/usb/gadget/udc/net2280.c
> > index 3828c2e..1898a4b 100644
> > --- a/drivers/usb/gadget/udc/net2280.c
> > +++ b/drivers/usb/gadget/udc/net2280.c
> > @@ -3573,7 +3573,11 @@ static void net2280_remove(struct pci_dev *pdev)
> >  	BUG_ON(dev->driver);
> >
> >  	/* then clean up the resources we allocated during probe() */
> > -	net2280_led_shutdown(dev);
> > +
> > +	if (dev->regs) {
> > +		net2280_led_shutdown(dev);
> > +		iounmap(dev->regs);
> > +	}
> >  	if (dev->requests) {
> >  		int		i;
> >  		for (i = 1; i < 5; i++) {
> > @@ -3588,8 +3592,6 @@ static void net2280_remove(struct pci_dev *pdev)
> >  		free_irq(pdev->irq, dev);
> >  	if (dev->quirks & PLX_PCIE)
> >  		pci_disable_msi(pdev);
> > -	if (dev->regs)
> > -		iounmap(dev->regs);
> >  	if (dev->region)
> >  		release_mem_region(pci_resource_start(pdev, 0),
> >  				pci_resource_len(pdev, 0));
>
> No, you must not move the iounmap() call, because an interrupt could
> theoretically occur at any time.

Yeah, I was suspicious about it.

> Either just live with an extra test of dev->regs, or else move the
> net2280_led_shutdown() call later.

In this case I think it is safe to move the net2280_led_shutdown()  
call, as the function is only turning off the LEDs.

I'll send a patch shortly.
Thank you
--
Gustavo A. R. Silva