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 B9F13C001B2 for ; Thu, 8 Dec 2022 17:40:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229637AbiLHRkU (ORCPT ); Thu, 8 Dec 2022 12:40:20 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49620 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229463AbiLHRkT (ORCPT ); Thu, 8 Dec 2022 12:40:19 -0500 Received: from netrider.rowland.org (netrider.rowland.org [192.131.102.5]) by lindbergh.monkeyblade.net (Postfix) with SMTP id DC80D7DA75 for ; Thu, 8 Dec 2022 09:40:17 -0800 (PST) Received: (qmail 734591 invoked by uid 1000); 8 Dec 2022 12:40:17 -0500 Date: Thu, 8 Dec 2022 12:40:17 -0500 From: Alan Stern To: Oliver Neukum Cc: syzbot , WeitaoWang-oc@zhaoxin.com, arnd@arndb.de, gregkh@linuxfoundation.org, khalid.masum.92@gmail.com, kishon@ti.com, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, syzkaller-bugs@googlegroups.com Subject: Re: [syzbot] KASAN: use-after-free Read in __usb_hcd_giveback_urb (2) Message-ID: References: <0000000000002fc8dc05ef267a9f@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org On Thu, Dec 08, 2022 at 03:36:45PM +0100, Oliver Neukum wrote: > On 06.12.22 16:38, Alan Stern wrote: > > Hi, > > > Oliver: > > > > This looks like a bug in the anchor API. > > Yes, it does. > > On Tue, Dec 06, 2022 at 02:43:41AM -0800, syzbot wrote: > > > Hello, > > > > > > syzbot found the following issue on: > > > > > > HEAD commit: ef4d3ea40565 afs: Fix server->active leak in afs_put_server > > > git tree: upstream > > > console output: https://syzkaller.appspot.com/x/log.txt?x=100b244d880000 > > > kernel config: https://syzkaller.appspot.com/x/.config?x=8e7e79f8a1e34200 > > > dashboard link: https://syzkaller.appspot.com/bug?extid=712fd0e60dda3ba34642 > > > compiler: Debian clang version 13.0.1-++20220126092033+75e33f71c2da-1~exp1~20220126212112.63, GNU ld (GNU Binutils for Debian) 2.35.2 > > > > > > Unfortunately, I don't have any reproducer for this issue yet. > > > > > > Downloadable assets: > > > disk image: https://storage.googleapis.com/syzbot-assets/ef790e7777cd/disk-ef4d3ea4.raw.xz > > > vmlinux: https://storage.googleapis.com/syzbot-assets/2ed3c6bc9230/vmlinux-ef4d3ea4.xz > > > kernel image: https://storage.googleapis.com/syzbot-assets/f1dbd004fa88/bzImage-ef4d3ea4.xz > > > > > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > > > Reported-by: syzbot+712fd0e60dda3ba34642@syzkaller.appspotmail.com > > > > > > xpad 3-1:179.65: xpad_irq_in - usb_submit_urb failed with result -19 > > > xpad 3-1:179.65: xpad_irq_out - usb_submit_urb failed with result -19 > > > ================================================================== > > > BUG: KASAN: use-after-free in register_lock_class+0x8d2/0x9b0 kernel/locking/lockdep.c:1338 > > > __wake_up+0xf8/0x1c0 kernel/sched/wait.c:156 > > > __usb_hcd_giveback_urb+0x3a0/0x530 drivers/usb/core/hcd.c:1674 > > > > This is the call to usb_anchor_resume_wakeups(). The call is made after > > the completion handler callback. Evidently the xpad driver deallocated > > the anchor during that time window. This can happen if the driver is > > just waiting for its last URB to complete before freeing all its memory. > > Yes, complete() had run. > > > > I don't know what the best solution is. It may be necessary to refcount > > anchors somehow. > > Then we cannot embed them anymore. Many drivers would need a lot of changes. > xpad included. It's hard to tell what's really going on. Looking at xpad_stop_output(), you see that it doesn't do anything if xpad->type is XTYPE_UNKNOWN. Is that what happened here? I can't figure out where the underlying race is. Maybe it's not directly connected with anchors after all. > As far as I can tell the order we decrease use_count is correct. But: > > 6ec4147e7bdbd (Hans de Goede 2013-10-09 17:01:41 +0200 1674) usb_anchor_resume_wakeups(anchor); > 94dfd7edfd5c9 (Ming Lei 2013-07-03 22:53:07 +0800 1675) atomic_dec(&urb->use_count); > > Do we need to guarantee memory ordering here? I don't think we need to do anything more. usb_kill_urb() is careful to wait for completion handlers to finish, and we already have smp_mb__after_atomic() barriers in the appropriate places to ensure proper memory ordering. Alan Stern