On Mon, Jul 29, 2013 at 04:52:12PM +0300, Felipe Balbi wrote:
> On Mon, Jul 29, 2013 at 05:15:58PM +0400, Alexey Khoroshilov wrote:
> > On 07/29/2013 04:52 PM, Felipe Balbi wrote:
> > > Hi,
> > >
> > > On Fri, Jul 26, 2013 at 07:26:05PM +0400, Alexey Khoroshilov wrote:
> > >> On 07/25/2013 09:30 PM, Felipe Balbi wrote:
> > >>> On Wed, Jul 24, 2013 at 12:20:17AM +0400, Alexey Khoroshilov wrote:
> > >>>> mv_u3d_nuke() expects to be calles with ep->u3d->lock held,
> > >>>> because mv_u3d_done() does. But mv_u3d_ep_disable() calls it
> > >>>> without lock that can lead to unpleasant consequences.
> > >>>>
> > >>>> Found by Linux Driver Verification project (linuxtesting.org).
> > >>>>
> > >>>> Signed-off-by: Alexey Khoroshilov <khoroshilov@xxxxxxxxx>
> > >>> which commit introduced the bug ? Which kernels are affected by this bug ?
> > >> The bug is present from the very beginning: commit 3d4eb9d of 15 June 2012.
> > >> So it is in the mainline since v3.5.
> > > Alright, do you want to have the same fix in stable kernels ? Is it
> > > necessary at all or do we consider it 'never worked before' and send it
> > > in the next merge window ?
> >
> > It is a tricky point. From one point of view it 'never worked before',
> > but as far as the bug leads to a data race with unpredictable
> > consequences I would prefer to have it fixed within 3.11 timeframe.
>
> Alright, I have already sent my pull request for v3.11-rc3, so once -rc4
> is tagged (in about a week) I'll send this patch.
BTW, I just found another bug in mv_u3d_core.c, here's a patch:
From f6bb304cff095219ecfa6daa082ea834a9cf52bf Mon Sep 17 00:00:00 2001
From: Felipe Balbi <balbi@xxxxxx>
Date: Mon, 29 Jul 2013 16:58:29 +0300
Subject: [PATCH] usb: gadget: mv_u3d_core: don't call ->disconnect() from
stop_activity()
mv_u3d_stop_activity() should not be calling ->disconnect()
directly as udc-core will already handle that for us.
Signed-off-by: Felipe Balbi <balbi@xxxxxx>
---
Completely untested. It's clear that mv u3d driver
shouldn't be calling gadget_driver->disconnect()
at that time, but perhaps there are other bugs in the
driver which this will uncover.
drivers/usb/gadget/mv_u3d_core.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/usb/gadget/mv_u3d_core.c b/drivers/usb/gadget/mv_u3d_core.c
index 7acbca9..948f77a 100644
--- a/drivers/usb/gadget/mv_u3d_core.c
+++ b/drivers/usb/gadget/mv_u3d_core.c
@@ -1397,13 +1397,6 @@ void mv_u3d_stop_activity(struct mv_u3d *u3d, struct usb_gadget_driver *driver)
list_for_each_entry(ep, &u3d->gadget.ep_list, ep.ep_list) {
mv_u3d_nuke(ep, -ESHUTDOWN);
}
-
- /* report disconnect; the driver is already quiesced */
- if (driver) {
- spin_unlock(&u3d->lock);
- driver->disconnect(&u3d->gadget);
- spin_lock(&u3d->lock);
- }
}
static void mv_u3d_irq_process_error(struct mv_u3d *u3d)
--
1.8.3.4.840.g6a90778
--
balbi
Attachment:
signature.asc
Description: Digital signature
