[PATCH net v4] usbnet: fix EVENT_RX_PAUSED race in usbnet_skb_return()

[PATCH net v4] usbnet: fix EVENT_RX_PAUSED race in usbnet_skb_return()

[Ömer Mete Kaya]

On SMP, usbnet_skb_return() can observe EVENT_RX_PAUSED as set on CPU0
while usbnet_resume_rx() clears the flag and drains rxq_pause on CPU1.
The skb can then be queued after the drain has completed and remain
stranded until the next MTU change or device stop.

Synchronize the EVENT_RX_PAUSED state transition and paused-queue
dequeue operations with rxq_pause.lock, dropping the lock around each
usbnet_skb_return() call to avoid lock inversion. In usbnet_skb_return()
use a double-check pattern under rxq_pause.lock on the slow path,
keeping the normal RX fast path lock-free. Use spin_lock_irqsave() to
match the lockdep class shared with other sk_buff_head locks taken from
hardirq context in defer_bh().

Discovered by Sashiko, not triggered, not runtime-tested.

Fixes: 43daa96b166c ("usbnet: Stop RX Q on MTU change")
Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>

---
v4: Use spin_lock_irqsave()/spin_unlock_irqrestore() to match lockdep
    class shared with other sk_buff_head locks taken from hardirq
    context in defer_bh(); restore local_bh_disable()/local_bh_enable()
    around drain loop to preserve BH context.

 drivers/net/usb/usbnet.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 84f97f448b2d..9c4a23d8cb86 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -329,9 +329,14 @@ void usbnet_skb_return(struct usbnet *dev, struct sk_buff *skb)
 	unsigned long flags;
 	int	status;

-	if (test_bit(EVENT_RX_PAUSED, &dev->flags)) {
-		skb_queue_tail(&dev->rxq_pause, skb);
-		return;
+	if (unlikely(test_bit(EVENT_RX_PAUSED, &dev->flags))) {
+		spin_lock_irqsave(&dev->rxq_pause.lock, flags);
+		if (test_bit(EVENT_RX_PAUSED, &dev->flags)) {
+			__skb_queue_tail(&dev->rxq_pause, skb);
+			spin_unlock_irqrestore(&dev->rxq_pause.lock, flags);
+			return;
+		}
+		spin_unlock_irqrestore(&dev->rxq_pause.lock, flags);
 	}

 	/* only update if unset to allow minidriver rx_fixup override */
@@ -700,15 +705,18 @@ void usbnet_resume_rx(struct usbnet *dev)
 {
 	struct sk_buff *skb;
 	int num = 0;
+	unsigned long flags;

 	local_bh_disable();
+	spin_lock_irqsave(&dev->rxq_pause.lock, flags);
 	clear_bit(EVENT_RX_PAUSED, &dev->flags);
-
-	while ((skb = skb_dequeue(&dev->rxq_pause)) != NULL) {
+	while ((skb = __skb_dequeue(&dev->rxq_pause)) != NULL) {
+		spin_unlock_irqrestore(&dev->rxq_pause.lock, flags);
 		usbnet_skb_return(dev, skb);
+		spin_lock_irqsave(&dev->rxq_pause.lock, flags);
 		num++;
 	}
-
+	spin_unlock_irqrestore(&dev->rxq_pause.lock, flags);
 	queue_work(system_bh_wq, &dev->bh_work);
 	local_bh_enable();

--
2.56.0

Re: [PATCH net v4] usbnet: fix EVENT_RX_PAUSED race in usbnet_skb_return()

[Simon Horman]

On Tue, Oct 20, 2026 at 06:29:09PM +0300, Ömer Mete Kaya wrote:
> On SMP, usbnet_skb_return() can observe EVENT_RX_PAUSED as set on CPU0
> while usbnet_resume_rx() clears the flag and drains rxq_pause on CPU1.
> The skb can then be queued after the drain has completed and remain
> stranded until the next MTU change or device stop.
> 
> Synchronize the EVENT_RX_PAUSED state transition and paused-queue
> dequeue operations with rxq_pause.lock, dropping the lock around each
> usbnet_skb_return() call to avoid lock inversion. In usbnet_skb_return()
> use a double-check pattern under rxq_pause.lock on the slow path,
> keeping the normal RX fast path lock-free. Use spin_lock_irqsave() to
> match the lockdep class shared with other sk_buff_head locks taken from
> hardirq context in defer_bh().
> 
> Discovered by Sashiko, not triggered, not runtime-tested.
> 
> Fixes: 43daa96b166c ("usbnet: Stop RX Q on MTU change")
> Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>
> 
> ---
> v4: Use spin_lock_irqsave()/spin_unlock_irqrestore() to match lockdep
>     class shared with other sk_buff_head locks taken from hardirq
>     context in defer_bh(); restore local_bh_disable()/local_bh_enable()
>     around drain loop to preserve BH context.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net v4] usbnet: fix EVENT_RX_PAUSED race in usbnet_skb_return()

[Jakub Kicinski]

On Tue, 20 Oct 2026 18:29:09 +0300 Ömer Mete Kaya wrote:
>  	struct sk_buff *skb;
>  	int num = 0;
> +	unsigned long flags;

reverse xmas tree ordering please

>  	local_bh_disable();
> +	spin_lock_irqsave(&dev->rxq_pause.lock, flags);
>  	clear_bit(EVENT_RX_PAUSED, &dev->flags);
> -
> -	while ((skb = skb_dequeue(&dev->rxq_pause)) != NULL) {
> +	while ((skb = __skb_dequeue(&dev->rxq_pause)) != NULL) {
> +		spin_unlock_irqrestore(&dev->rxq_pause.lock, flags);
>  		usbnet_skb_return(dev, skb);
> +		spin_lock_irqsave(&dev->rxq_pause.lock, flags);
>  		num++;
>  	}
> -
> +	spin_unlock_irqrestore(&dev->rxq_pause.lock, flags);

The retaking of the lock in the loop looks ugly / unnecessary to me

          struct sk_buff_head q;
          struct sk_buff *skb;
          unsigned long flags;
          int num = 0;

          __skb_queue_head_init(&q);

          local_bh_disable();
          spin_lock_irqsave(&dev->rxq_pause.lock, flags);
          clear_bit(EVENT_RX_PAUSED, &dev->flags);
          skb_queue_splice_init(&dev->rxq_pause, &q);
          spin_unlock_irqrestore(&dev->rxq_pause.lock, flags);

          while ((skb = __skb_dequeue(&q))) {
                  usbnet_skb_return(dev, skb);
                  num++;
          }
-- 
pw-bot: cr