* [syzbot] [usb?] BUG: sleeping function called from invalid context in mon_complete
@ 2025-09-13 1:58 syzbot
2025-09-15 1:29 ` [PATCH] usb: mon: Make mon_bus::lock a raw spinlock Lizhi Xu
0 siblings, 1 reply; 8+ messages in thread
From: syzbot @ 2025-09-13 1:58 UTC (permalink / raw)
To: gregkh, linux-kernel, linux-usb, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: f777d1112ee5 Merge tag 'vfs-6.17-rc6.fixes' of git://git.k..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10d23562580000
kernel config: https://syzkaller.appspot.com/x/.config?x=429771c55b615e85
dashboard link: https://syzkaller.appspot.com/bug?extid=205ef33a3b636b4181fb
compiler: Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
Unfortunately, I don't have any reproducer for this issue yet.
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/c6eded69cf3a/disk-f777d111.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/9b977d9bf359/vmlinux-f777d111.xz
kernel image: https://storage.googleapis.com/syzbot-assets/0743e13a2027/bzImage-f777d111.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+205ef33a3b636b4181fb@syzkaller.appspotmail.com
usb 38-1: SetAddress Request (5) to port 0
BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 45, name: kworker/1:1
preempt_count: 0, expected: 0
RCU nest depth: 0, expected: 0
6 locks held by kworker/1:1/45:
#0: ffff888021690938 ((wq_completion)usb_hub_wq){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3211 [inline]
#0: ffff888021690938 ((wq_completion)usb_hub_wq){+.+.}-{0:0}, at: process_scheduled_works+0x9b4/0x17b0 kernel/workqueue.c:3319
#1: ffffc90000b67bc0 ((work_completion)(&hub->events)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3212 [inline]
#1: ffffc90000b67bc0 ((work_completion)(&hub->events)){+.+.}-{0:0}, at: process_scheduled_works+0x9ef/0x17b0 kernel/workqueue.c:3319
#2: ffff888146758188 (&dev->mutex){....}-{4:4}, at: device_lock include/linux/device.h:911 [inline]
#2: ffff888146758188 (&dev->mutex){....}-{4:4}, at: hub_event+0x184/0x4a20 drivers/usb/core/hub.c:5898
#3: ffff88814675d5b8 (&port_dev->status_lock){+.+.}-{4:4}, at: usb_lock_port drivers/usb/core/hub.c:3251 [inline]
#3: ffff88814675d5b8 (&port_dev->status_lock){+.+.}-{4:4}, at: hub_port_connect drivers/usb/core/hub.c:5463 [inline]
#3: ffff88814675d5b8 (&port_dev->status_lock){+.+.}-{4:4}, at: hub_port_connect_change drivers/usb/core/hub.c:5706 [inline]
#3: ffff88814675d5b8 (&port_dev->status_lock){+.+.}-{4:4}, at: port_event drivers/usb/core/hub.c:5870 [inline]
#3: ffff88814675d5b8 (&port_dev->status_lock){+.+.}-{4:4}, at: hub_event+0x21b8/0x4a20 drivers/usb/core/hub.c:5952
#4: ffff8880283ada58 (hcd->address0_mutex){+.+.}-{4:4}, at: hub_port_connect drivers/usb/core/hub.c:5464 [inline]
#4: ffff8880283ada58 (hcd->address0_mutex){+.+.}-{4:4}, at: hub_port_connect_change drivers/usb/core/hub.c:5706 [inline]
#4: ffff8880283ada58 (hcd->address0_mutex){+.+.}-{4:4}, at: port_event drivers/usb/core/hub.c:5870 [inline]
#4: ffff8880283ada58 (hcd->address0_mutex){+.+.}-{4:4}, at: hub_event+0x21e5/0x4a20 drivers/usb/core/hub.c:5952
#5: ffff8880289ba668 (&mbus->lock#2){+.+.}-{3:3}, at: spin_lock include/linux/spinlock_rt.h:44 [inline]
#5: ffff8880289ba668 (&mbus->lock#2){+.+.}-{3:3}, at: mon_bus_complete drivers/usb/mon/mon_main.c:134 [inline]
#5: ffff8880289ba668 (&mbus->lock#2){+.+.}-{3:3}, at: mon_complete+0x5c/0x200 drivers/usb/mon/mon_main.c:147
irq event stamp: 1218766
hardirqs last enabled at (1218765): [<ffffffff8afa3325>] __raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:151 [inline]
hardirqs last enabled at (1218765): [<ffffffff8afa3325>] _raw_spin_unlock_irqrestore+0x85/0x110 kernel/locking/spinlock.c:194
hardirqs last disabled at (1218766): [<ffffffff8718ba3c>] vhci_urb_enqueue+0xb2c/0xe70 drivers/usb/usbip/vhci_hcd.c:817
softirqs last enabled at (1211928): [<ffffffff8184cfa4>] __local_bh_enable_ip+0x1a4/0x270 kernel/softirq.c:262
softirqs last disabled at (1211904): [<ffffffff88dfaeaf>] local_bh_disable include/linux/bottom_half.h:20 [inline]
softirqs last disabled at (1211904): [<ffffffff88dfaeaf>] rcu_read_lock_bh include/linux/rcupdate.h:892 [inline]
softirqs last disabled at (1211904): [<ffffffff88dfaeaf>] __dev_queue_xmit+0x26f/0x3b70 net/core/dev.c:4650
CPU: 1 UID: 0 PID: 45 Comm: kworker/1:1 Not tainted syzkaller #0 PREEMPT_{RT,(full)}
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/12/2025
Workqueue: usb_hub_wq hub_event
Call Trace:
<TASK>
dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
__might_resched+0x44b/0x5d0 kernel/sched/core.c:8957
__rt_spin_lock kernel/locking/spinlock_rt.c:48 [inline]
rt_spin_lock+0xc7/0x2c0 kernel/locking/spinlock_rt.c:57
spin_lock include/linux/spinlock_rt.h:44 [inline]
mon_bus_complete drivers/usb/mon/mon_main.c:134 [inline]
mon_complete+0x5c/0x200 drivers/usb/mon/mon_main.c:147
usbmon_urb_complete include/linux/usb/hcd.h:738 [inline]
__usb_hcd_giveback_urb+0x254/0x5e0 drivers/usb/core/hcd.c:1647
vhci_urb_enqueue+0xb4f/0xe70 drivers/usb/usbip/vhci_hcd.c:818
usb_hcd_submit_urb+0x320/0x1a80 drivers/usb/core/hcd.c:1546
usb_start_wait_urb+0x114/0x4c0 drivers/usb/core/message.c:59
usb_internal_control_msg drivers/usb/core/message.c:103 [inline]
usb_control_msg+0x232/0x3e0 drivers/usb/core/message.c:154
hub_set_address drivers/usb/core/hub.c:4769 [inline]
hub_port_init+0xe24/0x2800 drivers/usb/core/hub.c:5074
hub_port_connect drivers/usb/core/hub.c:5495 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5706 [inline]
port_event drivers/usb/core/hub.c:5870 [inline]
hub_event+0x2532/0x4a20 drivers/usb/core/hub.c:5952
process_one_work kernel/workqueue.c:3236 [inline]
process_scheduled_works+0xade/0x17b0 kernel/workqueue.c:3319
worker_thread+0x8a0/0xda0 kernel/workqueue.c:3400
kthread+0x70e/0x8a0 kernel/kthread.c:463
ret_from_fork+0x3f9/0x770 arch/x86/kernel/process.c:148
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
</TASK>
usb 38-1: new SuperSpeed USB device number 5 using vhci_hcd
usb 38-1: device descriptor read/8, error -110
usb usb38-port1: unable to enumerate USB device
---
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.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] usb: mon: Make mon_bus::lock a raw spinlock
2025-09-13 1:58 [syzbot] [usb?] BUG: sleeping function called from invalid context in mon_complete syzbot
@ 2025-09-15 1:29 ` Lizhi Xu
2025-09-15 14:19 ` Alan Stern
0 siblings, 1 reply; 8+ messages in thread
From: Lizhi Xu @ 2025-09-15 1:29 UTC (permalink / raw)
To: syzbot+205ef33a3b636b4181fb
Cc: gregkh, linux-kernel, linux-usb, syzkaller-bugs
Interrupts are disabled before entering usb_hcd_giveback_urb().
A spinlock_t becomes a sleeping lock on PREEMPT_RT, so it cannot be
acquired with disabled interrupts.
Make mon_bus::lock a raw spinlock so it can be used with interrupts disabled.
syz reported:
BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
CPU: 1 UID: 0 PID: 45 Comm: kworker/1:1 Not tainted syzkaller #0 PREEMPT_{RT,(full)}
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/12/2025
Workqueue: usb_hub_wq hub_event
Call Trace:
<TASK>
dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
__might_resched+0x44b/0x5d0 kernel/sched/core.c:8957
rt_spin_lock+0xc7/0x2c0 kernel/locking/spinlock_rt.c:57
spin_lock include/linux/spinlock_rt.h:44 [inline]
mon_bus_complete drivers/usb/mon/mon_main.c:134 [inline]
mon_complete+0x5c/0x200 drivers/usb/mon/mon_main.c:147
usbmon_urb_complete include/linux/usb/hcd.h:738 [inline]
__usb_hcd_giveback_urb+0x254/0x5e0 drivers/usb/core/hcd.c:1647
vhci_urb_enqueue+0xb4f/0xe70 drivers/usb/usbip/vhci_hcd.c:818
Reported-by: syzbot+205ef33a3b636b4181fb@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=205ef33a3b636b4181fb
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
drivers/usb/mon/mon_main.c | 24 +++++++++---------------
drivers/usb/mon/mon_text.c | 6 +++---
drivers/usb/mon/usb_mon.h | 2 +-
3 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/drivers/usb/mon/mon_main.c b/drivers/usb/mon/mon_main.c
index af852d53aac6..83d19b769d84 100644
--- a/drivers/usb/mon/mon_main.c
+++ b/drivers/usb/mon/mon_main.c
@@ -38,7 +38,7 @@ void mon_reader_add(struct mon_bus *mbus, struct mon_reader *r)
unsigned long flags;
struct list_head *p;
- spin_lock_irqsave(&mbus->lock, flags);
+ raw_spin_lock_irqsave(&mbus->lock, flags);
if (mbus->nreaders == 0) {
if (mbus == &mon_bus0) {
list_for_each (p, &mon_buses) {
@@ -52,7 +52,7 @@ void mon_reader_add(struct mon_bus *mbus, struct mon_reader *r)
}
mbus->nreaders++;
list_add_tail(&r->r_link, &mbus->r_list);
- spin_unlock_irqrestore(&mbus->lock, flags);
+ raw_spin_unlock_irqrestore(&mbus->lock, flags);
kref_get(&mbus->ref);
}
@@ -66,12 +66,12 @@ void mon_reader_del(struct mon_bus *mbus, struct mon_reader *r)
{
unsigned long flags;
- spin_lock_irqsave(&mbus->lock, flags);
+ raw_spin_lock_irqsave(&mbus->lock, flags);
list_del(&r->r_link);
--mbus->nreaders;
if (mbus->nreaders == 0)
mon_stop(mbus);
- spin_unlock_irqrestore(&mbus->lock, flags);
+ raw_spin_unlock_irqrestore(&mbus->lock, flags);
kref_put(&mbus->ref, mon_bus_drop);
}
@@ -80,14 +80,12 @@ void mon_reader_del(struct mon_bus *mbus, struct mon_reader *r)
*/
static void mon_bus_submit(struct mon_bus *mbus, struct urb *urb)
{
- unsigned long flags;
struct mon_reader *r;
- spin_lock_irqsave(&mbus->lock, flags);
+ guard(raw_spinlock_irqsave)(&mbus->lock);
mbus->cnt_events++;
list_for_each_entry(r, &mbus->r_list, r_link)
r->rnf_submit(r->r_data, urb);
- spin_unlock_irqrestore(&mbus->lock, flags);
}
static void mon_submit(struct usb_bus *ubus, struct urb *urb)
@@ -104,14 +102,12 @@ static void mon_submit(struct usb_bus *ubus, struct urb *urb)
*/
static void mon_bus_submit_error(struct mon_bus *mbus, struct urb *urb, int error)
{
- unsigned long flags;
struct mon_reader *r;
- spin_lock_irqsave(&mbus->lock, flags);
+ guard(raw_spinlock_irqsave)(&mbus->lock);
mbus->cnt_events++;
list_for_each_entry(r, &mbus->r_list, r_link)
r->rnf_error(r->r_data, urb, error);
- spin_unlock_irqrestore(&mbus->lock, flags);
}
static void mon_submit_error(struct usb_bus *ubus, struct urb *urb, int error)
@@ -128,14 +124,12 @@ static void mon_submit_error(struct usb_bus *ubus, struct urb *urb, int error)
*/
static void mon_bus_complete(struct mon_bus *mbus, struct urb *urb, int status)
{
- unsigned long flags;
struct mon_reader *r;
- spin_lock_irqsave(&mbus->lock, flags);
+ guard(raw_spinlock_irqsave)(&mbus->lock);
mbus->cnt_events++;
list_for_each_entry(r, &mbus->r_list, r_link)
r->rnf_complete(r->r_data, urb, status);
- spin_unlock_irqrestore(&mbus->lock, flags);
}
static void mon_complete(struct usb_bus *ubus, struct urb *urb, int status)
@@ -277,7 +271,7 @@ static void mon_bus_init(struct usb_bus *ubus)
if (mbus == NULL)
goto err_alloc;
kref_init(&mbus->ref);
- spin_lock_init(&mbus->lock);
+ raw_spin_lock_init(&mbus->lock);
INIT_LIST_HEAD(&mbus->r_list);
/*
@@ -304,7 +298,7 @@ static void mon_bus0_init(void)
struct mon_bus *mbus = &mon_bus0;
kref_init(&mbus->ref);
- spin_lock_init(&mbus->lock);
+ raw_spin_lock_init(&mbus->lock);
INIT_LIST_HEAD(&mbus->r_list);
mbus->text_inited = mon_text_add(mbus, NULL);
diff --git a/drivers/usb/mon/mon_text.c b/drivers/usb/mon/mon_text.c
index 68b9b2b41189..b482c610dad1 100644
--- a/drivers/usb/mon/mon_text.c
+++ b/drivers/usb/mon/mon_text.c
@@ -307,15 +307,15 @@ static struct mon_event_text *mon_text_fetch(struct mon_reader_text *rp,
struct list_head *p;
unsigned long flags;
- spin_lock_irqsave(&mbus->lock, flags);
+ raw_spin_lock_irqsave(&mbus->lock, flags);
if (list_empty(&rp->e_list)) {
- spin_unlock_irqrestore(&mbus->lock, flags);
+ raw_spin_unlock_irqrestore(&mbus->lock, flags);
return NULL;
}
p = rp->e_list.next;
list_del(p);
--rp->nevents;
- spin_unlock_irqrestore(&mbus->lock, flags);
+ raw_spin_unlock_irqrestore(&mbus->lock, flags);
return list_entry(p, struct mon_event_text, e_link);
}
diff --git a/drivers/usb/mon/usb_mon.h b/drivers/usb/mon/usb_mon.h
index aa64efaba366..d2a342feaad3 100644
--- a/drivers/usb/mon/usb_mon.h
+++ b/drivers/usb/mon/usb_mon.h
@@ -17,7 +17,7 @@
struct mon_bus {
struct list_head bus_link;
- spinlock_t lock;
+ raw_spinlock_t lock;
struct usb_bus *u_bus;
int text_inited;
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] usb: mon: Make mon_bus::lock a raw spinlock
2025-09-15 1:29 ` [PATCH] usb: mon: Make mon_bus::lock a raw spinlock Lizhi Xu
@ 2025-09-15 14:19 ` Alan Stern
2025-09-16 1:41 ` [PATCH V2] usbip: Fix locking bug in RT-enabled kernels Lizhi Xu
0 siblings, 1 reply; 8+ messages in thread
From: Alan Stern @ 2025-09-15 14:19 UTC (permalink / raw)
To: Lizhi Xu
Cc: syzbot+205ef33a3b636b4181fb, gregkh, linux-kernel, linux-usb,
syzkaller-bugs
On Mon, Sep 15, 2025 at 09:29:13AM +0800, Lizhi Xu wrote:
> Interrupts are disabled before entering usb_hcd_giveback_urb().
This needs to be fixed in the usbip driver. There is no need to change
usbmon.
> A spinlock_t becomes a sleeping lock on PREEMPT_RT, so it cannot be
> acquired with disabled interrupts.
>
> Make mon_bus::lock a raw spinlock so it can be used with interrupts disabled.
See commit 8d63c83d8eb9 ("USB: gadget: dummy-hcd: Fix locking bug in
RT-enabled kernels") for an example of how to avoid disabling
interrupts before calling usb_hcd_giveback_urb().
Alan Stern
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH V2] usbip: Fix locking bug in RT-enabled kernels
2025-09-15 14:19 ` Alan Stern
@ 2025-09-16 1:41 ` Lizhi Xu
2025-09-16 15:47 ` Alan Stern
0 siblings, 1 reply; 8+ messages in thread
From: Lizhi Xu @ 2025-09-16 1:41 UTC (permalink / raw)
To: stern
Cc: gregkh, linux-kernel, linux-usb, lizhi.xu,
syzbot+205ef33a3b636b4181fb, syzkaller-bugs
Interrupts are disabled before entering usb_hcd_giveback_urb().
A spinlock_t becomes a sleeping lock on PREEMPT_RT, so it cannot be
acquired with disabled interrupts.
Save the interrupt status and restore it after usb_hcd_giveback_urb().
syz reported:
BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
Call Trace:
dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
rt_spin_lock+0xc7/0x2c0 kernel/locking/spinlock_rt.c:57
spin_lock include/linux/spinlock_rt.h:44 [inline]
mon_bus_complete drivers/usb/mon/mon_main.c:134 [inline]
mon_complete+0x5c/0x200 drivers/usb/mon/mon_main.c:147
usbmon_urb_complete include/linux/usb/hcd.h:738 [inline]
__usb_hcd_giveback_urb+0x254/0x5e0 drivers/usb/core/hcd.c:1647
vhci_urb_enqueue+0xb4f/0xe70 drivers/usb/usbip/vhci_hcd.c:818
Reported-by: syzbot+205ef33a3b636b4181fb@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=205ef33a3b636b4181fb
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
V1 -> V2: fix it in usbip
drivers/usb/usbip/vhci_hcd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index e70fba9f55d6..eb6de7e8ea7b 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -809,15 +809,15 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
no_need_xmit:
usb_hcd_unlink_urb_from_ep(hcd, urb);
no_need_unlink:
- spin_unlock_irqrestore(&vhci->lock, flags);
if (!ret) {
/* usb_hcd_giveback_urb() should be called with
* irqs disabled
*/
- local_irq_disable();
+ spin_unlock(&vhci->lock);
usb_hcd_giveback_urb(hcd, urb, urb->status);
- local_irq_enable();
+ spin_lock(&vhci->lock);
}
+ spin_unlock_irqrestore(&vhci->lock, flags);
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2] usbip: Fix locking bug in RT-enabled kernels
2025-09-16 1:41 ` [PATCH V2] usbip: Fix locking bug in RT-enabled kernels Lizhi Xu
@ 2025-09-16 15:47 ` Alan Stern
2025-09-16 23:33 ` Shuah Khan
0 siblings, 1 reply; 8+ messages in thread
From: Alan Stern @ 2025-09-16 15:47 UTC (permalink / raw)
To: Lizhi Xu, Valentina Manea, Shuah Khan
Cc: gregkh, linux-kernel, linux-usb, syzbot+205ef33a3b636b4181fb,
syzkaller-bugs
On Tue, Sep 16, 2025 at 09:41:43AM +0800, Lizhi Xu wrote:
> Interrupts are disabled before entering usb_hcd_giveback_urb().
> A spinlock_t becomes a sleeping lock on PREEMPT_RT, so it cannot be
> acquired with disabled interrupts.
>
> Save the interrupt status and restore it after usb_hcd_giveback_urb().
>
> syz reported:
> BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
> Call Trace:
> dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
> rt_spin_lock+0xc7/0x2c0 kernel/locking/spinlock_rt.c:57
> spin_lock include/linux/spinlock_rt.h:44 [inline]
> mon_bus_complete drivers/usb/mon/mon_main.c:134 [inline]
> mon_complete+0x5c/0x200 drivers/usb/mon/mon_main.c:147
> usbmon_urb_complete include/linux/usb/hcd.h:738 [inline]
> __usb_hcd_giveback_urb+0x254/0x5e0 drivers/usb/core/hcd.c:1647
> vhci_urb_enqueue+0xb4f/0xe70 drivers/usb/usbip/vhci_hcd.c:818
>
> Reported-by: syzbot+205ef33a3b636b4181fb@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=205ef33a3b636b4181fb
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
> V1 -> V2: fix it in usbip
>
> drivers/usb/usbip/vhci_hcd.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> index e70fba9f55d6..eb6de7e8ea7b 100644
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
> @@ -809,15 +809,15 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
> no_need_xmit:
> usb_hcd_unlink_urb_from_ep(hcd, urb);
> no_need_unlink:
> - spin_unlock_irqrestore(&vhci->lock, flags);
> if (!ret) {
> /* usb_hcd_giveback_urb() should be called with
> * irqs disabled
> */
> - local_irq_disable();
> + spin_unlock(&vhci->lock);
> usb_hcd_giveback_urb(hcd, urb, urb->status);
> - local_irq_enable();
> + spin_lock(&vhci->lock);
> }
> + spin_unlock_irqrestore(&vhci->lock, flags);
> return ret;
> }
This looks right to me; it's the same pattern that the other host
controller drivers use. However, the final decision is up to the usbip
maintainers.
Also, there are several other places in the usbip drivers where
usb_hcd_giveback_urb() gets called; shouldn't they all be changed to
follow this pattern?
Alan Stern
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] usbip: Fix locking bug in RT-enabled kernels
2025-09-16 15:47 ` Alan Stern
@ 2025-09-16 23:33 ` Shuah Khan
2025-09-17 0:51 ` Lizhi Xu
0 siblings, 1 reply; 8+ messages in thread
From: Shuah Khan @ 2025-09-16 23:33 UTC (permalink / raw)
To: Alan Stern, Lizhi Xu, Valentina Manea, Shuah Khan
Cc: gregkh, linux-kernel, linux-usb, syzbot+205ef33a3b636b4181fb,
syzkaller-bugs, Shuah Khan
On 9/16/25 09:47, Alan Stern wrote:
> On Tue, Sep 16, 2025 at 09:41:43AM +0800, Lizhi Xu wrote:
>> Interrupts are disabled before entering usb_hcd_giveback_urb().
>> A spinlock_t becomes a sleeping lock on PREEMPT_RT, so it cannot be
>> acquired with disabled interrupts.
>>
>> Save the interrupt status and restore it after usb_hcd_giveback_urb().
>>
>> syz reported:
>> BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
>> Call Trace:
>> dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
>> rt_spin_lock+0xc7/0x2c0 kernel/locking/spinlock_rt.c:57
>> spin_lock include/linux/spinlock_rt.h:44 [inline]
>> mon_bus_complete drivers/usb/mon/mon_main.c:134 [inline]
>> mon_complete+0x5c/0x200 drivers/usb/mon/mon_main.c:147
>> usbmon_urb_complete include/linux/usb/hcd.h:738 [inline]
>> __usb_hcd_giveback_urb+0x254/0x5e0 drivers/usb/core/hcd.c:1647
>> vhci_urb_enqueue+0xb4f/0xe70 drivers/usb/usbip/vhci_hcd.c:818
>>
>> Reported-by: syzbot+205ef33a3b636b4181fb@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=205ef33a3b636b4181fb
>> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
>> ---
>> V1 -> V2: fix it in usbip
>>
>> drivers/usb/usbip/vhci_hcd.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
>> index e70fba9f55d6..eb6de7e8ea7b 100644
>> --- a/drivers/usb/usbip/vhci_hcd.c
>> +++ b/drivers/usb/usbip/vhci_hcd.c
>> @@ -809,15 +809,15 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
>> no_need_xmit:
>> usb_hcd_unlink_urb_from_ep(hcd, urb);
>> no_need_unlink:
>> - spin_unlock_irqrestore(&vhci->lock, flags);
>> if (!ret) {
>> /* usb_hcd_giveback_urb() should be called with
>> * irqs disabled
>> */
>> - local_irq_disable();
>> + spin_unlock(&vhci->lock);
>> usb_hcd_giveback_urb(hcd, urb, urb->status);
>> - local_irq_enable();
>> + spin_lock(&vhci->lock);
>> }
>> + spin_unlock_irqrestore(&vhci->lock, flags);
>> return ret;
>> }
>
> This looks right to me; it's the same pattern that the other host
> controller drivers use. However, the final decision is up to the usbip
> maintainers.
>
> Also, there are several other places in the usbip drivers where
> usb_hcd_giveback_urb() gets called; shouldn't they all be changed to
> follow this pattern?
>
Looks good to me.
+1 on changing all other instances - can we do that?
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] usbip: Fix locking bug in RT-enabled kernels
2025-09-16 23:33 ` Shuah Khan
@ 2025-09-17 0:51 ` Lizhi Xu
2025-09-17 16:35 ` Shuah Khan
0 siblings, 1 reply; 8+ messages in thread
From: Lizhi Xu @ 2025-09-17 0:51 UTC (permalink / raw)
To: skhan
Cc: gregkh, linux-kernel, linux-usb, lizhi.xu, shuah, stern,
syzbot+205ef33a3b636b4181fb, syzkaller-bugs, valentina.manea.m
On Tue, 16 Sep 2025 17:33:44 -0600, Shuah Khan wrote:
>On 9/16/25 09:47, Alan Stern wrote:
>> On Tue, Sep 16, 2025 at 09:41:43AM +0800, Lizhi Xu wrote:
>>> Interrupts are disabled before entering usb_hcd_giveback_urb().
>>> A spinlock_t becomes a sleeping lock on PREEMPT_RT, so it cannot be
>>> acquired with disabled interrupts.
>>>
>>> Save the interrupt status and restore it after usb_hcd_giveback_urb().
>>>
>>> syz reported:
>>> BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
>>> Call Trace:
>>> dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
>>> rt_spin_lock+0xc7/0x2c0 kernel/locking/spinlock_rt.c:57
>>> spin_lock include/linux/spinlock_rt.h:44 [inline]
>>> mon_bus_complete drivers/usb/mon/mon_main.c:134 [inline]
>>> mon_complete+0x5c/0x200 drivers/usb/mon/mon_main.c:147
>>> usbmon_urb_complete include/linux/usb/hcd.h:738 [inline]
>>> __usb_hcd_giveback_urb+0x254/0x5e0 drivers/usb/core/hcd.c:1647
>>> vhci_urb_enqueue+0xb4f/0xe70 drivers/usb/usbip/vhci_hcd.c:818
>>>
>>> Reported-by: syzbot+205ef33a3b636b4181fb@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?extid=205ef33a3b636b4181fb
>>> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
>>> ---
>>> V1 -> V2: fix it in usbip
>>>
>>> drivers/usb/usbip/vhci_hcd.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
>>> index e70fba9f55d6..eb6de7e8ea7b 100644
>>> --- a/drivers/usb/usbip/vhci_hcd.c
>>> +++ b/drivers/usb/usbip/vhci_hcd.c
>>> @@ -809,15 +809,15 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
>>> no_need_xmit:
>>> usb_hcd_unlink_urb_from_ep(hcd, urb);
>>> no_need_unlink:
>>> - spin_unlock_irqrestore(&vhci->lock, flags);
>>> if (!ret) {
>>> /* usb_hcd_giveback_urb() should be called with
>>> * irqs disabled
>>> */
>>> - local_irq_disable();
>>> + spin_unlock(&vhci->lock);
>>> usb_hcd_giveback_urb(hcd, urb, urb->status);
>>> - local_irq_enable();
>>> + spin_lock(&vhci->lock);
>>> }
>>> + spin_unlock_irqrestore(&vhci->lock, flags);
>>> return ret;
>>> }
>>
>> This looks right to me; it's the same pattern that the other host
>> controller drivers use. However, the final decision is up to the usbip
>> maintainers.
>>
>> Also, there are several other places in the usbip drivers where
>> usb_hcd_giveback_urb() gets called; shouldn't they all be changed to
>> follow this pattern?
>>
>
>Looks good to me.
>+1 on changing all other instances - can we do that?
I'm replying to both of you. Personally, I suggest this isn't necessary
right now; it's safer to wait until a problem is reported before fixing it.
Also, the context of several other calls to usb_hcd_giveback_urb() in usbip
differs from the current issue. Enabling RT_PREEMPT shouldn't cause similar
issues.
BR,
Lizhi
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] usbip: Fix locking bug in RT-enabled kernels
2025-09-17 0:51 ` Lizhi Xu
@ 2025-09-17 16:35 ` Shuah Khan
0 siblings, 0 replies; 8+ messages in thread
From: Shuah Khan @ 2025-09-17 16:35 UTC (permalink / raw)
To: Lizhi Xu, gregkh
Cc: linux-kernel, linux-usb, shuah, stern,
syzbot+205ef33a3b636b4181fb, syzkaller-bugs, valentina.manea.m,
Shuah Khan
On 9/16/25 18:51, Lizhi Xu wrote:
> On Tue, 16 Sep 2025 17:33:44 -0600, Shuah Khan wrote:
>> On 9/16/25 09:47, Alan Stern wrote:
>>> On Tue, Sep 16, 2025 at 09:41:43AM +0800, Lizhi Xu wrote:
>>>> Interrupts are disabled before entering usb_hcd_giveback_urb().
>>>> A spinlock_t becomes a sleeping lock on PREEMPT_RT, so it cannot be
>>>> acquired with disabled interrupts.
>>>>
>>>> Save the interrupt status and restore it after usb_hcd_giveback_urb().
>>>>
>>>> syz reported:
>>>> BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
>>>> Call Trace:
>>>> dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
>>>> rt_spin_lock+0xc7/0x2c0 kernel/locking/spinlock_rt.c:57
>>>> spin_lock include/linux/spinlock_rt.h:44 [inline]
>>>> mon_bus_complete drivers/usb/mon/mon_main.c:134 [inline]
>>>> mon_complete+0x5c/0x200 drivers/usb/mon/mon_main.c:147
>>>> usbmon_urb_complete include/linux/usb/hcd.h:738 [inline]
>>>> __usb_hcd_giveback_urb+0x254/0x5e0 drivers/usb/core/hcd.c:1647
>>>> vhci_urb_enqueue+0xb4f/0xe70 drivers/usb/usbip/vhci_hcd.c:818
>>>>
>>>> Reported-by: syzbot+205ef33a3b636b4181fb@syzkaller.appspotmail.com
>>>> Closes: https://syzkaller.appspot.com/bug?extid=205ef33a3b636b4181fb
>>>> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
>>>> ---
>>>> V1 -> V2: fix it in usbip
>>>>
>>>> drivers/usb/usbip/vhci_hcd.c | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
>>>> index e70fba9f55d6..eb6de7e8ea7b 100644
>>>> --- a/drivers/usb/usbip/vhci_hcd.c
>>>> +++ b/drivers/usb/usbip/vhci_hcd.c
>>>> @@ -809,15 +809,15 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
>>>> no_need_xmit:
>>>> usb_hcd_unlink_urb_from_ep(hcd, urb);
>>>> no_need_unlink:
>>>> - spin_unlock_irqrestore(&vhci->lock, flags);
>>>> if (!ret) {
>>>> /* usb_hcd_giveback_urb() should be called with
>>>> * irqs disabled
>>>> */
>>>> - local_irq_disable();
>>>> + spin_unlock(&vhci->lock);
>>>> usb_hcd_giveback_urb(hcd, urb, urb->status);
>>>> - local_irq_enable();
>>>> + spin_lock(&vhci->lock);
>>>> }
>>>> + spin_unlock_irqrestore(&vhci->lock, flags);
>>>> return ret;
>>>> }
>>>
>>> This looks right to me; it's the same pattern that the other host
>>> controller drivers use. However, the final decision is up to the usbip
>>> maintainers.
>>>
>>> Also, there are several other places in the usbip drivers where
>>> usb_hcd_giveback_urb() gets called; shouldn't they all be changed to
>>> follow this pattern?
>>>
>>
>> Looks good to me.
>> +1 on changing all other instances - can we do that?
> I'm replying to both of you. Personally, I suggest this isn't necessary
> right now; it's safer to wait until a problem is reported before fixing it.
>
> Also, the context of several other calls to usb_hcd_giveback_urb() in usbip
> differs from the current issue. Enabling RT_PREEMPT shouldn't cause similar
> issues.
Fair enough reasoning to wait. Here is my Acked by.
Acked-by: Shuah Khan <skhan@linuxfoundation.org>
Greg, please pick this up.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-09-17 16:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-13 1:58 [syzbot] [usb?] BUG: sleeping function called from invalid context in mon_complete syzbot
2025-09-15 1:29 ` [PATCH] usb: mon: Make mon_bus::lock a raw spinlock Lizhi Xu
2025-09-15 14:19 ` Alan Stern
2025-09-16 1:41 ` [PATCH V2] usbip: Fix locking bug in RT-enabled kernels Lizhi Xu
2025-09-16 15:47 ` Alan Stern
2025-09-16 23:33 ` Shuah Khan
2025-09-17 0:51 ` Lizhi Xu
2025-09-17 16:35 ` Shuah Khan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox