From: Alan Stern <stern@rowland.harvard.edu>
To: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Roger Quadros <rogerq@ti.com>, Peter Chen <peter.chen@nxp.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: [PATCH RFC 1/4] USB: UDC: Don't wipe deallocated memory
Date: Wed, 29 Jul 2020 16:22:31 -0400 [thread overview]
Message-ID: <20200729202231.GB1584059@rowland.harvard.edu> (raw)
Abusing the kernel's device model, some UDC drivers (including
dwc3 and cdns3) register and unregister their gadget structures
multiple times. This is strictly forbidden; device structures may not
be reused.
Commit fac323471df6 ("usb: udc: allow adding and removing the same
gadget device") attempted to work around this restriction by zeroing
out the memory occupied by a gadget's embedded struct device when the
gadget is unregistered. However, it does so at the wrong time,
immediately following the call to device_unregister(). At that point
there may still be outstanding references to the device, and
overwriting its memory is likely to cause trouble. Even worse, if
there are no outstanding references then the gadget's memory may have
been deallocated, and so gadget must be considered to be a stale
pointer when it is passed to memset().
This patch fixes the problem by moving the offending memset to the
device's release routine, which gets called only when all references
have been dropped. (Actually the call gets moved to the default
release routine, renamed from "usb_udc_nop_release" to
"usb_udc_zero_release" to indicate that it now zeroes out the memory.
This routine is the one used by dwc3 and cdns3; other drivers may not
use it.)
This doesn't fix the underlying problem. UDC drivers that register
their gadgets multiple times should be rewritten to allocate their
gadget structures dynamically, using a new one each time. Until that
is done, this will at least remove one potential source of errors.
On the other hand, the patch may create new errors if the release
routine doesn't get called until after the gadget has been
re-registered.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: Roger Quadros <rogerq@ti.com>
CC: Peter Chen <peter.chen@nxp.com>
CC: Anton Vasilyev <vasilyev@ispras.ru>
CC: Evgeny Novikov <novikov@ispras.ru>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/usb/gadget/udc/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Index: usb-devel/drivers/usb/gadget/udc/core.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/udc/core.c
+++ usb-devel/drivers/usb/gadget/udc/core.c
@@ -1138,9 +1138,10 @@ static void usb_udc_release(struct devic
static const struct attribute_group *usb_udc_attr_groups[];
-static void usb_udc_nop_release(struct device *dev)
+static void usb_udc_zero_release(struct device *dev)
{
dev_vdbg(dev, "%s\n", __func__);
+ memset(dev, 0, sizeof(*dev));
}
/* should be called with udc_lock held */
@@ -1184,7 +1185,7 @@ int usb_add_gadget_udc_release(struct de
if (release)
gadget->dev.release = release;
else
- gadget->dev.release = usb_udc_nop_release;
+ gadget->dev.release = usb_udc_zero_release;
device_initialize(&gadget->dev);
@@ -1338,7 +1339,6 @@ void usb_del_gadget_udc(struct usb_gadge
flush_work(&gadget->work);
device_unregister(&udc->dev);
device_unregister(&gadget->dev);
- memset(&gadget->dev, 0x00, sizeof(gadget->dev));
}
EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
next reply other threads:[~2020-07-29 20:22 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-29 20:22 Alan Stern [this message]
2020-07-30 3:28 ` [PATCH RFC 1/4] USB: UDC: Don't wipe deallocated memory Peter Chen
2020-07-30 5:19 ` Greg KH
2020-07-30 7:31 ` Peter Chen
2020-07-30 15:07 ` Alan Stern
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=20200729202231.GB1584059@rowland.harvard.edu \
--to=stern@rowland.harvard.edu \
--cc=benh@kernel.crashing.org \
--cc=felipe.balbi@linux.intel.com \
--cc=linux-usb@vger.kernel.org \
--cc=novikov@ispras.ru \
--cc=peter.chen@nxp.com \
--cc=rogerq@ti.com \
--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).