linux-kernel - Re: [BUG] usbip: vhci: Sleeping function called from invalid context in vhci_urb_enqueue on PREEMPT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <28544110-3021-43da-b32e-9785c81a42c1@kzalloc.com>
Date: Sat, 16 Aug 2025 11:18:02 +0900
From: Yunseong Kim <ysk@...lloc.com>
To: Alan Stern <stern@...land.harvard.edu>
Cc: linux-usb@...r.kernel.org, gregkh@...uxfoundation.org,
 Andrey Konovalov <andreyknvl@...gle.com>,
 Shuah Khan <skhan@...uxfoundation.org>, Thomas Gleixner
 <tglx@...utronix.de>, Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
 Clark Williams <clrkwllms@...nel.org>, Steven Rostedt <rostedt@...dmis.org>,
 linux-rt-users@...r.kernel.org, linux-kernel@...r.kernel.org,
 syzkaller@...glegroups.com
Subject: Re: [BUG] usbip: vhci: Sleeping function called from invalid context
 in vhci_urb_enqueue on PREEMPT_RT

Hi Alan,

On 8/16/25 10:51 AM, Alan Stern wrote:
> On Sat, Aug 16, 2025 at 10:29:34AM +0900, Yunseong Kim wrote:
>> While testing a PREEMPT_RT enabled kernel (based on v6.17.0-rc1),
>> I encountered a "BUG: sleeping function called from invalid context"
>> error originating from the USB/IP VHCI driver.
>>
>> On PREEMPT_RT configurations, standard spin_lock() calls are replaced by
>> rt_spin_lock(). Since rt_spin_lock() may sleep when contended, it must not
>> be called from an atomic context (e.g., with interrupts disabled).
>>
>> The issue occurs within the vhci_urb_enqueue function This function
>> explicitly disables local interrupts using local_irq_disable() immediately
>> before calling usb_hcd_giveback_urb(), adhering to HCD requirements.
> 
> ...
> 
>> This error reported after this work:
>> It occurs after going through the code below:
>>
>>  static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
>>  {
>>  
>>  	...
>>  
>>  no_need_unlink:
>>  	spin_unlock_irqrestore(&vhci->lock, flags);
>>  	if (!ret) {
>>  		/* usb_hcd_giveback_urb() should be called with
>>  		 * irqs disabled
>>  		 */
>>  		local_irq_disable(); // <--- Entering atomic context (IRQs disabled)
>>  		usb_hcd_giveback_urb(hcd, urb, urb->status);
>>  		local_irq_enable();
>>  	}
>>  	return ret;
>>  }
>>
>>  static void mon_bus_complete(struct mon_bus *mbus, struct urb *urb, int status)
>>  {
>>  	...
>>  	spin_lock_irqsave(&mbus->lock, flags);
>                   ^
> ------------------^
> 
>>  	...
>>  }
>>
>> When called with interrupts disabled, usb_hcd_giveback_urb() eventually
>> leads to mon_complete() in the USB monitoring, if usbmon is enabled,
>> via __usb_hcd_giveback_urb().
>>
>> mon_complete() attempts to acquire a lock via spin_lock(), observed in the
>> trace within the inlined mon_bus_complete.
> 
> Look again.  mon_bus_complete() calls spin_lock_irqsave(), not 
> spin_lock().
> 
> Is the kernel tree that you are using different from Linus's tree?
I think this part is a macro, so it appears this way.

Link: https://github.com/torvalds/linux/blob/v6.17-rc1/include/linux/spinlock_rt.h#L96

#define spin_lock_irqsave(lock, flags)			 \
	do {						 \
		typecheck(unsigned long, flags);	 \
		flags = 0;				 \
		spin_lock(lock);			 \
	} while (0)

My tree is indeed 6.17-rc1. I made a mistake in the diagram,
which caused the misunderstanding. I’ve redrawn the diagram:

  kworker (hub_event)
      |
      v
  vhci_urb_enqueue() [drivers/usb/usbip/vhci_hcd.c]
      |
      |---> spin_unlock_irqrestore(&vhci->lock, flags);
      |     (Context: IRQs Enabled, Process Context)
      |---> local_irq_disable();
      |
      |     *** STATE CHANGE: IRQs Disabled (Atomic Context) ***
      |
      +-----> usb_hcd_giveback_urb() [drivers/usb/core/hcd.c]
              |
              v
              __usb_hcd_giveback_urb()
              |
              v
              mon_complete() [drivers/usb/mon/mon_main.c]
              |
              |---> spin_lock_irqsave() [include/linux/spinlock_rt.h]
                    |
                    v https://github.com/torvalds/linux/blob/v6.17-rc1/include/linux/spinlock_rt.h#L96
                    spin_lock() [kernel/locking/spinlock_rt.c] <--- Attempts to acquire lock
                    |
                    | [On PREEMPT_RT]
                    v
                    rt_spin_lock() [kernel/locking/spinlock_rt.c]
                    |
                    v
                    [May Sleep if contended]
                    |
      X <----------- BUG: Sleeping in atomic context (IRQs are disabled!)

      |
      |---> local_irq_enable();
            (Context: IRQs Enabled)


> Alan Stern

Thank you!

Yunseong Kim