On Tue, Apr 11, 2017 at 10:12:01AM -0400, Alan Stern wrote:
> On Tue, 11 Apr 2017, Felipe Balbi wrote:
> 
> > > Oddly enough, yes.  But it doesn't explain why this code doesn't blow 
> > > up every time it gets called, in its current form.
> > 
> > Well, it does :-)
> > 
> > dev_get_drvdata(_dev) -> NULL -> kfree(NULL)
> > 
> > We're just leaking memory. I guess a patch like below would be best:
> > 
> > diff --git a/drivers/usb/gadget/udc/net2280.c 
> > b/drivers/usb/gadget/udc/net2280.c
> > index 3828c2ec8623..4dc04253da61 100644
> > --- a/drivers/usb/gadget/udc/net2280.c
> > +++ b/drivers/usb/gadget/udc/net2280.c
> > @@ -3555,13 +3555,6 @@ static irqreturn_t net2280_irq(int irq, void *_dev)
> >  
> >  
> > /*-------------------------------------------------------------------------*/
> >  
> > -static void gadget_release(struct device *_dev)
> > -{
> > -   struct net2280  *dev = dev_get_drvdata(_dev);
> > -
> > -   kfree(dev);
> > -}
> > -
> >  /* tear down the binding between this driver and the pci device */
> >  
> >  static void net2280_remove(struct pci_dev *pdev)
> > @@ -3598,6 +3591,8 @@ static void net2280_remove(struct pci_dev *pdev)
> >     device_remove_file(&pdev->dev, &dev_attr_registers);
> >  
> >     ep_info(dev, "unbind\n");
> > +
> > +   kfree(dev);
> >  }
> >  
> >  /* wrap this driver around the specified device, but
> > @@ -3775,8 +3770,7 @@ static int net2280_probe(struct pci_dev *pdev, const 
> > struct pci_device_id *id)
> >     if (retval)
> >             goto done;
> >  
> > -   retval = usb_add_gadget_udc_release(&pdev->dev, &dev->gadget,
> > -                   gadget_release);
> > +   retval = usb_add_gadget_udc(&pdev->dev, &dev->gadget);
> >     if (retval)
> >             goto done;
> >     return 0;
> 
> Maybe...  But I can't shake the feeling that Greg KH would strongly 
> disagree.  Hasn't he said, many times in the past, that any dynamically 
> allocated device structure _must_ have a real release routine?  
> usb_udc_nop_release() doesn't qualify.

Aw, I wanted to publically yell at someone like the kernel documentation
says I am allowed to do so if anyone does such a foolish thing :)

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to