[PATCH net v6] usbnet: fix smp_processor_id() use in preemptible context

[PATCH net v6] usbnet: fix smp_processor_id() use in preemptible context

[Ömer Mete Kaya]

tx_complete() calls this_cpu_ptr() before disabling preemption, which
triggers a BUG when running with CONFIG_DEBUG_PREEMPT:

  BUG: using smp_processor_id() in preemptible code in tx_complete

Fix by using get_cpu_ptr()/put_cpu_ptr() which disable preemption and
return the per-CPU pointer atomically. The usbnet_skb_return() hunk
is a hardening change: that path runs in softirq context so no warning
fires there, but the same fix is applied for consistency.

Fixes: ed194d136769 ("usb: core: remove local_irq_save() around ->complete() handler")
Suggested-by: David Laight <david.laight.linux@gmail.com>
Reported-by: syzbot+04cd90bb99c6ef81a65d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=04cd90bb99c6ef81a65d
Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>

---
v6: Fix commit message and Fixes tag as pointed out by Sashiko:
    - BUG is in tx_complete(), not usbnet_skb_return()
    - usbnet_skb_return() hunk is hardening, not a fix
    - CONFIG_DEBUG_PREEMPT, not PREEMPT_FULL
    - Fixes tag updated to ed194d136769
---
 drivers/net/usb/usbnet.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index a19ecf718f36..84f97f448b2d 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -325,7 +325,7 @@ static void __usbnet_status_stop_force(struct usbnet *dev)
  */
 void usbnet_skb_return(struct usbnet *dev, struct sk_buff *skb)
 {
-	struct pcpu_sw_netstats *stats64 = this_cpu_ptr(dev->net->tstats);
+	struct pcpu_sw_netstats *stats64;
 	unsigned long flags;
 	int	status;

@@ -338,10 +338,12 @@ void usbnet_skb_return(struct usbnet *dev, struct sk_buff *skb)
 	if (skb->protocol == 0)
 		skb->protocol = eth_type_trans(skb, dev->net);

+	stats64 = get_cpu_ptr(dev->net->tstats);
 	flags = u64_stats_update_begin_irqsave(&stats64->syncp);
 	u64_stats_inc(&stats64->rx_packets);
 	u64_stats_add(&stats64->rx_bytes, skb->len);
 	u64_stats_update_end_irqrestore(&stats64->syncp, flags);
+	put_cpu_ptr(dev->net->tstats);

 	netif_dbg(dev, rx_status, dev->net, "< rx, len %zu, type 0x%x\n",
 		  skb->len + sizeof(struct ethhdr), skb->protocol);
@@ -1298,13 +1300,15 @@ static void tx_complete(struct urb *urb)
 	struct usbnet		*dev = entry->dev;

 	if (urb->status == 0) {
-		struct pcpu_sw_netstats *stats64 = this_cpu_ptr(dev->net->tstats);
+		struct pcpu_sw_netstats *stats64;
 		unsigned long flags;

+		stats64 = get_cpu_ptr(dev->net->tstats);
 		flags = u64_stats_update_begin_irqsave(&stats64->syncp);
 		u64_stats_add(&stats64->tx_packets, entry->packets);
 		u64_stats_add(&stats64->tx_bytes, entry->length);
 		u64_stats_update_end_irqrestore(&stats64->syncp, flags);
+		put_cpu_ptr(dev->net->tstats);
 	} else {
 		dev->net->stats.tx_errors++;

--
2.55.0

Re: [PATCH net v6] usbnet: fix smp_processor_id() use in preemptible context

[Jakub Kicinski]

On Mon,  5 Oct 2026 00:59:25 +0300 Ömer Mete Kaya wrote:
> tx_complete() calls this_cpu_ptr() before disabling preemption, which
> triggers a BUG when running with CONFIG_DEBUG_PREEMPT:
> 
>   BUG: using smp_processor_id() in preemptible code in tx_complete
> 
> Fix by using get_cpu_ptr()/put_cpu_ptr() which disable preemption and
> return the per-CPU pointer atomically. The usbnet_skb_return() hunk
> is a hardening change: that path runs in softirq context so no warning
> fires there, but the same fix is applied for consistency.

This looks odd, how did we miss this for 8 years.

Greg is probably busy, but would be good to get a confirmation
from either him or some other USB expert that the callbacks
can indeed be called in process context.

FWIW stack trace from syzbot

 <TASK>
 check_preemption_disabled+0xd8/0xe0 lib/smp_processor_id.c:47
 tx_complete+0x237/0x770 drivers/net/usb/usbnet.c:1301
 __usb_hcd_giveback_urb+0x38d/0x610 drivers/usb/core/hcd.c:1657
 usb_hcd_giveback_urb+0x3ca/0x4a0 drivers/usb/core/hcd.c:1741
 vhci_recv_ret_submit drivers/usb/usbip/vhci_rx.c:107 [inline]
 vhci_rx_pdu drivers/usb/usbip/vhci_rx.c:242 [inline]
 vhci_rx_loop+0x60e/0xa60 drivers/usb/usbip/vhci_rx.c:265
 kthread+0x370/0x450 kernel/kthread.c:436
 ret_from_fork+0x72b/0xd50 arch/x86/kernel/process.c:158

Re: [PATCH net v6] usbnet: fix smp_processor_id() use in preemptible context

[Alan Stern]

On Wed, Oct 07, 2026 at 08:00:49PM -0700, Jakub Kicinski wrote:
> On Mon,  5 Oct 2026 00:59:25 +0300 Ömer Mete Kaya wrote:
> > tx_complete() calls this_cpu_ptr() before disabling preemption, which
> > triggers a BUG when running with CONFIG_DEBUG_PREEMPT:
> > 
> >   BUG: using smp_processor_id() in preemptible code in tx_complete
> > 
> > Fix by using get_cpu_ptr()/put_cpu_ptr() which disable preemption and
> > return the per-CPU pointer atomically. The usbnet_skb_return() hunk
> > is a hardening change: that path runs in softirq context so no warning
> > fires there, but the same fix is applied for consistency.
> 
> This looks odd, how did we miss this for 8 years.
> 
> Greg is probably busy, but would be good to get a confirmation
> from either him or some other USB expert that the callbacks
> can indeed be called in process context.

They can be called in BH context with interrupts enabled.  Is that close 
enough?

Alan Stern

Re: [PATCH net v6] usbnet: fix smp_processor_id() use in preemptible context

[Jakub Kicinski]

On Thu, 8 Oct 2026 10:16:02 -0400 Alan Stern wrote:
> > This looks odd, how did we miss this for 8 years.
> > 
> > Greg is probably busy, but would be good to get a confirmation
> > from either him or some other USB expert that the callbacks
> > can indeed be called in process context.  
> 
> They can be called in BH context with interrupts enabled.  Is that close 
> enough?

BH should be fine on !RT. The patch, AFAIU, is because vhci calls
the completion callbacks in pure, unadulterated process context.
I'm trying to figure out how urgent the fix is, basically.
If it's a vhci bug then the usbnet fix is at most an RT problem.
Also if vhci is doing something wrong we don't want a truckload
of slop patches sent our way to fix 300 drivers :S

Re: [PATCH net v6] usbnet: fix smp_processor_id() use in preemptible context

[Alan Stern]

On Thu, Oct 08, 2026 at 09:43:38AM -0700, Jakub Kicinski wrote:
> On Thu, 8 Oct 2026 10:16:02 -0400 Alan Stern wrote:
> > > This looks odd, how did we miss this for 8 years.
> > > 
> > > Greg is probably busy, but would be good to get a confirmation
> > > from either him or some other USB expert that the callbacks
> > > can indeed be called in process context.  
> > 
> > They can be called in BH context with interrupts enabled.  Is that close 
> > enough?
> 
> BH should be fine on !RT. The patch, AFAIU, is because vhci calls
> the completion callbacks in pure, unadulterated process context.
> I'm trying to figure out how urgent the fix is, basically.
> If it's a vhci bug then the usbnet fix is at most an RT problem.
> Also if vhci is doing something wrong we don't want a truckload
> of slop patches sent our way to fix 300 drivers :S

As far as I am aware, there aren't really any guarantees on the 
context of a USB URB-completion callback.  The kerneldoc for struct urb 
in include/linux/usb.h says "The completion callback is made 
in_interrupt()", but that is most definitely out of date.

Drivers shouldn't rely on any particular context guarantees.  Not even 
whether local irqs are enabled/disabled.

Alan Stern

Re: [PATCH net v6] usbnet: fix smp_processor_id() use in preemptible context

[Greg Kroah-Hartman]

On Thu, Oct 08, 2026 at 02:03:34PM -0400, Alan Stern wrote:
> On Thu, Oct 08, 2026 at 09:43:38AM -0700, Jakub Kicinski wrote:
> > On Thu, 8 Oct 2026 10:16:02 -0400 Alan Stern wrote:
> > > > This looks odd, how did we miss this for 8 years.
> > > > 
> > > > Greg is probably busy, but would be good to get a confirmation
> > > > from either him or some other USB expert that the callbacks
> > > > can indeed be called in process context.  
> > > 
> > > They can be called in BH context with interrupts enabled.  Is that close 
> > > enough?
> > 
> > BH should be fine on !RT. The patch, AFAIU, is because vhci calls
> > the completion callbacks in pure, unadulterated process context.
> > I'm trying to figure out how urgent the fix is, basically.
> > If it's a vhci bug then the usbnet fix is at most an RT problem.
> > Also if vhci is doing something wrong we don't want a truckload
> > of slop patches sent our way to fix 300 drivers :S
> 
> As far as I am aware, there aren't really any guarantees on the 
> context of a USB URB-completion callback.  The kerneldoc for struct urb 
> in include/linux/usb.h says "The completion callback is made 
> in_interrupt()", but that is most definitely out of date.

I don't think so, I think some platforms still have those callbacks in
irq context, unless we changed to threaded irq handlers everywhere?  I
could have missed that, but we should still write the callbacks to
assume that and then we should be fine even if we aren't in irq context,
right?

> Drivers shouldn't rely on any particular context guarantees.  Not even 
> whether local irqs are enabled/disabled.

Agreed.

thanks,

greg k-h