Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752425AbdIURf5 (ORCPT ); Thu, 21 Sep 2017 13:35:57 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:40692 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751598AbdIURfy (ORCPT ); Thu, 21 Sep 2017 13:35:54 -0400 Date: Thu, 21 Sep 2017 13:35:53 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Andrey Konovalov cc: Felipe Balbi , Greg Kroah-Hartman , USB list , LKML , Dmitry Vyukov , Kostya Serebryany , syzkaller Subject: Re: usb/gadget: copy_to_user called with spinlock held In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1345 Lines: 42 On Thu, 21 Sep 2017, Andrey Konovalov wrote: > Hi! > > I've got the following report while fuzzing the kernel with syzkaller. > > On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18). > > Line numbers might be a little off, due to some local changes to > gadgetfs code but the issue is AFAIU with calling copy_to_user() with > spinlock held in ep0_read(). I see that there's a FIXME exactly about > that and I was wondering if there's any chance of getting this fixed? > > Thanks! The patch below, which goes on top of the gadgetfs patch from a few days ago, should eliminate this problem. Alan Stern Index: usb-4.x/drivers/usb/gadget/legacy/inode.c =================================================================== --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c +++ usb-4.x/drivers/usb/gadget/legacy/inode.c @@ -986,11 +986,14 @@ ep0_read (struct file *fd, char __user * retval = -EIO; else { len = min (len, (size_t)dev->req->actual); -// FIXME don't call this with the spinlock held ... + ++dev->udc_usage; + spin_unlock_irq(&dev->lock); if (copy_to_user (buf, dev->req->buf, len)) retval = -EFAULT; else retval = len; + spin_lock_irq(&dev->lock); + --dev->udc_usage; clean_req (dev->gadget->ep0, dev->req); /* NOTE userspace can't yet choose to stall */ }