Re: [PATCH v3 2/6] USB: UDC: net2280: Fix memory leaks

[Peter Chen <peter.chen@nxp.com>]

On 20-08-13 09:57:13, Alan Stern wrote:
> On Thu, Aug 13, 2020 at 11:19:49AM +0800, Peter Chen wrote:
> > From: Alan Stern <stern@rowland.harvard.edu>
> > 
> > As Anton and Evgeny have noted, the net2280 UDC driver has a problem
> > with leaking memory along some of its failure pathways.  It also has
> > another problem, not previously noted, in that some of the failure
> > pathways will call usb_del_gadget_udc() without first calling
> > usb_add_gadget_udc_release().  And it leaks memory by calling kfree()
> > when it should call put_device().
> > 
> > Previous attempts to fix the problems have failed because of lack of
> > support in the UDC core for separately initializing and adding
> > gadgets, or for separately deleting and freeing gadgets.  The previous
> > patch in this series adds the necessary support, making it possible to
> > fix the outstanding problems properly.
> > 
> > This patch adds an "added" flag to the net2280 structure to indicate
> > whether or not the gadget has been registered (and thus whether or not
> > to call usb_del_gadget()), and it fixes the deallocation issues by
> > calling usb_put_gadget() at the appropriate point.
> > 
> > A similar memory leak issue, apparently never before recognized, stems
> > from the fact that the driver never initializes the drvdata field in
> > the gadget's embedded struct device!  Evidently this wasn't noticed
> > because the pointer is only ever used as an argument to kfree(), which
> > doesn't mind getting called with a NULL pointer. In fact, the drvdata
> > for gadget device will be written by usb_composite_dev structure if
> > any gadget class is loaded, so it needs to use usb_gadget structure
> > to get net2280 private data.
> > 
> > CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Reported-by: Anton Vasilyev <vasilyev@ispras.ru>
> > Reported-by: Evgeny Novikov <novikov@ispras.ru>
> > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> > ---
> >  drivers/usb/gadget/udc/net2280.c | 13 +++++++++----
> >  drivers/usb/gadget/udc/net2280.h |  1 +
> >  2 files changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
> > index 7530bd9a08c4..31e49cc34316 100644
> > --- a/drivers/usb/gadget/udc/net2280.c
> > +++ b/drivers/usb/gadget/udc/net2280.c
> > @@ -3561,7 +3561,9 @@ static irqreturn_t net2280_irq(int irq, void *_dev)
> >  
> >  static void gadget_release(struct device *_dev)
> >  {
> > -	struct net2280	*dev = dev_get_drvdata(_dev);
> > +	struct usb_gadget	*gadget = container_of(_dev,
> > +					struct usb_gadget, dev);
> > +	struct net2280	*dev = container_of(gadget, struct net2280, gadget);
> 
> Please change this to
> 
> 	struct net2280	*dev = container_of(_dev, struct net2280, gadget,dev);
> 
> And do the same for the net2272 patch.
> 

Sorry, my oops. Please skip this patch set.

-- 

Thanks,
Peter Chen