linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [BUG] usb: sleepable spinlock used in USB bh_worker softirq on PREEMPT_RT
@ 2025-08-10 15:05 Yunseong Kim
  2025-08-11  9:15 ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 3+ messages in thread
From: Yunseong Kim @ 2025-08-10 15:05 UTC (permalink / raw)
  To: linux-usb
  Cc: linux-rt-users, gregkh, stern, Thomas Gleixner,
	Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
	linux-kernel, syzkaller

While running a PREEMPT_RT-enabled kernel I observed a sleepable
spinlock (rt_spin_lock) being taken from a softirq context within
the USB core framework. On PREEMPT_RT, spin_lock() may sleep when
contended. This is unsafe in softirq context and can cause hangs or
deadlocks.

Below is the relevant call path, reformatted for clarity:

(1) softirq context: workqueue_softirq_action

 --> bh_worker()
     --> process_scheduled_works()
         --> process_one_work()
              --> usb_giveback_urb_bh()
                   --> __usb_hcd_giveback_urb()
                        --> hub_irq()
                             --> hub_resubmit_irq_urb()
                                 --> spin_lock()
                                         |
                                         --> rt_spin_lock()
                                             ** <-- sleepable spinlock
                                                    used in softirq
                                                    context **
Stack trace excerpt:

| BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
| in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 15, name: ksoftirqd/0
| preempt_count: 0, expected: 0
| RCU nest depth: 2, expected: 2
| CPU: 0 UID: 0 PID: 15 Comm: ksoftirqd/0 Not tainted 6.16.0-11245-gc87c2e4c1589 #35 PREEMPT_RT 
| Hardware name: QEMU KVM Virtual Machine, BIOS 2025.02-8ubuntu1 06/11/2025
| Call trace:
|  show_stack+0x2c/0x3c arch/arm64/kernel/stacktrace.c:499 (C)
|  __dump_stack+0x30/0x40 lib/dump_stack.c:94
|  dump_stack_lvl+0x148/0x1d8 lib/dump_stack.c:120
|  dump_stack+0x1c/0x3c lib/dump_stack.c:129
|  __might_resched+0x2e4/0x52c kernel/sched/core.c:8957
|  __rt_spin_lock kernel/locking/spinlock_rt.c:48 [inline]
|  rt_spin_lock+0xa8/0x1bc kernel/locking/spinlock_rt.c:57
|  spin_lock include/linux/spinlock_rt.h:44 [inline]
|  hub_resubmit_irq_urb+0x2c/0x168 drivers/usb/core/hub.c:687
|  hub_irq+0x2ac/0x3d0 drivers/usb/core/hub.c:814
|  __usb_hcd_giveback_urb+0x2f0/0x5e8 drivers/usb/core/hcd.c:1663
|  usb_giveback_urb_bh+0x234/0x3c4 drivers/usb/core/hcd.c:1697
|  process_one_work kernel/workqueue.c:3236 [inline]
|  process_scheduled_works+0x678/0xd18 kernel/workqueue.c:3319
|  bh_worker+0x2f0/0x59c kernel/workqueue.c:3579
|  workqueue_softirq_action+0x104/0x14c kernel/workqueue.c:3606
|  tasklet_hi_action+0x18/0x8c kernel/softirq.c:860
|  handle_softirqs+0x208/0x63c kernel/softirq.c:579
|  run_ksoftirqd+0x64/0x264 kernel/softirq.c:968
|  smpboot_thread_fn+0x4ac/0x908 kernel/smpboot.c:160
|  kthread+0x5e8/0x734 kernel/kthread.c:464
|  ret_from_fork+0x10/0x20 arch/arm64/kernel/entry.S:844


(2) softirq context: run_ksoftirqd()

--> handle_softirqs()
    --> tasklet_action()
        --> workqueue_softirq_action()
            --> bh_worker()
                --> process_scheduled_works()
                    --> process_one_work()
                        --> usb_giveback_urb_bh()
                            --> __usb_hcd_giveback_urb()
                                --> async_completed()
                                    --> spin_lock()
                                         |
                                         --> rt_spin_lock()
                                            ** <-- BUG: sleeping function
                                                    called from invalid
                                                    context **

Stack trace excerpt:

| BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
| in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 15, name: ksoftirqd/0
| preempt_count: 0, expected: 0
| RCU nest depth: 2, expected: 2
| CPU: 0 UID: 0 PID: 15 Comm: ksoftirqd/0 Not tainted 6.16.0-10393-g3267b1c07fff-dirty #43 PREEMPT_RT 
| Hardware name: QEMU KVM Virtual Machine, BIOS 2025.02-8ubuntu1 06/11/2025
| Call trace:
|  show_stack+0x2c/0x3c arch/arm64/kernel/stacktrace.c:499 (C)
|  __dump_stack+0x30/0x40 lib/dump_stack.c:94
|  dump_stack_lvl+0x148/0x1d8 lib/dump_stack.c:120
|  dump_stack+0x1c/0x3c lib/dump_stack.c:129
|  __might_resched+0x2e4/0x52c kernel/sched/core.c:8957
|  __rt_spin_lock kernel/locking/spinlock_rt.c:48 [inline]
|  rt_spin_lock+0xa8/0x1bc kernel/locking/spinlock_rt.c:57
|  spin_lock include/linux/spinlock_rt.h:44 [inline]
|  async_completed+0x70/0xabc drivers/usb/core/devio.c:633
|  __usb_hcd_giveback_urb+0x2f0/0x5e8 drivers/usb/core/hcd.c:1663
|  usb_giveback_urb_bh+0x234/0x3c4 drivers/usb/core/hcd.c:1697
|  process_one_work kernel/workqueue.c:3236 [inline]
|  process_scheduled_works+0x678/0xd18 kernel/workqueue.c:3319
|  bh_worker+0x2f0/0x59c kernel/workqueue.c:3579
|  workqueue_softirq_action+0x104/0x14c kernel/workqueue.c:3606
|  tasklet_action+0x18/0x8c kernel/softirq.c:854
|  handle_softirqs+0x208/0x63c kernel/softirq.c:579
|  run_ksoftirqd+0x64/0x264 kernel/softirq.c:968
|  smpboot_thread_fn+0x4ac/0x908 kernel/smpboot.c:160
|  kthread+0x5e8/0x734 kernel/kthread.c:464
|  ret_from_fork+0x10/0x20 arch/arm64/kernel/entry.S:844

Softirq context is not allowed to sleep. PREEMPT_RT replaces spinlock
implementations with sleepable variants to improve RT performance.
This is fine in process context but unsafe in atomic contexts such as
softirq. This specific instance is in the USB core bh path, so a
framework-level fix seems appropriate.

Because USB activity frequently triggers hub interrupts and URB
givebacks "urb->complete(urb)", this issue can be hit easily on
PREEMPT_RT systems. This means that using USB devices under PREEMPT_RT
may be unstable in its current state, especially under high I/O load or
when multiple devices are active. The likelihood of encountering this
problem increases with frequent plug/unplug events, isochronous transfers,
or heavy USB traffic.

At the moment, I think the following solutions might be possible:
 1. Use raw spinlocks (raw_spin_lock_irqsave/restore) in bh paths.
 2. Move sleepable work into process context (regular workqueues).
 3. Avoid locking entirely in these atomic paths if possible.

I believe this requires a change in the USB core framework rather than
just in individual drivers.

Kernel config, full logs, and reproduction steps can be provided on
request.

This bug was uncovered during my work to fixing KCOV for PREEMPT_RT awareness.
Link: https://lore.kernel.org/all/ee26e7b2-80dd-49b1-bca2-61e460f73c2d@kzalloc.com/t/#u

Best Regards,
Yunseong Kim

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [BUG] usb: sleepable spinlock used in USB bh_worker softirq on PREEMPT_RT
  2025-08-10 15:05 [BUG] usb: sleepable spinlock used in USB bh_worker softirq on PREEMPT_RT Yunseong Kim
@ 2025-08-11  9:15 ` Sebastian Andrzej Siewior
  2025-08-11 15:40   ` Yunseong Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Sebastian Andrzej Siewior @ 2025-08-11  9:15 UTC (permalink / raw)
  To: Yunseong Kim
  Cc: linux-usb, linux-rt-users, gregkh, stern, Thomas Gleixner,
	Clark Williams, Steven Rostedt, linux-kernel, syzkaller

On 2025-08-11 00:05:49 [+0900], Yunseong Kim wrote:
> While running a PREEMPT_RT-enabled kernel I observed a sleepable
> spinlock (rt_spin_lock) being taken from a softirq context within
> the USB core framework. On PREEMPT_RT, spin_lock() may sleep when
> contended. This is unsafe in softirq context and can cause hangs or
> deadlocks.
> 
> I believe this requires a change in the USB core framework rather than
> just in individual drivers.
> 
> Kernel config, full logs, and reproduction steps can be provided on
> request.
> 
> This bug was uncovered during my work to fixing KCOV for PREEMPT_RT awareness.
> Link: https://lore.kernel.org/all/ee26e7b2-80dd-49b1-bca2-61e460f73c2d@kzalloc.com/t/#u

I'm confused. Is this new or this the same bug that was reported by you
in the thread you linked?
The kcov issue should be fixed by
	https://lore.kernel.org/all/20250811082745.ycJqBXMs@linutronix.de/

> Best Regards,
> Yunseong Kim

Sebastian

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [BUG] usb: sleepable spinlock used in USB bh_worker softirq on PREEMPT_RT
  2025-08-11  9:15 ` Sebastian Andrzej Siewior
@ 2025-08-11 15:40   ` Yunseong Kim
  0 siblings, 0 replies; 3+ messages in thread
From: Yunseong Kim @ 2025-08-11 15:40 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-usb, linux-rt-users, gregkh, stern, Thomas Gleixner,
	Clark Williams, Steven Rostedt, linux-kernel, syzkaller

Hi Sebastian,

Thank you for your prompt reply and clarification.

On 8/11/25 6:15 오후, Sebastian Andrzej Siewior wrote:
> On 2025-08-11 00:05:49 [+0900], Yunseong Kim wrote:
>> While running a PREEMPT_RT-enabled kernel I observed a sleepable
>> spinlock (rt_spin_lock) being taken from a softirq context within
>> the USB core framework. On PREEMPT_RT, spin_lock() may sleep when
>> contended. This is unsafe in softirq context and can cause hangs or
>> deadlocks.
>>
> …
>> I believe this requires a change in the USB core framework rather than
>> just in individual drivers.
>>
>> Kernel config, full logs, and reproduction steps can be provided on
>> request.
>>
>> This bug was uncovered during my work to fixing KCOV for PREEMPT_RT awareness.
>> Link: https://lore.kernel.org/all/ee26e7b2-80dd-49b1-bca2-61e460f73c2d@kzalloc.com/t/#u
> 
> I'm confused. Is this new or this the same bug that was reported by you
> in the thread you linked?
> The kcov issue should be fixed by
> 	https://lore.kernel.org/all/20250811082745.ycJqBXMs@linutronix.de/

You are absolutely right.

While PREEMPT_RT does shift most softirq handling to the ksoftirqd process
context where sleeping locks are generally allowed, I overlooked the ironclad
rule: DO NOT SLEEP if interrupts are disabled. Even in ksoftirqd, disabling
interrupts creates an atomic context, and acquiring a sleeping lock while
irqs_disabled() is true can be disastrous in real-time environments.

The bug I reported, specifically the call stack observed where rt_spin_lock was
taken from a softirq context within the USB core framework, was a direct
consequence of this misunderstanding. This issue was uncovered during my work on
fixing KCOV for PREEMPT_RT awareness and was consistently reproducible across my
v1 through v3 patches. For reference, my v3 work can be found here:
 https://lore.kernel.org/lkml/ddd14f62-b6c9-4984-84be-6c999ea92e30@kzalloc.com/t/#u.

I have applied your patch:
 https://lore.kernel.org/all/20250811082745.ycJqBXMs@linutronix.de/

-static inline unsigned long kcov_remote_start_usb_softirq(u64 id)
+static inline void kcov_remote_start_usb_softirq(u64 id)
 {
-	unsigned long flags = 0;
-
-	if (in_serving_softirq()) {
-		local_irq_save(flags);
+	if (in_serving_softirq() && !in_hardirq())
 		kcov_remote_start_usb(id);

!in_hardirq(), I can definitely confirm that the specific call stack and the bug
I reported are no longer reproducible. Your fix has indeed addressed the
underlying issue.

>> Best Regards,
>> Yunseong Kim
> 
> Sebastian


Thank you again for your guidance and for resolving this problem!

Best Regards,
Yunseong Kim



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-08-11 15:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-10 15:05 [BUG] usb: sleepable spinlock used in USB bh_worker softirq on PREEMPT_RT Yunseong Kim
2025-08-11  9:15 ` Sebastian Andrzej Siewior
2025-08-11 15:40   ` Yunseong Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).