From: Peter Chen <peter.chen@nxp.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>,
Anton Vasilyev <vasilyev@ispras.ru>,
Evgeny Novikov <novikov@ispras.ru>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
USB mailing list <linux-usb@vger.kernel.org>
Subject: Re: [PATCH RFC 3/4] USB: UDC: net2280: Fix memory leaks
Date: Fri, 7 Aug 2020 09:44:46 +0000 [thread overview]
Message-ID: <20200807094406.GB17207@b29397-desktop> (raw)
In-Reply-To: <20200807033423.GA17207@b29397-desktop>
On 20-08-07 03:35:17, Peter Chen wrote:
> On 20-07-29 16:23:28, Alan Stern wrote:
> > 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.
> >
> > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> > Reported-By: Anton Vasilyev <vasilyev@ispras.ru>
> > Reported-By: Evgeny Novikov <novikov@ispras.ru>
> > CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >
> > ---
> >
> > drivers/usb/gadget/udc/net2280.c | 10 +++++++---
> > drivers/usb/gadget/udc/net2280.h | 1 +
> > 2 files changed, 8 insertions(+), 3 deletions(-)
> >
> > Index: usb-devel/drivers/usb/gadget/udc/net2280.c
> > ===================================================================
> > --- usb-devel.orig/drivers/usb/gadget/udc/net2280.c
> > +++ usb-devel/drivers/usb/gadget/udc/net2280.c
> > @@ -3572,7 +3572,8 @@ static void net2280_remove(struct pci_de
> > {
> > struct net2280 *dev = pci_get_drvdata(pdev);
> >
> > - usb_del_gadget_udc(&dev->gadget);
> > + if (dev->added)
> > + usb_del_gadget(&dev->gadget);
> >
> > BUG_ON(dev->driver);
> >
> > @@ -3603,6 +3604,7 @@ static void net2280_remove(struct pci_de
> > device_remove_file(&pdev->dev, &dev_attr_registers);
> >
> > ep_info(dev, "unbind\n");
> > + usb_put_gadget(&dev->gadget);
> > }
> >
> > /* wrap this driver around the specified device, but
> > @@ -3624,6 +3626,8 @@ static int net2280_probe(struct pci_dev
> > }
> >
> > pci_set_drvdata(pdev, dev);
> > + dev_set_drvdata(&dev->gadget.dev, dev);
>
> The gadget device's drvdata will be written by usb_composite_dev
> structure pointer at composite_bind, composite_bind is called after
> loading any gadget class drivers, so you can't depend on it at
> gadget_release.
>
I do some fixes about it for the new series, if anything is not suitable,
please commit.
--
Thanks,
Peter Chen
prev parent reply other threads:[~2020-08-07 9:44 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-29 20:23 [PATCH RFC 3/4] USB: UDC: net2280: Fix memory leaks Alan Stern
2020-08-07 3:35 ` Peter Chen
2020-08-07 9:44 ` Peter Chen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200807094406.GB17207@b29397-desktop \
--to=peter.chen@nxp.com \
--cc=benh@kernel.crashing.org \
--cc=felipe.balbi@linux.intel.com \
--cc=linux-usb@vger.kernel.org \
--cc=novikov@ispras.ru \
--cc=stern@rowland.harvard.edu \
--cc=vasilyev@ispras.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).