* [PATCH] usb: host: xhci-plat: fix support for XHCI_SKIP_PHY_INIT quirk
From: Pali Rohár @ 2020-12-21 15:09 UTC (permalink / raw)
To: Mathias Nyman, Greg Kroah-Hartman, Peter Chen
Cc: Jun Li, linux-usb, linux-kernel
Currently init_quirk callbacks for xhci platform drivers are called
xhci_plat_setup() function which is called after chip reset completes.
It happens in the middle of the usb_add_hcd() function.
But XHCI_SKIP_PHY_INIT quirk is checked in the xhci_plat_probe() function
prior calling usb_add_hcd() function. Therefore this XHCI_SKIP_PHY_INIT
currently does nothing as prior xhci_plat_setup() it is not set.
Quirk XHCI_SKIP_PHY_INIT is only setting hcd->skip_phy_initialization value
which really needs to be set prior calling usb_add_hcd() as this function
at its beginning skips PHY init if this member is set.
This patch fixes implementation of the XHCI_SKIP_PHY_INIT quirk by calling
init_quirk callbacks (via xhci_priv_init_quirk()) prior checking if
XHCI_SKIP_PHY_INIT is set.
Fixes: f768e718911e0 ("usb: host: xhci-plat: add priv quirk for skip PHY initialization")
Signed-off-by: Pali Rohár <pali@kernel.org>
---
drivers/usb/host/xhci-plat.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 4d34f6005381..58704c5b002b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -89,13 +89,6 @@ static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
/* called during probe() after chip reset completes */
static int xhci_plat_setup(struct usb_hcd *hcd)
{
- int ret;
-
-
- ret = xhci_priv_init_quirk(hcd);
- if (ret)
- return ret;
-
return xhci_gen_setup(hcd, xhci_plat_quirks);
}
@@ -330,6 +323,13 @@ static int xhci_plat_probe(struct platform_device *pdev)
hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node);
xhci->shared_hcd->tpl_support = hcd->tpl_support;
+
+ if (priv) {
+ ret = xhci_priv_init_quirk(hcd);
+ if (ret)
+ goto disable_usb_phy;
+ }
+
if (priv && (priv->quirks & XHCI_SKIP_PHY_INIT))
hcd->skip_phy_initialization = 1;
--
2.20.1
^ permalink raw reply related
* memory leak in mcba_usb_probe
From: syzbot @ 2020-12-21 8:54 UTC (permalink / raw)
To: a.darwish, bigeasy, gregkh, linux-kernel, linux-usb, stern,
syzkaller-bugs, tglx
Hello,
syzbot found the following issue on:
HEAD commit: 467f8165 Merge tag 'close-range-cloexec-unshare-v5.11' of ..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15933e0f500000
kernel config: https://syzkaller.appspot.com/x/.config?x=37c889fb8b2761af
dashboard link: https://syzkaller.appspot.com/bug?extid=57281c762a3922e14dfe
compiler: gcc (GCC) 10.1.0-syz 20200507
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14fe2b9b500000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13bd2287500000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+57281c762a3922e14dfe@syzkaller.appspotmail.com
BUG: memory leak
unreferenced object 0xffff88811101c080 (size 64):
comm "kworker/0:2", pid 56, jiffies 4294942309 (age 8.240s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000e42b7c1c>] kmalloc include/linux/slab.h:557 [inline]
[<00000000e42b7c1c>] hcd_buffer_alloc+0x149/0x190 drivers/usb/core/buffer.c:134
[<0000000042899d1a>] usb_alloc_coherent+0x42/0x60 drivers/usb/core/usb.c:897
[<00000000abbb04b6>] mcba_usb_start drivers/net/can/usb/mcba_usb.c:644 [inline]
[<00000000abbb04b6>] mcba_usb_probe+0x27b/0x430 drivers/net/can/usb/mcba_usb.c:846
[<00000000b5e48b67>] usb_probe_interface+0x177/0x370 drivers/usb/core/driver.c:396
[<00000000a0416f71>] really_probe+0x159/0x480 drivers/base/dd.c:561
[<00000000cce96f4d>] driver_probe_device+0x84/0x100 drivers/base/dd.c:745
[<00000000b0116c0e>] __device_attach_driver+0xee/0x110 drivers/base/dd.c:851
[<00000000da4bf16e>] bus_for_each_drv+0xb7/0x100 drivers/base/bus.c:431
[<00000000e826f962>] __device_attach+0x122/0x250 drivers/base/dd.c:919
[<00000000fb35d32b>] bus_probe_device+0xc6/0xe0 drivers/base/bus.c:491
[<0000000083f168a1>] device_add+0x5be/0xc30 drivers/base/core.c:3091
[<000000002b4245b1>] usb_set_configuration+0x9d9/0xb90 drivers/usb/core/message.c:2164
[<00000000fa502fa0>] usb_generic_driver_probe+0x8c/0xc0 drivers/usb/core/generic.c:238
[<000000004e1ab3a9>] usb_probe_device+0x5c/0x140 drivers/usb/core/driver.c:293
[<00000000a0416f71>] really_probe+0x159/0x480 drivers/base/dd.c:561
[<00000000cce96f4d>] driver_probe_device+0x84/0x100 drivers/base/dd.c:745
BUG: memory leak
unreferenced object 0xffff88811101c0c0 (size 64):
comm "kworker/0:2", pid 56, jiffies 4294942309 (age 8.240s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000e42b7c1c>] kmalloc include/linux/slab.h:557 [inline]
[<00000000e42b7c1c>] hcd_buffer_alloc+0x149/0x190 drivers/usb/core/buffer.c:134
[<0000000042899d1a>] usb_alloc_coherent+0x42/0x60 drivers/usb/core/usb.c:897
[<00000000abbb04b6>] mcba_usb_start drivers/net/can/usb/mcba_usb.c:644 [inline]
[<00000000abbb04b6>] mcba_usb_probe+0x27b/0x430 drivers/net/can/usb/mcba_usb.c:846
[<00000000b5e48b67>] usb_probe_interface+0x177/0x370 drivers/usb/core/driver.c:396
[<00000000a0416f71>] really_probe+0x159/0x480 drivers/base/dd.c:561
[<00000000cce96f4d>] driver_probe_device+0x84/0x100 drivers/base/dd.c:745
[<00000000b0116c0e>] __device_attach_driver+0xee/0x110 drivers/base/dd.c:851
[<00000000da4bf16e>] bus_for_each_drv+0xb7/0x100 drivers/base/bus.c:431
[<00000000e826f962>] __device_attach+0x122/0x250 drivers/base/dd.c:919
[<00000000fb35d32b>] bus_probe_device+0xc6/0xe0 drivers/base/bus.c:491
[<0000000083f168a1>] device_add+0x5be/0xc30 drivers/base/core.c:3091
[<000000002b4245b1>] usb_set_configuration+0x9d9/0xb90 drivers/usb/core/message.c:2164
[<00000000fa502fa0>] usb_generic_driver_probe+0x8c/0xc0 drivers/usb/core/generic.c:238
[<000000004e1ab3a9>] usb_probe_device+0x5c/0x140 drivers/usb/core/driver.c:293
[<00000000a0416f71>] really_probe+0x159/0x480 drivers/base/dd.c:561
[<00000000cce96f4d>] driver_probe_device+0x84/0x100 drivers/base/dd.c:745
BUG: memory leak
unreferenced object 0xffff88811101c100 (size 64):
comm "kworker/0:2", pid 56, jiffies 4294942309 (age 8.240s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000e42b7c1c>] kmalloc include/linux/slab.h:557 [inline]
[<00000000e42b7c1c>] hcd_buffer_alloc+0x149/0x190 drivers/usb/core/buffer.c:134
[<0000000042899d1a>] usb_alloc_coherent+0x42/0x60 drivers/usb/core/usb.c:897
[<00000000abbb04b6>] mcba_usb_start drivers/net/can/usb/mcba_usb.c:644 [inline]
[<00000000abbb04b6>] mcba_usb_probe+0x27b/0x430 drivers/net/can/usb/mcba_usb.c:846
[<00000000b5e48b67>] usb_probe_interface+0x177/0x370 drivers/usb/core/driver.c:396
[<00000000a0416f71>] really_probe+0x159/0x480 drivers/base/dd.c:561
[<00000000cce96f4d>] driver_probe_device+0x84/0x100 drivers/base/dd.c:745
[<00000000b0116c0e>] __device_attach_driver+0xee/0x110 drivers/base/dd.c:851
[<00000000da4bf16e>] bus_for_each_drv+0xb7/0x100 drivers/base/bus.c:431
[<00000000e826f962>] __device_attach+0x122/0x250 drivers/base/dd.c:919
[<00000000fb35d32b>] bus_probe_device+0xc6/0xe0 drivers/base/bus.c:491
[<0000000083f168a1>] device_add+0x5be/0xc30 drivers/base/core.c:3091
[<000000002b4245b1>] usb_set_configuration+0x9d9/0xb90 drivers/usb/core/message.c:2164
[<00000000fa502fa0>] usb_generic_driver_probe+0x8c/0xc0 drivers/usb/core/generic.c:238
[<000000004e1ab3a9>] usb_probe_device+0x5c/0x140 drivers/usb/core/driver.c:293
[<00000000a0416f71>] really_probe+0x159/0x480 drivers/base/dd.c:561
[<00000000cce96f4d>] driver_probe_device+0x84/0x100 drivers/base/dd.c:745
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* [Bug 210801] TopSeed Technology Corp. eHome Infrared Transceiver spams log with Error: urb status = -71
From: bugzilla-daemon @ 2020-12-21 6:46 UTC (permalink / raw)
To: linux-usb
In-Reply-To: <bug-210801-208809@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210801
Jiri Slaby (jirislaby@gmail.com) changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jirislaby@gmail.com,
| |oliver@neukum.org
--- Comment #1 from Jiri Slaby (jirislaby@gmail.com) ---
(In reply to Hans-Peter Jansen from comment #0)
> 5.10.1-7-preempt #1 SMP PREEMPT Tue Dec 15 08:33:02 UTC 2020 (c8c5afb)
> x86_64 x86_64 x86_64 GNU/Linux
I suppose this is a Kernel:stable kernel?
> Dec 20 17:03:46 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
>
> with the rate of > 40/second.
Maybe Oliver has an idea?
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: port power is on again after turning off by user space
From: Peter Chen @ 2020-12-21 5:37 UTC (permalink / raw)
To: Alan Stern; +Cc: Jun Li, linux-usb@vger.kernel.org
In-Reply-To: <20201216155144.GA238371@rowland.harvard.edu>
On 20-12-16 10:51:44, Alan Stern wrote:
> On Wed, Dec 16, 2020 at 02:56:20AM +0000, Peter Chen wrote:
> > On 20-12-15 10:55:41, Alan Stern wrote:
> > > You've got the general idea.
> > >
> > > Normally ports are owned by the hub driver. If one of them loses power
> > > for some reason (for example, the user turns it off), the hub driver
> > > will turn the power back on. This is because the hub driver wants
> > > ports to be powered at all times unless they are in runtime suspend.
> > >
> > > The way to prevent the hub driver from managing the port power is to
> > > claim the port for the user, by issuing the USBDEVFS_CLAIM_PORT ioctl.
> > > Also, when that is done, the kernel wno't try to manage a device
> > > attached to the port -- that is, the kernel won't automatically install
> > > a configuration for a new device and it won't try to probe drivers for
> > > the device's interfaces if the user installs a config.
> > >
> >
> > Alan, we have several use cases for power switchable HUB, one of the use
> > cases is USB port is managed by kernel (eg, needs mass storage
> > class), but needs to toggle port power, is it reasonable we add a sysfs
> > entry to support it?
>
> Can you give more information about your use cases? After all, if the
> port power is turned off then the port can't possibly handle
> mass-storage devices -- or anything else.
Sorry, Alan. Due to holiday season, the U.S team doesn't reply me the
use case. I think the basic use cases are emulate the hot-plug test for
USB devices, the USB devices could be Flash Drive on market or DUT (Device
Under test) for the same controller works at device mode. Assume one
typical test case:
Plug in Flash Drive at port 1-1.1 during the boots up:
while (1) {
- Check Flash Drive is there (eg, cat /proc/partitions)
- Turn port 1 at 1-1 off
- Check Flash Drive is gone
- Turn port 1 at 1-1 on
}
>
> On the other hand, if the port is managed by the kernel then the kernel
> (not the user) should be responsible for deciding whether or not to
> turn off the port's power.
>
> If there's some real reason for turning the port power off for an
> extended period of time, the user can claim the port and turn off the
> power. Then later on, the user can release the port and turn the power
> back on.
>
Yes, I think this is one of the use cases. We want power power control
at one application (A), but different with our test application(B), it means
if the user claims the port, and power off using A, then the A will end.
After the B finished running, A runs again for power on, but at this time,
the port owner has changed.
--
Thanks,
Peter Chen
^ permalink raw reply
* Re: Correct ordering of phy_init and phy_power_on
From: Kishon Vijay Abraham I @ 2020-12-21 3:15 UTC (permalink / raw)
To: Ahmad Fatoum, Vinod Koul, Minas Harutyunyan, linux-usb
Cc: linux-kernel@vger.kernel.org, Pengutronix Kernel Team,
Jules Maselbas
In-Reply-To: <6cd01e79-fdc0-3bd4-32b5-a85142533f8a@pengutronix.de>
Hi,
On 21/12/20 4:36 am, Ahmad Fatoum wrote:
> Hello,
>
> I just noticed that USB controller drivers differ in the order in which they
> do phy_init and phy_power_on. For example:
>
> __dwc2_lowlevel_hw_enable():
>
> ret = phy_power_on(hsotg->phy);
> if (ret == 0)
> ret = phy_init(hsotg->phy);
>
> dwc3_core_init():
>
> ret = dwc3_core_soft_reset(dwc); // internally does phy_init(dwc->usb2_generic_phy);
> /* [snip] */
> ret = phy_power_on(dwc->usb2_generic_phy);
>
>
> My initial assumption has been init -> power_on, but at least the phy-stm32-usbphyc
> (used with dwc2) is written with the assumption that exit -> power_off (and therefore
> power_on -> init). If they are swapped, disabling fails.
>
> So how was it meant to be?
It is intended to be ->init() and then ->power_on(). So ideally it
should be the way dwc3 is.
Thanks,
Kishon
^ permalink raw reply
* Correct ordering of phy_init and phy_power_on
From: Ahmad Fatoum @ 2020-12-20 23:06 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Vinod Koul, Minas Harutyunyan, linux-usb
Cc: linux-kernel@vger.kernel.org, Pengutronix Kernel Team, linux-usb,
Jules Maselbas
Hello,
I just noticed that USB controller drivers differ in the order in which they
do phy_init and phy_power_on. For example:
__dwc2_lowlevel_hw_enable():
ret = phy_power_on(hsotg->phy);
if (ret == 0)
ret = phy_init(hsotg->phy);
dwc3_core_init():
ret = dwc3_core_soft_reset(dwc); // internally does phy_init(dwc->usb2_generic_phy);
/* [snip] */
ret = phy_power_on(dwc->usb2_generic_phy);
My initial assumption has been init -> power_on, but at least the phy-stm32-usbphyc
(used with dwc2) is written with the assumption that exit -> power_off (and therefore
power_on -> init). If they are swapped, disabling fails.
So how was it meant to be?
Cheers,
Ahmad
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* KASAN: null-ptr-deref Write in vhci_shutdown_connection
From: syzbot @ 2020-12-20 18:44 UTC (permalink / raw)
To: gregkh, linux-kernel, linux-usb, shuah, syzkaller-bugs,
valentina.manea.m
Hello,
syzbot found the following issue on:
HEAD commit: 5e60366d Merge tag 'fallthrough-fixes-clang-5.11-rc1' of g..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13f05613500000
kernel config: https://syzkaller.appspot.com/x/.config?x=503d0089cd701d6d
dashboard link: https://syzkaller.appspot.com/bug?extid=a93fba6d384346a761e3
compiler: gcc (GCC) 10.1.0-syz 20200507
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14d0d8c5500000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1058e41f500000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+a93fba6d384346a761e3@syzkaller.appspotmail.com
vhci_hcd: stop threads
vhci_hcd: release socket
vhci_hcd: disconnect device
==================================================================
BUG: KASAN: null-ptr-deref in instrument_atomic_read_write include/linux/instrumented.h:101 [inline]
BUG: KASAN: null-ptr-deref in atomic_fetch_add_relaxed include/asm-generic/atomic-instrumented.h:142 [inline]
BUG: KASAN: null-ptr-deref in __refcount_add include/linux/refcount.h:193 [inline]
BUG: KASAN: null-ptr-deref in __refcount_inc include/linux/refcount.h:250 [inline]
BUG: KASAN: null-ptr-deref in refcount_inc include/linux/refcount.h:267 [inline]
BUG: KASAN: null-ptr-deref in get_task_struct include/linux/sched/task.h:102 [inline]
BUG: KASAN: null-ptr-deref in kthread_stop+0x90/0x760 kernel/kthread.c:591
Write of size 4 at addr 0000000000000024 by task kworker/u4:2/46
CPU: 0 PID: 46 Comm: kworker/u4:2 Not tainted 5.10.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: usbip_event event_handler
Call Trace:
__dump_stack lib/dump_stack.c:79 [inline]
dump_stack+0x107/0x163 lib/dump_stack.c:120
__kasan_report mm/kasan/report.c:549 [inline]
kasan_report.cold+0x5/0x37 mm/kasan/report.c:562
check_memory_region_inline mm/kasan/generic.c:186 [inline]
check_memory_region+0x13d/0x180 mm/kasan/generic.c:192
instrument_atomic_read_write include/linux/instrumented.h:101 [inline]
atomic_fetch_add_relaxed include/asm-generic/atomic-instrumented.h:142 [inline]
__refcount_add include/linux/refcount.h:193 [inline]
__refcount_inc include/linux/refcount.h:250 [inline]
refcount_inc include/linux/refcount.h:267 [inline]
get_task_struct include/linux/sched/task.h:102 [inline]
kthread_stop+0x90/0x760 kernel/kthread.c:591
vhci_shutdown_connection+0x17f/0x340 drivers/usb/usbip/vhci_hcd.c:1021
event_handler+0x1f0/0x4f0 drivers/usb/usbip/usbip_event.c:78
process_one_work+0x98d/0x1630 kernel/workqueue.c:2275
worker_thread+0x64c/0x1120 kernel/workqueue.c:2421
kthread+0x3b1/0x4a0 kernel/kthread.c:292
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:296
==================================================================
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 46 Comm: kworker/u4:2 Tainted: G B 5.10.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: usbip_event event_handler
Call Trace:
__dump_stack lib/dump_stack.c:79 [inline]
dump_stack+0x107/0x163 lib/dump_stack.c:120
panic+0x343/0x77f kernel/panic.c:231
end_report+0x58/0x5e mm/kasan/report.c:106
__kasan_report mm/kasan/report.c:552 [inline]
kasan_report.cold+0xd/0x37 mm/kasan/report.c:562
check_memory_region_inline mm/kasan/generic.c:186 [inline]
check_memory_region+0x13d/0x180 mm/kasan/generic.c:192
instrument_atomic_read_write include/linux/instrumented.h:101 [inline]
atomic_fetch_add_relaxed include/asm-generic/atomic-instrumented.h:142 [inline]
__refcount_add include/linux/refcount.h:193 [inline]
__refcount_inc include/linux/refcount.h:250 [inline]
refcount_inc include/linux/refcount.h:267 [inline]
get_task_struct include/linux/sched/task.h:102 [inline]
kthread_stop+0x90/0x760 kernel/kthread.c:591
vhci_shutdown_connection+0x17f/0x340 drivers/usb/usbip/vhci_hcd.c:1021
event_handler+0x1f0/0x4f0 drivers/usb/usbip/usbip_event.c:78
process_one_work+0x98d/0x1630 kernel/workqueue.c:2275
worker_thread+0x64c/0x1120 kernel/workqueue.c:2421
kthread+0x3b1/0x4a0 kernel/kthread.c:292
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:296
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* [Bug 210767] uvcvideo: Webcam (1f3a:100e) stopped working after 8a652a17e3c005dcdae31b6c8fdf14382a29cbbe
From: bugzilla-daemon @ 2020-12-20 16:42 UTC (permalink / raw)
To: linux-usb
In-Reply-To: <bug-210767-208809@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210767
--- Comment #8 from Till Dörges (doerges@pre-sense.de) ---
Tests were against 5.10.1-2.g8f3d468.
I applied the patch from comment 5
(0001-media-uvcvideo-Accept-invalid-bFormatIndex-and-bFram.patch).
The device is working as expected with that patch.
Thanks!
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 210767] uvcvideo: Webcam (1f3a:100e) stopped working after 8a652a17e3c005dcdae31b6c8fdf14382a29cbbe
From: bugzilla-daemon @ 2020-12-20 16:26 UTC (permalink / raw)
To: linux-usb
In-Reply-To: <bug-210767-208809@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210767
--- Comment #7 from Till Dörges (doerges@pre-sense.de) ---
(In reply to Laurent Pinchart from comment #6)
> Assuming the patch fixes the issue, I'll submit it to the linux-media
> mailing list.
Will try to give the patch a shot asap.
> Till, are you OK to have your name included in the commit
> messages as the reporter ?
Yes, including my name is okay.
Thanks for the quick turnaround!
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 210801] New: TopSeed Technology Corp. eHome Infrared Transceiver spams log with Error: urb status = -71
From: bugzilla-daemon @ 2020-12-20 16:19 UTC (permalink / raw)
To: linux-usb
https://bugzilla.kernel.org/show_bug.cgi?id=210801
Bug ID: 210801
Summary: TopSeed Technology Corp. eHome Infrared Transceiver
spams log with Error: urb status = -71
Product: Drivers
Version: 2.5
Kernel Version: 5.10.1
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: USB
Assignee: drivers_usb@kernel-bugs.kernel.org
Reporter: hpj@urpla.net
Regression: No
Hi,
using a TopSeed Technology Corp. eHome Infrared Transceiver here, my kernel log
is spammed with:
Dec 20 17:03:46 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
with the rate of > 40/second.
Device is shown as:
Bus 006 Device 006: ID 1784:0006 TopSeed Technology Corp. eHome Infrared
Transceiver
lsusb -vs 006:048
Bus 006 Device 048: ID 1784:0006 TopSeed Technology Corp. eHome Infrared
Transceiver
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 0
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 8
idVendor 0x1784 TopSeed Technology Corp.
idProduct 0x0006 eHome Infrared Transceiver
bcdDevice 1.01
iManufacturer 1 TopSeed Technology Corp.
iProduct 2 eHome Infrared Transceiver
iSerial 3 TS002j0C
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 0x0020
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xa0
(Bus Powered)
Remote Wakeup
MaxPower 100mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass 255 Vendor Specific Subclass
bInterfaceProtocol 255 Vendor Specific Protocol
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01 EP 1 OUT
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0020 1x 32 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0020 1x 32 bytes
bInterval 0
can't get device qualifier: Resource temporarily unavailable
can't get debug descriptor: Resource temporarily unavailable
Device Status: 0x0001
Self Powered
When plugged in, these messages are triggered:
Dec 20 17:13:57 kernel: usb 6-10.4: new full-speed USB device number 49 using
xhci_hcd
Dec 20 17:13:58 kernel: usb 6-10.4: config 1 interface 0 altsetting 0 endpoint
0x1 has an invalid bInterval 0, changing to 10
Dec 20 17:13:58 kernel: usb 6-10.4: config 1 interface 0 altsetting 0 endpoint
0x81 has an invalid bInterval 0, changing to 10
Dec 20 17:13:58 kernel: usb 6-10.4: New USB device found, idVendor=1784,
idProduct=0006, bcdDevice= 1.01
Dec 20 17:13:58 kernel: usb 6-10.4: New USB device strings: Mfr=1, Product=2,
SerialNumber=3
Dec 20 17:13:58 kernel: usb 6-10.4: Product: eHome Infrared Transceiver
Dec 20 17:13:58 kernel: usb 6-10.4: Manufacturer: TopSeed Technology Corp.
Dec 20 17:13:58 kernel: usb 6-10.4: SerialNumber: TS002j0C
Dec 20 17:13:58 kernel: Registered IR keymap rc-rc6-mce
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: rc rc0: Media Center Ed. eHome Infrared Remote
Transceiver (1784:0006) as
/devices/pci0000:00/0000:00:14.0/usb6/6-10/6-10.4/6-10.4:1.0/rc/rc0
Dec 20 17:13:58 kernel: rc rc0: lirc_dev: driver mceusb registered at minor =
0, raw IR receiver, raw IR transmitter
Dec 20 17:13:58 kernel: input: Media Center Ed. eHome Infrared Remote
Transceiver (1784:0006) as
/devices/pci0000:00/0000:00:14.0/usb6/6-10/6-10.4/6-10.4:1.0/rc/rc0/input50
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Registered TopSeed Technology Corp.
eHome Infrared Transceiver with mce emulator interface version 1
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: 2 tx ports (0x0 cabled) and 2 rx
sensors (0x0 active)
Dec 20 17:13:58 mtp-probe[6430]: checking bus 6, device 49:
"/sys/devices/pci0000:00/0000:00:14.0/usb6/6-10/6-10.4"
Dec 20 17:13:58 mtp-probe[6430]: bus: 6, device: 49 was not an MTP device
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 upowerd[5834]: treating change event as add on
/sys/devices/pci0000:00/0000:00:14.0/usb6/6-10/6-10.4
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: mce write urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 systemd-logind[1892]: Watching system buttons on
/dev/input/event26 (Media Center Ed. eHome Infrared Remote Transceiver
(1784:0006))
Dec 20 17:13:58 upowerd[5834]: treating change event as add on
/sys/devices/pci0000:00/0000:00:14.0/usb6/6-10/6-10.4/6-10.4:1.0
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 mtp-probe[6449]: checking bus 6, device 49:
"/sys/devices/pci0000:00/0000:00:14.0/usb6/6-10/6-10.4"
Dec 20 17:13:58 mtp-probe[6449]: bus: 6, device: 49 was not an MTP device
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kcminit[6453]: Initializing "kcm_mouse" : "kcminit_mouse"
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:58 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
Dec 20 17:13:59 kernel: mceusb 6-10.4:1.0: Error: urb status = -71
It used to work fine with earlier kernels.
openSUSE Tumbleweed 20201216
5.10.1-7-preempt #1 SMP PREEMPT Tue Dec 15 08:33:02 UTC 2020 (c8c5afb) x86_64
x86_64 x86_64 GNU/Linux
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 210767] uvcvideo: Webcam (1f3a:100e) stopped working after 8a652a17e3c005dcdae31b6c8fdf14382a29cbbe
From: bugzilla-daemon @ 2020-12-20 14:29 UTC (permalink / raw)
To: linux-usb
In-Reply-To: <bug-210767-208809@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210767
--- Comment #6 from Laurent Pinchart (laurent.pinchart+bugzilla-kernel@ideasonboard.com) ---
Assuming the patch fixes the issue, I'll submit it to the linux-media mailing
list. Till, are you OK to have your name included in the commit messages as the
reporter ?
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 210767] uvcvideo: Webcam (1f3a:100e) stopped working after 8a652a17e3c005dcdae31b6c8fdf14382a29cbbe
From: bugzilla-daemon @ 2020-12-20 14:27 UTC (permalink / raw)
To: linux-usb
In-Reply-To: <bug-210767-208809@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210767
--- Comment #5 from Laurent Pinchart (laurent.pinchart+bugzilla-kernel@ideasonboard.com) ---
Created attachment 294241
--> https://bugzilla.kernel.org/attachment.cgi?id=294241&action=edit
Tentative patch
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 210767] uvcvideo: Webcam (1f3a:100e) stopped working after 8a652a17e3c005dcdae31b6c8fdf14382a29cbbe
From: bugzilla-daemon @ 2020-12-20 14:26 UTC (permalink / raw)
To: linux-usb
In-Reply-To: <bug-210767-208809@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210767
--- Comment #4 from Laurent Pinchart (laurent.pinchart+bugzilla-kernel@ideasonboard.com) ---
Thank you for the lsusb output.
The problem is caused by a bug in the device firmware, so we'll need to work
around it. I'll propose a patch.
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 210767] uvcvideo: Webcam (1f3a:100e) stopped working after 8a652a17e3c005dcdae31b6c8fdf14382a29cbbe
From: bugzilla-daemon @ 2020-12-20 11:55 UTC (permalink / raw)
To: linux-usb
In-Reply-To: <bug-210767-208809@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210767
--- Comment #3 from Till Dörges (doerges@pre-sense.de) ---
The device also seems to have issues with USB suspend
(https://sourceforge.net/p/linux-uvc/mailman/message/37178728/)
Is this in any way related?
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: cdc_ncm kernel log spam with trendnet 2.5G USB adapter
From: Roland Dreier @ 2020-12-19 22:21 UTC (permalink / raw)
To: Oliver Neukum; +Cc: netdev, linux-usb
In-Reply-To: <3a9b2c8c275d56d9c7904cf9b5177047b196173d.camel@neukum.org>
(Apologies, trying one more time with a better mailer)
Sorry it took so long, but I finally got a chance to test the patches. They
seem to work well, but they only get rid of the downlink / uplink speed spam -
I still get the following filling my kernel log with a patched kernel:
[ 29.830383] cdc_ncm 2-2:2.0 enp0s2u2c2: network connection: connected
[ 29.894359] cdc_ncm 2-2:2.0 enp0s2u2c2: network connection: connected
[ 29.958601] cdc_ncm 2-2:2.0 enp0s2u2c2: network connection: connected
[ 30.022473] cdc_ncm 2-2:2.0 enp0s2u2c2: network connection: connected
[ 30.086548] cdc_ncm 2-2:2.0 enp0s2u2c2: network connection: connected
with the below patch on top of your 3, then my kernel log is clean.
Please apply your patches plus my patch, and feel free to add
Tested-by: Roland Dreier <roland@kernel.org>
to the other three.
--- 8< ---------- 8< ---
Subject: [PATCH] CDC-NCM: remove "connected" log message
The cdc_ncm driver passes network connection notifications up to
usbnet_link_change(), which is the right place for any logging.
Remove the netdev_info() duplicating this from the driver itself.
This stops devices such as my "TRENDnet USB 10/100/1G/2.5G LAN"
(ID 20f4:e02b) adapter from spamming the kernel log with
cdc_ncm 2-2:2.0 enp0s2u2c2: network connection: connected
messages every 60 msec or so.
Signed-off-by: Roland Dreier <roland@kernel.org>
---
drivers/net/usb/cdc_ncm.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index a45fcc44facf..50d3a4e6d445 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1850,9 +1850,6 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
* USB_CDC_NOTIFY_NETWORK_CONNECTION notification shall be
* sent by device after USB_CDC_NOTIFY_SPEED_CHANGE.
*/
- netif_info(dev, link, dev->net,
- "network connection: %sconnected\n",
- !!event->wValue ? "" : "dis");
usbnet_link_change(dev, !!event->wValue, 0);
break;
--
2.29.2
^ permalink raw reply related
* [Bug 210767] uvcvideo: Webcam (1f3a:100e) stopped working after 8a652a17e3c005dcdae31b6c8fdf14382a29cbbe
From: bugzilla-daemon @ 2020-12-19 19:51 UTC (permalink / raw)
To: linux-usb
In-Reply-To: <bug-210767-208809@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210767
--- Comment #2 from Till Dörges (doerges@pre-sense.de) ---
Created attachment 294237
--> https://bugzilla.kernel.org/attachment.cgi?id=294237&action=edit
Output from lsusb as root
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* UBSAN: shift-out-of-bounds in mceusb_dev_recv
From: syzbot @ 2020-12-19 18:14 UTC (permalink / raw)
To: andreyknvl, linux-kernel, linux-media, linux-usb, mchehab, sean,
syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: 5e60366d Merge tag 'fallthrough-fixes-clang-5.11-rc1' of g..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
console output: https://syzkaller.appspot.com/x/log.txt?x=17386d37500000
kernel config: https://syzkaller.appspot.com/x/.config?x=5cea7506b7139727
dashboard link: https://syzkaller.appspot.com/bug?extid=ec3b3128c576e109171d
compiler: gcc (GCC) 10.1.0-syz 20200507
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=178e3e0f500000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1029fccb500000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+ec3b3128c576e109171d@syzkaller.appspotmail.com
================================================================================
UBSAN: shift-out-of-bounds in drivers/media/rc/mceusb.c:1173:29
shift exponent 119 is too large for 32-bit type 'int'
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.10.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:79 [inline]
dump_stack+0x107/0x163 lib/dump_stack.c:120
ubsan_epilogue+0xb/0x5a lib/ubsan.c:148
__ubsan_handle_shift_out_of_bounds.cold+0xb1/0x181 lib/ubsan.c:395
mceusb_handle_command drivers/media/rc/mceusb.c:1173 [inline]
mceusb_process_ir_data drivers/media/rc/mceusb.c:1278 [inline]
mceusb_dev_recv.cold+0x188/0x220 drivers/media/rc/mceusb.c:1376
__usb_hcd_giveback_urb+0x2b0/0x5c0 drivers/usb/core/hcd.c:1657
usb_hcd_giveback_urb+0x38c/0x430 drivers/usb/core/hcd.c:1728
dummy_timer+0x11f4/0x32a0 drivers/usb/gadget/udc/dummy_hcd.c:1971
call_timer_fn+0x1a5/0x690 kernel/time/timer.c:1417
expire_timers kernel/time/timer.c:1462 [inline]
__run_timers.part.0+0x692/0xa50 kernel/time/timer.c:1731
__run_timers kernel/time/timer.c:1712 [inline]
run_timer_softirq+0x80/0x120 kernel/time/timer.c:1744
__do_softirq+0x1b7/0x9c5 kernel/softirq.c:343
asm_call_irq_on_stack+0xf/0x20
</IRQ>
__run_on_irqstack arch/x86/include/asm/irq_stack.h:26 [inline]
run_on_irqstack_cond arch/x86/include/asm/irq_stack.h:77 [inline]
do_softirq_own_stack+0x80/0xa0 arch/x86/kernel/irq_64.c:77
invoke_softirq kernel/softirq.c:226 [inline]
__irq_exit_rcu+0x119/0x1b0 kernel/softirq.c:420
irq_exit_rcu+0x5/0x10 kernel/softirq.c:432
sysvec_apic_timer_interrupt+0x43/0xa0 arch/x86/kernel/apic/apic.c:1096
asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:628
RIP: 0010:native_save_fl arch/x86/include/asm/irqflags.h:29 [inline]
RIP: 0010:arch_local_save_flags arch/x86/include/asm/irqflags.h:79 [inline]
RIP: 0010:arch_irqs_disabled arch/x86/include/asm/irqflags.h:169 [inline]
RIP: 0010:acpi_safe_halt drivers/acpi/processor_idle.c:111 [inline]
RIP: 0010:acpi_idle_do_entry+0x1c9/0x250 drivers/acpi/processor_idle.c:516
Code: 8d 61 7f fb 84 db 75 ac e8 04 5b 7f fb e8 4f 0f 85 fb e9 0c 00 00 00 e8 f5 5a 7f fb 0f 00 2d ce 86 87 00 e8 e9 5a 7f fb fb f4 <9c> 5b 81 e3 00 02 00 00 fa 31 ff 48 89 de e8 c4 62 7f fb 48 85 db
RSP: 0018:ffffffff87407d60 EFLAGS: 00000293
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffffffff87431940 RSI: ffffffff85c0eb77 RDI: ffffffff85c0eb61
RBP: ffff888102eb7064 R08: 0000000000000001 R09: 0000000000000001
R10: ffffffff8145fae8 R11: 0000000000000000 R12: 0000000000000001
R13: ffff888102eb7000 R14: ffff888102eb7064 R15: ffff888105c87004
acpi_idle_enter+0x355/0x4f0 drivers/acpi/processor_idle.c:647
cpuidle_enter_state+0x1b1/0xc80 drivers/cpuidle/cpuidle.c:237
cpuidle_enter+0x4a/0xa0 drivers/cpuidle/cpuidle.c:351
call_cpuidle kernel/sched/idle.c:158 [inline]
cpuidle_idle_call kernel/sched/idle.c:239 [inline]
do_idle+0x3df/0x580 kernel/sched/idle.c:299
cpu_startup_entry+0x14/0x20 kernel/sched/idle.c:396
start_kernel+0x498/0x4b9 init/main.c:1061
secondary_startup_64_no_verify+0xb0/0xbb
================================================================================
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* [PATCH] USB: cdc-wdm: Fix use after free in service_outstanding_interrupt().
From: Tetsuo Handa @ 2020-12-19 15:25 UTC (permalink / raw)
To: oneukum
Cc: syzbot, linux-usb, syzkaller-bugs, andreyknvl, gregkh, gustavoars,
ingrassia, lee.jones
In-Reply-To: <7a740f5a-65f9-b1d6-1224-4938d61b74a5@i-love.sakura.ne.jp>
syzbot is reporting UAF at usb_submit_urb() [1], for
service_outstanding_interrupt() is not checking WDM_DISCONNECTING
before calling usb_submit_urb(). Close the race by doing same checks
wdm_read() does upon retry.
Also, while wdm_read() checks WDM_DISCONNECTING with desc->rlock held,
service_interrupt_work() does not hold desc->rlock. Thus, it is possible
that usb_submit_urb() is called from service_outstanding_interrupt() from
service_interrupt_work() after WDM_DISCONNECTING was set and kill_urbs()
from wdm_disconnect() completed. Thus, move kill_urbs() in
wdm_disconnect() to after cancel_work_sync() (which makes sure that
service_interrupt_work() is no longer running) completed.
Although it seems to be safe to dereference desc->intf->dev in
service_outstanding_interrupt() even if WDM_DISCONNECTING was already set
because desc->rlock or cancel_work_sync() prevents wdm_disconnect() from
reaching list_del() before service_outstanding_interrupt() completes,
let's not emit error message if WDM_DISCONNECTING is set by
wdm_disconnect() while usb_submit_urb() is in progress.
[1] https://syzkaller.appspot.com/bug?extid=9e04e2df4a32fb661daf
Reported-by: syzbot <syzbot+9e04e2df4a32fb661daf@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
drivers/usb/class/cdc-wdm.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 02d0cfd23bb2..508b1c3f8b73 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -465,13 +465,23 @@ static int service_outstanding_interrupt(struct wdm_device *desc)
if (!desc->resp_count || !--desc->resp_count)
goto out;
+ if (test_bit(WDM_DISCONNECTING, &desc->flags)) {
+ rv = -ENODEV;
+ goto out;
+ }
+ if (test_bit(WDM_RESETTING, &desc->flags)) {
+ rv = -EIO;
+ goto out;
+ }
+
set_bit(WDM_RESPONDING, &desc->flags);
spin_unlock_irq(&desc->iuspin);
rv = usb_submit_urb(desc->response, GFP_KERNEL);
spin_lock_irq(&desc->iuspin);
if (rv) {
- dev_err(&desc->intf->dev,
- "usb_submit_urb failed with result %d\n", rv);
+ if (!test_bit(WDM_DISCONNECTING, &desc->flags))
+ dev_err(&desc->intf->dev,
+ "usb_submit_urb failed with result %d\n", rv);
/* make sure the next notification trigger a submit */
clear_bit(WDM_RESPONDING, &desc->flags);
@@ -1027,9 +1037,9 @@ static void wdm_disconnect(struct usb_interface *intf)
wake_up_all(&desc->wait);
mutex_lock(&desc->rlock);
mutex_lock(&desc->wlock);
- kill_urbs(desc);
cancel_work_sync(&desc->rxwork);
cancel_work_sync(&desc->service_outs_intr);
+ kill_urbs(desc);
mutex_unlock(&desc->wlock);
mutex_unlock(&desc->rlock);
--
2.25.1
^ permalink raw reply related
* [PATCH 5.4 27/34] USB: sisusbvga: Make console support depend on BROKEN
From: Greg Kroah-Hartman @ 2020-12-19 13:03 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Thomas Gleixner, Thomas Winischhofer,
linux-usb
In-Reply-To: <20201219125341.384025953@linuxfoundation.org>
From: Thomas Gleixner <tglx@linutronix.de>
commit 862ee699fefe1e6d6f2c1518395f0b999b8beb15 upstream.
The console part of sisusbvga is broken vs. printk(). It uses in_atomic()
to detect contexts in which it cannot sleep despite the big fat comment in
preempt.h which says: Do not use in_atomic() in driver code.
in_atomic() does not work on kernels with CONFIG_PREEMPT_COUNT=n which
means that spin/rw_lock held regions are not detected by it.
There is no way to make this work by handing context information through to
the driver and this only can be solved once the core printk infrastructure
supports sleepable console drivers.
Make it depend on BROKEN for now.
Fixes: 1bbb4f2035d9 ("[PATCH] USB: sisusb[vga] update")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Thomas Winischhofer <thomas@winischhofer.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20201019101109.603244207@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/usb/misc/sisusbvga/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/usb/misc/sisusbvga/Kconfig
+++ b/drivers/usb/misc/sisusbvga/Kconfig
@@ -16,7 +16,7 @@ config USB_SISUSBVGA
config USB_SISUSBVGA_CON
bool "Text console and mode switching support" if USB_SISUSBVGA
- depends on VT
+ depends on VT && BROKEN
select FONT_8x16
---help---
Say Y here if you want a VGA text console via the USB dongle or
^ permalink raw reply
* [PATCH 5.9 43/49] USB: sisusbvga: Make console support depend on BROKEN
From: Greg Kroah-Hartman @ 2020-12-19 12:58 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Thomas Gleixner, Thomas Winischhofer,
linux-usb
In-Reply-To: <20201219125344.671832095@linuxfoundation.org>
From: Thomas Gleixner <tglx@linutronix.de>
commit 862ee699fefe1e6d6f2c1518395f0b999b8beb15 upstream.
The console part of sisusbvga is broken vs. printk(). It uses in_atomic()
to detect contexts in which it cannot sleep despite the big fat comment in
preempt.h which says: Do not use in_atomic() in driver code.
in_atomic() does not work on kernels with CONFIG_PREEMPT_COUNT=n which
means that spin/rw_lock held regions are not detected by it.
There is no way to make this work by handing context information through to
the driver and this only can be solved once the core printk infrastructure
supports sleepable console drivers.
Make it depend on BROKEN for now.
Fixes: 1bbb4f2035d9 ("[PATCH] USB: sisusb[vga] update")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Thomas Winischhofer <thomas@winischhofer.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20201019101109.603244207@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/usb/misc/sisusbvga/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/usb/misc/sisusbvga/Kconfig
+++ b/drivers/usb/misc/sisusbvga/Kconfig
@@ -16,7 +16,7 @@ config USB_SISUSBVGA
config USB_SISUSBVGA_CON
bool "Text console and mode switching support" if USB_SISUSBVGA
- depends on VT
+ depends on VT && BROKEN
select FONT_8x16
help
Say Y here if you want a VGA text console via the USB dongle or
^ permalink raw reply
* [PATCH 5.10 14/16] USB: sisusbvga: Make console support depend on BROKEN
From: Greg Kroah-Hartman @ 2020-12-19 12:57 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Thomas Gleixner, Thomas Winischhofer,
linux-usb
In-Reply-To: <20201219125339.066340030@linuxfoundation.org>
From: Thomas Gleixner <tglx@linutronix.de>
commit 862ee699fefe1e6d6f2c1518395f0b999b8beb15 upstream.
The console part of sisusbvga is broken vs. printk(). It uses in_atomic()
to detect contexts in which it cannot sleep despite the big fat comment in
preempt.h which says: Do not use in_atomic() in driver code.
in_atomic() does not work on kernels with CONFIG_PREEMPT_COUNT=n which
means that spin/rw_lock held regions are not detected by it.
There is no way to make this work by handing context information through to
the driver and this only can be solved once the core printk infrastructure
supports sleepable console drivers.
Make it depend on BROKEN for now.
Fixes: 1bbb4f2035d9 ("[PATCH] USB: sisusb[vga] update")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Thomas Winischhofer <thomas@winischhofer.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20201019101109.603244207@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/usb/misc/sisusbvga/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/usb/misc/sisusbvga/Kconfig
+++ b/drivers/usb/misc/sisusbvga/Kconfig
@@ -16,7 +16,7 @@ config USB_SISUSBVGA
config USB_SISUSBVGA_CON
bool "Text console and mode switching support" if USB_SISUSBVGA
- depends on VT
+ depends on VT && BROKEN
select FONT_8x16
help
Say Y here if you want a VGA text console via the USB dongle or
^ permalink raw reply
* Re: [PATCH v2 0/7] tty: add flag to suppress ready signalling on open
From: Mychaela Falconia @ 2020-12-18 18:03 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Maarten Brock, Jiri Slaby,
Mychaela N . Falconia, linux-serial, linux-usb, linux-kernel
In-Reply-To: <X9d039qPr/LO/2R/@localhost>
Greg K-H wrote:
> We see devices that are "obviously" not the real vid/pid all the time in
> the wild. There's nothing "illegal" about another company using your
> vid/pid, look at all of the ones out there already that use the FTDI
> vendor id yet are "clones", same with pl2303 devices.
But are those reusers of someone else's VID or PID coming to Linux
kernel maintainers with requests to modify ftdi_sio or pl2303 drivers
to work with their clones? Do you ever see LKML posts along the lines
of "Hi, I am so and so from such and such company, we are not FTDI but
we reuse FTDI's VID and PIDs, but our clone chip does not match the
original and we need to modify the ftdi_sio driver to work with our
poor man's clone chip" - do reusers of someone else's VID or PID come
here with such requests?
Let's apply this logic to the very specific example of USB VID:PID
0x0403:0x7152. Right now I am asking you, esteemed maintainers, to
accept my patch to ftdi_sio adding support for this new custom ID
along with the necessary quirk. If my patch were accepted and then
someone else squats on this USB ID, how would it be a problem for
anyone other than that squatter? Are you expecting that someone is
going to file a bug report saying "I am using 0x0403:0x7152 for my own
USB device, but there is a quirk for this ID in ftdi_sio, and I don't
want that quirk for MY device using that ID" - is this what you are
anticipating? If that scenario were to occur, how could it possibly
be right to choose that hypothetical complainant's interest over mine
if one can trivially check with FTDI and confirm that 0x0403:0x7152 is
officially allocated to Falconia and not to the other contesting party?
> We also have fuzzing devices that spoof vid/pid pairs in order to test
> kernel code, as well as being used as malicious devices to hack systems
> or do other "fun" things.
And where is the harm? If my patch adding ftdi_sio support for
0x0403:0x7152 with the special quirk I am asking for were to be
accepted, and then someone presents this USB ID to a target system as
a form of hack or fuzzing test, where is the harm? If anything, such
action would be a good thing, as it would exercise the quirk function
and ensure that it doesn't crash the kernel or anything like that.
Yes, the quirk flag will be set on that ttyUSB device - but if they
are deliberately presenting a USB ID whose owner said "this ID means
that this quirk is needed", then how is it a bad thing to apply the
quirk when that ID is detected, fuzzing/hacking or not?
> Blindly trusting these numbers are something
> we can no longer do.
Please clarify what you mean by "blindly trusting" in this very
specific context. Suppose some company makes an entirely new USB-
interfaced chip, they assign it a unique ID (obtained officially
through proper channels, not taking squatting on else's), and they
submit a brand new Linux driver for their brand new chip, a driver
that naturally binds to that chip's new unique ID - are you going to
reject that driver and keep the new chip forever unsupported by Linux
because you are concerned that someone else will reuse that same USB
ID for something completely different? How is my case any different?
The only difference is that in my case the custom hw product is a
board and not a chip - but how are board-level designs any less worthy
of mainline Linux support than chip-level ones?
Johan Hovold wrote:
> My reasons for proposing the NORDY sysfs interface *and* termios flag
> are as follows:
The patch series presented so far provides the sysfs interface and the
ftdi_sio ID-code-specific quirk for DUART28C (which would make me
happy), but does not add any new termios flag. Are you planning to
present a new patch series that also adds the new termios flag?
> You cannot generally rely on the state of these lines before opening
> the port at least once,
I disagree. Manufacturers of USB-serial chips like FT232x/FT2232x
*guarantee* (see FTDI's AN 184) that their DTR/RTS CMOS outputs *will
not* drive low (they will either drive high or be tristated) until and
unless an explicit command comes in on the USB control pipe to change
those signals to the "on" state of CMOS low output. Therefore,
hardware engineers have a right to demand from OS maintainers that
OSes (who are middlemen between applications and hw) provide a way to
NOT issue that particular USB control pipe command if it is not wanted
by the application.
> but for applications where this is possible
> and where even toggling them once is undesirable
s/undesirable/killer/, for the specific case of UART Channel B on
FreeCalypso DUART28C, the custom board with the custom USB ID at the
crux of my quest.
> we *can* provide
> some out-of-band mechanism to change the default state of the NORDY
> flag (but we could also ignore this use case, keeping the status
> quo).
In the case of the parenthetical option, are you basically saying
"screw you" to me and my users, refusing to mainline our zero-effect-
on-others USB ID-driven quirk patch, the one without which the device
cannot work with Linux?
> This could be an interface to control just the initial state of this
> flag after probe() or it can be used in parallel with the termios
> interface. The latter is what is implemented here.
My concern with the termios flag idea is that in the absence of a
specific proposed patch to review, I don't have a clear view of
exactly where this new flag will be inserted, and I have a concern
about this new flag breaking things.
Right now my custom userspace programs initialize termios like this:
#include <asm/ioctls.h>
#include <asm/termbits.h>
...
struct termios2 target_termios;
target_termios.c_iflag = IGNBRK;
target_termios.c_oflag = 0;
target_termios.c_cflag = br->termios_code | CLOCAL|HUPCL|CREAD|CS8;
target_termios.c_lflag = 0;
target_termios.c_cc[VMIN] = 1;
target_termios.c_cc[VTIME] = 0;
target_termios.c_ispeed = br->nonstd_speed;
target_termios.c_ospeed = br->nonstd_speed;
if (ioctl(target_fd, TCSETSF2, &target_termios) < 0) {
perror("TCSETSF2");
exit(1);
}
br->termios_code will be codes like B115200 for standard baud rates,
or BOTHER for GSM-specific baud rates like 812500, with the actual bps
number going into br->nonstd_speed. You mention C libraries below,
and there is an important lesson to be learned here: as I understand
it, support for non-standard baud rates via BOTHER was added to Linux
as in kernel ages ago (at the urging of embedded systems folks like
me), but libc people (whichever libc it is, I don't care for their
politics) apparently never got the memo, and it appears to be
impossible to set non-standard serial baud rates using standard libc
headers and termios APIs. It appears that the *only* way to set a
non-standard serial port baud rate from a lowly Linux userspace app is
to use <asm/xxx.h> kernel headers and raw ioctls - and as I quickly
found out, the necessary <asm/xxx.h> headers *conflict* with standard
libc ones, hence the necessary code needs to be moved out into its own
compilation unit that doesn't need much in the way of other libc
headers.
Back to your proposed new termios flag: are you thinking about adding
it to c_cflag, where CLOCAL and HUPCL currently live? If so, it would
be pretty much impossible for userspace code like mine to not clear
this new flag. You could argue that my approach of absolutely setting
all termios fields is wrong, that I should read the previous termios
state and make my desired changes, but considering that most termios
flags are just unwanted noise for raw byte I/O applications, approaches
like mine (init termios from scratch) are probably commonplace. OK,
I could read previous termios just to read and preserve the NORDY flag
- but how would I do it when it will be years (if ever) before the new
flag definition appears in userspace-available headers?
So here is my concern: suppose that the "generic" solution you guys
end up implementing and merging will revolve around a new termios flag
added to c_cflag or some similar place, along with a sysfs interface,
and maybe even an ftdi_sio driver quirk to set this flag automatically
when DUART28C custom USB ID is detected. The first open will be good
- but the termios flag will get reset, and breakage will occur with
subsequent runs of the same userspace programs messing up the hw. I
suppose I could implement a workaround by adding code to my FC host
tools userspace sw suite to always do a sysfs write before opening the
serial port, but it certainly isn't anywhere near clean.
If you still insist on a termios flag rather than just sysfs, one
possible clean solution would be to have two separate flags in the
kernel's internal data structures: the new termios flag would end up
inside struct ktermios in the kernel, whereas the flag manipulated via
sysfs or set in my DUART28C USB ID quirk goes into struct tty_port
iflags like in Johan's current patch. If either flag is set, suppress
automatic DTR & RTS assertion on open. Alternatively, put the new
flag under getserial/setserial Linux-specific ioctls, rather than
termios - then it won't get inadvertently cleared so easily.
> As a bonus, using sysfs for this allows this feature to be used also
> before NORDY support has been added to the c libraries.
This part is absolutely crucial: the new kernel feature will be
essentially useless if the lack of support in userspace-available
headers makes it inaccessible. For devices other than my specific
DUART28C which I still argue should be covered by an ftdi_sio driver
quirk, sysfs is the only viable option for immediate relief of users'
suffering.
Oh, and here is another reason why a USB-ID-driven ftdi_sio quirk
patch is pretty much an absolute requirement for me: even if some
generic solution that works for "any" serial port were to be implemented
in current bleeding edge mainline, it will be years before these new
kernels percolate to end users to a sufficient degree to be considered
ubiquitous. Thus for the foreseeable future, I *must* support users
of old kernels, and because the necessary feature does not exist there,
directing users to apply a local patch is the only option. And if I
am telling users to apply a local patch to their kernels, that patch
needs to be minimally-invasive-surgical, self-contained entirely within
the ftdi_sio driver and not touching anything outside of it, and as
easy as possible to apply to a very wide range of old kernel versions.
Whatever solution you guys are going to come up with in terms of new
termios flags and/or sysfs, it will be infeasible to package it in a
form of a retro-patch which any Joe End User can trivially apply to
his or her old system running whatever random old kernel version - but
my self-contained ftdi_sio patch adding support for my custom USB ID
with the needed quirk *is* trivially applicable by end users on just
about any old system. Thus I am begging and pleading with you to
accept my DUART28C USB ID quirk *in addition* to whatever "generic" or
"works for anyone" solution you esteemed gentlemen come up with.
> Note that one of the BSDs recently added a termios flag with the same
> semantics as the proposed NORDY which seems to suggest that this is
> interface is indeed a natural one.
Would you happen to have a link or at least some specific search
terms? I would like to know which BSD it is, how they named their new
termios flag which you say does the same thing, and where they inserted
it, to get an idea of what to expect if going this route.
> Side note: Also the Windows API has a setting for the state of these
> lines *after* open (i.e. similar to a termios flag),
I would not describe it as being similar to a termios flag, instead I
would describe it as similar to TIOCMBIS and TIOCMBIC. Perhaps they
retain the state on close-then-reopen sequences - I don't know that
part.
> and there are
> reports of Windows users not expecting the lines to be raised on first
> open
I read reports from Python users who were migrating apps from Windows
to Linux, expecting easy portability because it's Python, and they got
bitten by automatic DTR & RTS assertion under Linux which didn't
happen for them under Windows. I don't work with Python at all, so I
don't know much in the way of details, but supposedly Python maintainers
responded that it is a known Linux limitation, that they can control
the state of lines only after initial open, and can't avoid an initial
glitch - whereas under Windows they did avoid that initial glitch,
going through layers of Python, not even using Win32 API directly.
> (and behaviour changing between OS releases). For FTDI devices
> there appears to be some driver-specific out-of-band mechanism in the
> system properties for setting the default behaviour.
I didn't know this part.
Mychaela,
she/her/hers
^ permalink raw reply
* Re: [PATCH] xhci: tegra: Delay for disabling LFPS detector
From: Greg KH @ 2020-12-18 16:53 UTC (permalink / raw)
To: JC Kuo
Cc: mathias.nyman, thierry.reding, jonathanh, robh, linux-tegra,
linux-usb, nkristam
In-Reply-To: <20201218164234.128762-1-jckuo@nvidia.com>
On Sat, Dec 19, 2020 at 12:42:34AM +0800, JC Kuo wrote:
> Occasionally, we are seeing some SuperSpeed devices resumes right after
> being directed to U3. This commits add 500us delay to ensure LFPS
> detector is disabled before sending ACK to firmware.
>
> [ 16.099363] tegra-xusb 70090000.usb: entering ELPG
> [ 16.104343] tegra-xusb 70090000.usb: 2-1 isn't suspended: 0x0c001203
> [ 16.114576] tegra-xusb 70090000.usb: not all ports suspended: -16
> [ 16.120789] tegra-xusb 70090000.usb: entering ELPG failed
>
> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> ---
> drivers/usb/host/xhci-tegra.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 934be1686352..20cdc11f7dc6 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -623,6 +623,12 @@ static void tegra_xusb_mbox_handle(struct tegra_xusb *tegra,
> enable);
> if (err < 0)
> break;
> +
> + /*
> + * wait 500us for LFPS detector to be disabled before sending ACK
> + */
> + if (!enable)
> + usleep_range(500, 1000);
Where does the magic 500us come from? How can we "know" this is long
enough?
thanks,
greg k-h
^ permalink raw reply
* [PATCH] xhci: tegra: Delay for disabling LFPS detector
From: JC Kuo @ 2020-12-18 16:42 UTC (permalink / raw)
To: mathias.nyman, gregkh, thierry.reding, jonathanh, robh
Cc: linux-tegra, linux-usb, nkristam, JC Kuo
Occasionally, we are seeing some SuperSpeed devices resumes right after
being directed to U3. This commits add 500us delay to ensure LFPS
detector is disabled before sending ACK to firmware.
[ 16.099363] tegra-xusb 70090000.usb: entering ELPG
[ 16.104343] tegra-xusb 70090000.usb: 2-1 isn't suspended: 0x0c001203
[ 16.114576] tegra-xusb 70090000.usb: not all ports suspended: -16
[ 16.120789] tegra-xusb 70090000.usb: entering ELPG failed
Signed-off-by: JC Kuo <jckuo@nvidia.com>
---
drivers/usb/host/xhci-tegra.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 934be1686352..20cdc11f7dc6 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -623,6 +623,12 @@ static void tegra_xusb_mbox_handle(struct tegra_xusb *tegra,
enable);
if (err < 0)
break;
+
+ /*
+ * wait 500us for LFPS detector to be disabled before sending ACK
+ */
+ if (!enable)
+ usleep_range(500, 1000);
}
if (err < 0) {
--
2.25.1
^ permalink raw reply related
* Re: KASAN: use-after-free Read in service_outstanding_interrupt
From: Tetsuo Handa @ 2020-12-18 14:03 UTC (permalink / raw)
To: oneukum
Cc: syzbot, linux-usb, syzkaller-bugs, andreyknvl, gregkh, gustavoars,
ingrassia, lee.jones
In-Reply-To: <000000000000994d2a05b6b49959@google.com>
Hello, Oliver.
I guess that this is caused by not checking WDM_DISCONNECTING/WDM_RESETTING
before usb_submit_urb(), and below diff seems to avoid use-after-free for
the C reproducer syzbot has found.
Since service_outstanding_interrupt() is called from service_interrupt_work()
without holding desc->rlock, it is possible that kill_urbs() is called by
wdm_disconnect() when service_outstanding_interrupt() is preempted between
spin_unlock_irq(&desc->iuspin) and usb_submit_urb(), isn't it?
Therefore, we also need to make sure that kill_urbs(desc) is called after
cancel_work_sync(&desc->service_outs_intr) which waits for
service_interrupt_work() to return completed, don't we?
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 02d0cfd23bb2..508b1c3f8b73 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -465,13 +465,23 @@ static int service_outstanding_interrupt(struct wdm_device *desc)
if (!desc->resp_count || !--desc->resp_count)
goto out;
+ if (test_bit(WDM_DISCONNECTING, &desc->flags)) {
+ rv = -ENODEV;
+ goto out;
+ }
+ if (test_bit(WDM_RESETTING, &desc->flags)) {
+ rv = -EIO;
+ goto out;
+ }
+
set_bit(WDM_RESPONDING, &desc->flags);
spin_unlock_irq(&desc->iuspin);
rv = usb_submit_urb(desc->response, GFP_KERNEL);
spin_lock_irq(&desc->iuspin);
if (rv) {
- dev_err(&desc->intf->dev,
- "usb_submit_urb failed with result %d\n", rv);
+ if (!test_bit(WDM_DISCONNECTING, &desc->flags))
+ dev_err(&desc->intf->dev,
+ "usb_submit_urb failed with result %d\n", rv);
/* make sure the next notification trigger a submit */
clear_bit(WDM_RESPONDING, &desc->flags);
@@ -1027,9 +1037,9 @@ static void wdm_disconnect(struct usb_interface *intf)
wake_up_all(&desc->wait);
mutex_lock(&desc->rlock);
mutex_lock(&desc->wlock);
- kill_urbs(desc);
cancel_work_sync(&desc->rxwork);
cancel_work_sync(&desc->service_outs_intr);
+ kill_urbs(desc);
mutex_unlock(&desc->wlock);
mutex_unlock(&desc->rlock);
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox