Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752052AbdIMO7U (ORCPT ); Wed, 13 Sep 2017 10:59:20 -0400 Received: from netrider.rowland.org ([192.131.102.5]:47895 "HELO netrider.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1750993AbdIMO7R (ORCPT ); Wed, 13 Sep 2017 10:59:17 -0400 Date: Wed, 13 Sep 2017 10:59:15 -0400 (EDT) From: Alan Stern X-X-Sender: stern@netrider.rowland.org To: Dmitry Vyukov cc: Greg Kroah-Hartman , Andrey Konovalov , Johan Hovold , USB list , LKML , Kostya Serebryany , syzkaller Subject: Re: usb/uwb: WARNING in hwarc_neep_init/usb_submit_urb 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: 5586 Lines: 114 On Wed, 13 Sep 2017, Dmitry Vyukov wrote: > On Tue, Sep 12, 2017 at 9:57 PM, Greg Kroah-Hartman > wrote: > > On Tue, Sep 12, 2017 at 08:53:11PM +0200, Andrey Konovalov wrote: > >> Hi! > >> > >> I've got the following crash while fuzzing the kernel with syzkaller. > >> > >> On commit 81a84ad3cb5711cec79f4dd53a4ce026b092c432 (Sep 3). > >> > >> gadgetfs: bound to dummy_udc driver > >> usb 1-1: new full-speed USB device number 2 using dummy_hcd > >> gadgetfs: connected > >> gadgetfs: disconnected > >> gadgetfs: connected > >> usb 1-1: New USB device found, idVendor=0000, idProduct=0000 > >> usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=203 > >> usb 1-1: SerialNumber: a > >> gadgetfs: configuration #7 > >> usb 1-1: BOGUS urb xfer, pipe 1 != type 3 > >> ------------[ cut here ]------------ > >> WARNING: CPU: 0 PID: 3 at drivers/usb/core/urb.c:449 usb_submit_urb+0xf8a/0x11d0 > >> Modules linked in: > >> CPU: 0 PID: 3 Comm: kworker/0:0 Not tainted 4.13.0+ #111 > >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > >> Workqueue: usb_hub_wq hub_event > >> task: ffff88006bdc1a00 task.stack: ffff88006bde8000 > >> RIP: 0010:usb_submit_urb+0xf8a/0x11d0 drivers/usb/core/urb.c:448 > >> RSP: 0018:ffff88006bdee3c0 EFLAGS: 00010282 > >> RAX: 0000000000000029 RBX: ffff8800672a7200 RCX: 0000000000000000 > >> RDX: 0000000000000029 RSI: ffff88006c815c78 RDI: ffffed000d7bdc6a > >> RBP: ffff88006bdee4c0 R08: fffffbfff0fe00ff R09: fffffbfff0fe00ff > >> R10: 0000000000000018 R11: fffffbfff0fe00fe R12: 1ffff1000d7bdc7f > >> R13: 0000000000000003 R14: 0000000000000001 R15: ffff88006b02cc90 > >> FS: 0000000000000000(0000) GS:ffff88006c800000(0000) knlGS:0000000000000000 > >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > >> CR2: 00007fe4daddf000 CR3: 000000006add6000 CR4: 00000000000006f0 > >> Call Trace: > >> hwarc_neep_init+0x4ce/0x9c0 drivers/uwb/hwa-rc.c:710 > >> uwb_rc_add+0x2fb/0x730 drivers/uwb/lc-rc.c:361 > >> hwarc_probe+0x34e/0x9b0 drivers/uwb/hwa-rc.c:858 > >> usb_probe_interface+0x351/0x8d0 drivers/usb/core/driver.c:361 > >> really_probe drivers/base/dd.c:385 > >> driver_probe_device+0x610/0xa00 drivers/base/dd.c:529 > >> __device_attach_driver+0x230/0x290 drivers/base/dd.c:625 > >> bus_for_each_drv+0x15e/0x210 drivers/base/bus.c:463 > >> __device_attach+0x269/0x3c0 drivers/base/dd.c:682 > >> device_initial_probe+0x1f/0x30 drivers/base/dd.c:729 > >> bus_probe_device+0x1da/0x280 drivers/base/bus.c:523 > >> device_add+0xcf9/0x1640 drivers/base/core.c:1703 > >> usb_set_configuration+0x1064/0x1890 drivers/usb/core/message.c:1932 > >> generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174 > >> usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266 > >> really_probe drivers/base/dd.c:385 > >> driver_probe_device+0x610/0xa00 drivers/base/dd.c:529 > >> __device_attach_driver+0x230/0x290 drivers/base/dd.c:625 > >> bus_for_each_drv+0x15e/0x210 drivers/base/bus.c:463 > >> __device_attach+0x269/0x3c0 drivers/base/dd.c:682 > >> device_initial_probe+0x1f/0x30 drivers/base/dd.c:729 > >> bus_probe_device+0x1da/0x280 drivers/base/bus.c:523 > >> device_add+0xcf9/0x1640 drivers/base/core.c:1703 > >> usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457 > >> hub_port_connect drivers/usb/core/hub.c:4890 > >> hub_port_connect_change drivers/usb/core/hub.c:4996 > >> port_event drivers/usb/core/hub.c:5102 > >> hub_event+0x23c8/0x37c0 drivers/usb/core/hub.c:5182 > >> process_one_work+0x9fb/0x1570 kernel/workqueue.c:2097 > >> worker_thread+0x1e4/0x1350 kernel/workqueue.c:2231 > >> kthread+0x324/0x3f0 kernel/kthread.c:231 > >> ret_from_fork+0x25/0x30 arch/x86/entry/entry_64.S:425 > >> Code: 48 8b 85 30 ff ff ff 48 8d b8 98 00 00 00 e8 8e 93 07 ff 45 89 > >> e8 44 89 f1 4c 89 fa 48 89 c6 48 c7 c7 a0 e5 55 86 e8 20 08 8f fd <0f> > >> ff e9 9b f7 ff ff e8 4a 04 d6 fd e9 80 f7 ff ff e8 60 11 a6 > >> ---[ end trace 55d741234124cfc3 ]--- > > > > It's a WARN_ON(), here, not really a "problem", right? You are trying > > to fuzz the drivers by giving it crappy descriptors, and you triggered a > > valid warning from the kernel notifying you that your "hardware" is > > really an invalid USB device :) > > > > So nothing to really "fix" here, this is "working as expected", right? > > > WARNING means bug in kernel source code that kernel can tolerate (as > opposed to BUG). > Invalid inputs to kernel should not trigger WARNINGs nor BUGs. The > stack is pointless here, the registers are pointless, what's relevant > here is: > > usb 1-1: BOGUS urb xfer, pipe 1 != type 3 > > And this looks like enough information (can be extended if there are > some other relevant values). > WARNINGs on invalid inputs cause local DoS, does not allow any testing > automation and cause spam for kernel developers (what do you do when > you see WARNING/BUG on console in a subsystem that you are not aware > of? right, you notify maintainers). > > So, if it's just an invalid input to kernel, the action point here is > to change it to pr_err/pr_warn/dev_printk. The message indicates that the driver has called usb_submit_urb() after setting up the URB with usb_fill_int_urb(), but the endpoint descriptor says that the endpoint is bulk, not interrupt. Either the driver should detect that the endpoint has the wrong type and refuse to use it, or else it should allow for the possibility by calling usb_fill_bulk_urb() when necessary. Without knowing the specs for the device in question, I can't say which alternative would be better. Alan Stern