From patchwork Wed Aug 8 15:20:39 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Stern X-Patchwork-Id: 10560175 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 005A015A6 for ; Wed, 8 Aug 2018 15:20:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E36AE2B0F4 for ; Wed, 8 Aug 2018 15:20:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D76B62B108; Wed, 8 Aug 2018 15:20:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 48F3C2AF86 for ; Wed, 8 Aug 2018 15:20:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727069AbeHHRkr (ORCPT ); Wed, 8 Aug 2018 13:40:47 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:55198 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1726954AbeHHRkr (ORCPT ); Wed, 8 Aug 2018 13:40:47 -0400 Received: (qmail 5999 invoked by uid 2102); 8 Aug 2018 11:20:39 -0400 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 8 Aug 2018 11:20:39 -0400 Date: Wed, 8 Aug 2018 11:20:39 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Felipe Balbi cc: "D. Ziesche" , USB list Subject: [PATCH] USB: net2280: Fix erroneous synchronization change Message-ID: MIME-Version: 1.0 Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 Fixes: f16443a034c7 ("USB: gadgetfs, dummy-hcd, net2280: fix locking for callbacks") Reported-by: D. Ziesche Tested-by: D. Ziesche CC: --- [as1873] drivers/usb/gadget/udc/net2280.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Index: usb-4.x/drivers/usb/gadget/udc/net2280.c =================================================================== --- usb-4.x.orig/drivers/usb/gadget/udc/net2280.c +++ usb-4.x/drivers/usb/gadget/udc/net2280.c @@ -1545,11 +1545,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; } @@ -2466,8 +2469,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); } @@ -3341,6 +3347,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; @@ -3381,12 +3389,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; } } @@ -3405,6 +3415,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); @@ -3415,6 +3426,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; }