lkml.org 
[lkml]   [2018]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 062/111] USB: net2280: Fix erroneous synchronization change
    Date
    4.9-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Alan Stern <stern@rowland.harvard.edu>

    commit dec3c23c9aa1815f07d98ae0375b4cbc10971e13 upstream.

    Commit f16443a034c7 ("USB: gadgetfs, dummy-hcd, net2280: fix locking
    for callbacks") was based on a serious misunderstanding. It
    introduced regressions into both the dummy-hcd and net2280 drivers.

    The problem in dummy-hcd was fixed by commit 7dbd8f4cabd9 ("USB:
    dummy-hcd: Fix erroneous synchronization change"), but the problem in
    net2280 remains. Namely: the ->disconnect(), ->suspend(), ->resume(),
    and ->reset() callbacks must be invoked without the private lock held;
    otherwise a deadlock will occur when the callback routine tries to
    interact with the UDC driver.

    This patch largely is a reversion of the relevant parts of
    f16443a034c7. It also drops the private lock around the calls to
    ->suspend() and ->resume() (something the earlier patch forgot to do).
    This is safe from races with device interrupts because it occurs
    within the interrupt handler.

    Finally, the patch changes where the ->disconnect() callback is
    invoked when net2280_pullup() turns the pullup off. Rather than
    making the callback from within stop_activity() at a time when dropping
    the private lock could be unsafe, the callback is moved to a point
    after the lock has already been dropped.

    Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
    Fixes: f16443a034c7 ("USB: gadgetfs, dummy-hcd, net2280: fix locking for callbacks")
    Reported-by: D. Ziesche <dziesche@zes.com>
    Tested-by: D. Ziesche <dziesche@zes.com>
    CC: <stable@vger.kernel.org>
    Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/usb/gadget/udc/net2280.c | 16 ++++++++++++++--
    1 file changed, 14 insertions(+), 2 deletions(-)

    --- a/drivers/usb/gadget/udc/net2280.c
    +++ b/drivers/usb/gadget/udc/net2280.c
    @@ -1549,11 +1549,14 @@ static int net2280_pullup(struct usb_gad
    writel(tmp | BIT(USB_DETECT_ENABLE), &dev->usb->usbctl);
    } else {
    writel(tmp & ~BIT(USB_DETECT_ENABLE), &dev->usb->usbctl);
    - stop_activity(dev, dev->driver);
    + stop_activity(dev, NULL);
    }

    spin_unlock_irqrestore(&dev->lock, flags);

    + if (!is_on && dev->driver)
    + dev->driver->disconnect(&dev->gadget);
    +
    return 0;
    }

    @@ -2470,8 +2473,11 @@ static void stop_activity(struct net2280
    nuke(&dev->ep[i]);

    /* report disconnect; the driver is already quiesced */
    - if (driver)
    + if (driver) {
    + spin_unlock(&dev->lock);
    driver->disconnect(&dev->gadget);
    + spin_lock(&dev->lock);
    + }

    usb_reinit(dev);
    }
    @@ -3345,6 +3351,8 @@ next_endpoints:
    BIT(PCI_RETRY_ABORT_INTERRUPT))

    static void handle_stat1_irqs(struct net2280 *dev, u32 stat)
    +__releases(dev->lock)
    +__acquires(dev->lock)
    {
    struct net2280_ep *ep;
    u32 tmp, num, mask, scratch;
    @@ -3385,12 +3393,14 @@ static void handle_stat1_irqs(struct net
    if (disconnect || reset) {
    stop_activity(dev, dev->driver);
    ep0_start(dev);
    + spin_unlock(&dev->lock);
    if (reset)
    usb_gadget_udc_reset
    (&dev->gadget, dev->driver);
    else
    (dev->driver->disconnect)
    (&dev->gadget);
    + spin_lock(&dev->lock);
    return;
    }
    }
    @@ -3409,6 +3419,7 @@ static void handle_stat1_irqs(struct net
    tmp = BIT(SUSPEND_REQUEST_CHANGE_INTERRUPT);
    if (stat & tmp) {
    writel(tmp, &dev->regs->irqstat1);
    + spin_unlock(&dev->lock);
    if (stat & BIT(SUSPEND_REQUEST_INTERRUPT)) {
    if (dev->driver->suspend)
    dev->driver->suspend(&dev->gadget);
    @@ -3419,6 +3430,7 @@ static void handle_stat1_irqs(struct net
    dev->driver->resume(&dev->gadget);
    /* at high speed, note erratum 0133 */
    }
    + spin_lock(&dev->lock);
    stat &= ~tmp;
    }


    \
     
     \ /
      Last update: 2018-09-24 14:11    [W:4.316 / U:0.768 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site