From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CED14C433F5 for ; Sat, 7 May 2022 17:57:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1390536AbiEGSAt (ORCPT ); Sat, 7 May 2022 14:00:49 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40974 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1386606AbiEGSAp (ORCPT ); Sat, 7 May 2022 14:00:45 -0400 Received: from netrider.rowland.org (netrider.rowland.org [192.131.102.5]) by lindbergh.monkeyblade.net (Postfix) with SMTP id ABD5262C5 for ; Sat, 7 May 2022 10:56:58 -0700 (PDT) Received: (qmail 76931 invoked by uid 1000); 7 May 2022 13:56:57 -0400 Date: Sat, 7 May 2022 13:56:57 -0400 From: Alan Stern To: Schspa Shi Cc: Julia.Lawall@inria.fr, andreyknvl@gmail.com, balbi@kernel.org, gregkh@linuxfoundation.org, jannh@google.com, jj251510319013@gmail.com, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, syzbot+dc7c3ca638e773db07f6@syzkaller.appspotmail.com Subject: Re: [PATCH v2] usb: gadget: fix race when gadget driver register via ioctl Message-ID: References: <20220507160243.35304-1-schspa@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220507160243.35304-1-schspa@gmail.com> Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org On Sun, May 08, 2022 at 12:02:43AM +0800, Schspa Shi wrote: > The usb_gadget_register_driver can be called multi time by to > threads via USB_RAW_IOCTL_RUN ioctl syscall, which will lead > to multiple registrations. > > Call trace: > driver_register+0x220/0x3a0 drivers/base/driver.c:171 > usb_gadget_register_driver_owner+0xfb/0x1e0 > drivers/usb/gadget/udc/core.c:1546 > raw_ioctl_run drivers/usb/gadget/legacy/raw_gadget.c:513 [inline] > raw_ioctl+0x1883/0x2730 drivers/usb/gadget/legacy/raw_gadget.c:1220 > ioctl USB_RAW_IOCTL_RUN > > This routine allows two processes to register the same driver instance > via ioctl syscall. which lead to a race condition. > > We can fix it by adding a new STATE_DEV_REGISTERING device state to > avoid double register. Are you sure that this patch will fix the problem found by syzbot? That is, are you sure that the problem really was caused by two threads registering the same driver concurrently? The fact that the error was "use after free" suggests that the problem might be something else. It looks like one of the threads was trying to access the driver structure after the other thread had done something that caused it to be deallocated, which suggests an error in reference counting. Yes, this could be caused by two threads both registering the same driver. But the evidence doesn't prove that this is what happened, as far as I can see. Alan Stern > Reported-by: syzbot+dc7c3ca638e773db07f6@syzkaller.appspotmail.com > Link: https://lore.kernel.org/all/000000000000e66c2805de55b15a@google.com/ > > Signed-off-by: Schspa Shi > --- > drivers/usb/gadget/legacy/raw_gadget.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c > index b3be8db1ff63..b75f8f7b7b46 100644 > --- a/drivers/usb/gadget/legacy/raw_gadget.c > +++ b/drivers/usb/gadget/legacy/raw_gadget.c > @@ -146,6 +146,7 @@ enum dev_state { > STATE_DEV_OPENED, > STATE_DEV_INITIALIZED, > STATE_DEV_RUNNING, > + STATE_DEV_REGISTERING, > STATE_DEV_CLOSED, > STATE_DEV_FAILED > }; > @@ -508,6 +509,7 @@ static int raw_ioctl_run(struct raw_dev *dev, unsigned long value) > ret = -EINVAL; > goto out_unlock; > } > + dev->state = STATE_DEV_REGISTERING; > spin_unlock_irqrestore(&dev->lock, flags); > > ret = usb_gadget_register_driver(&dev->driver); > -- > 2.24.3 (Apple Git-128) >