This message generated a parse failure. Raw output follows here. Please use 'back' to navigate. From devnull@lkml.org Fri Apr 26 15:43:52 2024 >From mailfetcher Fri Apr 16 07:42:20 2021 Envelope-to: lkml@grols.ch Delivery-date: Fri, 16 Apr 2021 07:41:33 +0200 Received: from stout.grols.ch [195.201.141.146] by 72459556e3a9 with IMAP (fetchmail-6.3.26) for (single-drop); Fri, 16 Apr 2021 07:42:20 +0200 (CEST) Received: from vger.kernel.org ([23.128.96.18]) by stout.grols.ch with esmtp (Exim 4.89) (envelope-from ) id 1lXHES-0002eE-Ft for lkml@grols.ch; Fri, 16 Apr 2021 07:41:33 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231666AbhDPFlW (ORCPT ); Fri, 16 Apr 2021 01:41:22 -0400 Received: from sender4-of-o53.zoho.com ([136.143.188.53]:21345 "EHLO sender4-of-o53.zoho.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231598AbhDPFlV (ORCPT ); Fri, 16 Apr 2021 01:4 ARC-Seal: i=1; a=rsa-sha256; t=1618551645; cv=none; d=zohomail.com; s=zohoarc; b=ZeoBQETUSOqu2U6vnlj74+undnITnJ3L2EEsmkTHvybJBvBOYDv9tl2OjjC3CUMTR5R3VAANBCj5uPK6wvG2a1TnGwbiLBNg5k0QZroG2bSWDgNnS8m2am2LOG5DtC/ITcvSXfZqr9q+8xW/VP2Xi/D9q6PFWMgYqDbrdew ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1618551645; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To; bh=fvDfttZM4KjoqywY1vxhQFEBQkz4bYXvnPMQ5kjViCQ ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=anirudhrb.com; spf=pass smtp.mailfrom=mail@anirudhrb.com; dmarc=pass header.from= header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1618551645; s=zoho; d=anirudhrb.com; i=mail@anirudhrb.com; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:Content-Type:Content-Transfer-Encoding:In-Reply-To; bh=fvDfttZ Received: from anirudhrb.com (49.207.216.151 [49.207.216.151]) by mx.zohomail.com with SMTPS id 1618551642390440.0500003984672; Thu, 15 Apr 2021 22:40:42 -0700 (PDT) Date: Fri, 16 Apr 2021 11:10:35 +0530 From: Anirudh Rayabharam To: Alan Stern Cc: Dmitry Vyukov , syzbot , Andrey Konovalov , Felipe Balbi , Dan Carpenter , Subject: Re: [syzbot] general protection fault in gadget_setup Message-Id: References: <00000000000075c58405bfd6228c@google.com> <20210413161311.GC1454681@rowland.harvard.edu> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <20210413161311.GC1454681@rowland.harvard.edu> X-ZohoMailClient: External Precedence: bulk List-Id: X-Mailing-List: linux-kernel@vger.kernel.org Received-SPF: pass client-ip=23.128.96.18; envelope-from=linux-kernel-owner@vger.kernel.org; helo=vger.kernel.org X-Spam-Score: -2.0 X-Spam-Score-Bar: - X-Spam-Action: no action X-Spam-Report: Action: no action Symbol: R_SPF_ALLOW(-0.20) Symbol: RCVD_COUNT_THREE(0.00) Symbol: TO_DN_ALL(0.00) Symbol: DKIM_TRACE(0.00) Symbol: RCPT_COUNT_SEVEN(0.00) Symbol: MAILLIST(-0.10) Symbol: RCVD_NO_TLS_LAST(0.10) Symbol: FORGED_RECIPIENTS_MAILLIST(0.00) Sy On Tue, Apr 13, 2021 at 12:13:11PM -0400, Alan Stern wrote: > On Tue, Apr 13, 2021 at 10:12:05AM +0200, Dmitry Vyukov wrote: > > On Tue, Apr 13, 2021 at 10:08 AM syzbot > > wrote: > > > > > > Hello, > > > > > > syzbot found the following issue on: > > > > > > HEAD commit: 0f4498ce Merge tag 'for-5.12/dm-fixes-2' of git://git= =2Ekern.. > > > git tree: upstream > > > console output: https://syzkaller.appspot.com/x/log.txt?x=3D124adbf6d= 00000 > > > kernel config: https://syzkaller.appspot.com/x/.config?x=3Ddaeff30c2= 474a60f > > > dashboard link: https://syzkaller.appspot.com/bug?extid=3Deb4674092e6= cc8d9e0bd > > > userspace arch: i386 > > > > > > Unfortunately, I don't have any reproducer for this issue yet. > > > > > > IMPORTANT: if you fix the issue, please add the following tag to the = commit: > > > Reported-by: syzbot+eb4674092e6cc8d9e0bd@syzkaller.appspotmail.com > >=20 > > I suspect that the raw gadget_unbind() can be called while the timer > > is still active. gadget_unbind() sets gadget data to NULL. > > But I am not sure which unbind call this is: > > usb_gadget_remove_driver() or right in udc_bind_to_driver() due to a > > start error. >=20 > This certainly looks like a race between gadget_unbind and gadget_setup= =20 > in raw_gadget. >=20 > In theory, this race shouldn't matter. The gadget core is supposed to=20 > guarantee that there won't be any more callbacks to the gadget driver=20 > once the driver's unbind routine is called. That guarantee is enforced= =20 > in usb_gadget_remove_driver as follows: >=20 > usb_gadget_disconnect(udc->gadget); > if (udc->gadget->irq) > synchronize_irq(udc->gadget->irq); > udc->driver->unbind(udc->gadget); > usb_gadget_udc_stop(udc); >=20 > usb_gadget_disconnect turns off the pullup resistor, telling the host=20 > that the gadget is no longer connected and preventing the transmission=20 > of any more USB packets. Any packets that have already been received=20 > are sure to processed by the UDC driver's interrupt handler by the time= =20 > synchronize_irq returns. >=20 > But this doesn't work with dummy_hcd, because dummy_hcd doesn't use=20 > interrupts; it uses a timer instead. It does have code to emulate the=20 > effect of synchronize_irq, but that code doesn't get invoked at the=20 > right time -- it currently runs in usb_gadget_udc_stop, after the unbind= =20 > callback instead of before. Indeed, there's no way for=20 > usb_gadget_remove_driver to invoke this code before the unbind=20 > callback,. >=20 > I thought the synchronize_irq emulation problem had been completely=20 > solved, but evidently it hasn't. It looks like the best solution is to= =20 > add a call of the synchronize_irq emulation code in dummy_pullup. >=20 > Maybe we can test this reasoning by putting a delay just before the call= =20 > to dum->driver->setup. That runs in the timer handler, so it's not a=20 > good place to delay, but it may be okay just for testing purposes. >=20 > Hopefully this patch will make the race a lot more likely to occur. Is= =20 Hi Alan,=20 Indeed, I was able to reproduce this bug easily on my machine with your delay patch applied and using this syzkaller program: syz_usb_connect$cdc_ncm(0x1, 0x6e, &(0x7f0000000040)=3D{{0x12, 0x1, 0x0, 0x= 2, 0x0, 0x0, 0x8, 0x525, 0xa4a1, 0x40, 0x1, 0x2, 0x3, 0x1, [{{0x9, 0x2, 0x5= c, 0x2, 0x1, 0x0, 0x0, 0x0, {{0x9, 0x4, 0x0, 0x0, 0x1, 0x2, 0xd, 0x0, 0x0, = {{0x5}, {0x5}, {0xd}, {0x6}}, {{0x9, 0x5, 0x81, 0x3, 0x200}}}}}}]}}, &(0x7f= 0000000480)=3D{0x0, 0x0, 0x0, 0x0, 0x3, [{0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}= ]}) I also tested doing the synchronize_irq emulation in dummy_pullup and it fixed the issue. The patch is below. Thanks! - Anirudh. diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/du= mmy_hcd.c index ce24d4f28f2a..931d4612d859 100644 --- a/drivers/usb/gadget/udc/dummy_hcd.c +++ b/drivers/usb/gadget/udc/dummy_hcd.c @@ -903,6 +903,12 @@ static int dummy_pullup(struct usb_gadget *_gadget, in= t value) spin_lock_irqsave(&dum->lock, flags); dum->pullup =3D (value !=3D 0); set_link_state(dum_hcd); + /* emulate synchronize_irq(): wait for callbacks to finish */=1C + while (dum->callback_usage > 0) { + spin_unlock_irqrestore(&dum->lock, flags); + usleep_range(1000, 2000); + spin_lock_irqsave(&dum->lock, flags); + } spin_unlock_irqrestore(&dum->lock, flags); =20 usb_hcd_poll_rh_status(dummy_hcd_to_hcd(dum_hcd)); @@ -1005,13 +1011,6 @@ static int dummy_udc_stop(struct usb_gadget *g) dum->ints_enabled =3D 0; stop_activity(dum); =20 - /* emulate synchronize_irq(): wait for callbacks to finish */ - while (dum->callback_usage > 0) { - spin_unlock_irq(&dum->lock); - usleep_range(1000, 2000); - spin_lock_irq(&dum->lock); - } - dum->driver =3D NULL; spin_unlock_irq(&dum->lock); =20 @@ -1900,6 +1899,7 @@ static void dummy_timer(struct timer_list *t) if (value > 0) { ++dum->callback_usage; spin_unlock(&dum->lock); + mdelay(5); value =3D dum->driver->setup(&dum->gadget, &setup); spin_lock(&dum->lock);