public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: usb: rtl8150: fix use-after-free in rtl8150_start_xmit()
@ 2026-04-21 11:04 Morduan Zang
  2026-04-21 12:32 ` Andrew Lunn
  2026-04-21 20:05 ` Michal Pecio
  0 siblings, 2 replies; 3+ messages in thread
From: Morduan Zang @ 2026-04-21 11:04 UTC (permalink / raw)
  To: petkan
  Cc: davem, edumazet, kuba, pabeni, andrew+netdev, linux-usb, netdev,
	linux-kernel, syzkaller-bugs, Zhan Jun,
	syzbot+3f46c095ac0ca048cb71

From: Zhan Jun <zhanjun@uniontech.com>

syzbot reported a KASAN slab-use-after-free read in rtl8150_start_xmit()
when accessing skb->len for tx statistics after usb_submit_urb() has
been called:

  BUG: KASAN: slab-use-after-free in rtl8150_start_xmit+0x71f/0x760
    drivers/net/usb/rtl8150.c:712
  Read of size 4 at addr ffff88810eb7a930 by task kworker/0:4/5226

The URB completion handler write_bulk_callback() frees the skb via
dev_kfree_skb_irq(dev->tx_skb). The URB may complete on another CPU
in softirq context before usb_submit_urb() returns in the submitter,
so by the time the submitter reads skb->len the skb has already been
queued to the per-CPU completion_queue and freed by net_tx_action():

  CPU A (xmit)                      CPU B (USB completion softirq)
  ------------                      ------------------------------
  dev->tx_skb = skb;
  usb_submit_urb()      --+
                          |-------> write_bulk_callback()
                          |           dev_kfree_skb_irq(dev->tx_skb)
                          |         net_tx_action()
                          |           napi_skb_cache_put()   <-- free
  netdev->stats.tx_bytes  |
    += skb->len;          <-- UAF read

Fix it by caching skb->len before submitting the URB and using the
cached value when updating the tx_bytes counter. This mirrors the
fix pattern used by other USB network drivers.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+3f46c095ac0ca048cb71@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/69e69ee7.050a0220.24bfd3.002b.GAE@google.com/
Closes: https://syzkaller.appspot.com/bug?extid=3f46c095ac0ca048cb71
Signed-off-by: Zhan Jun <zhanjun@uniontech.com>
---
 drivers/net/usb/rtl8150.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 4cda0643afb6..6fc6be0cced6 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -683,6 +683,7 @@ static netdev_tx_t rtl8150_start_xmit(struct sk_buff *skb,
 					    struct net_device *netdev)
 {
 	rtl8150_t *dev = netdev_priv(netdev);
+	unsigned int skb_len;
 	int count, res;
 
 	/* pad the frame and ensure terminating USB packet, datasheet 9.2.3 */
@@ -694,6 +695,14 @@ static netdev_tx_t rtl8150_start_xmit(struct sk_buff *skb,
 		return NETDEV_TX_OK;
 	}
 
+	/*
+	 * Cache skb->len before submitting the URB: the URB completion
+	 * handler (write_bulk_callback) frees the skb, and it may run
+	 * on another CPU before usb_submit_urb() returns, which would
+	 * leave skb dangling here.
+	 */
+	skb_len = skb->len;
+
 	netif_stop_queue(netdev);
 	dev->tx_skb = skb;
 	usb_fill_bulk_urb(dev->tx_urb, dev->udev, usb_sndbulkpipe(dev->udev, 2),
@@ -709,7 +718,7 @@ static netdev_tx_t rtl8150_start_xmit(struct sk_buff *skb,
 		}
 	} else {
 		netdev->stats.tx_packets++;
-		netdev->stats.tx_bytes += skb->len;
+		netdev->stats.tx_bytes += skb_len;
 		netif_trans_update(netdev);
 	}
 
-- 
2.51.0


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

* Re: [PATCH] net: usb: rtl8150: fix use-after-free in rtl8150_start_xmit()
  2026-04-21 11:04 [PATCH] net: usb: rtl8150: fix use-after-free in rtl8150_start_xmit() Morduan Zang
@ 2026-04-21 12:32 ` Andrew Lunn
  2026-04-21 20:05 ` Michal Pecio
  1 sibling, 0 replies; 3+ messages in thread
From: Andrew Lunn @ 2026-04-21 12:32 UTC (permalink / raw)
  To: Morduan Zang
  Cc: petkan, davem, edumazet, kuba, pabeni, andrew+netdev, linux-usb,
	netdev, linux-kernel, syzkaller-bugs, Zhan Jun,
	syzbot+3f46c095ac0ca048cb71

On Tue, Apr 21, 2026 at 07:04:12PM +0800, Morduan Zang wrote:
> From: Zhan Jun <zhanjun@uniontech.com>
> 
> syzbot reported a KASAN slab-use-after-free read in rtl8150_start_xmit()
> when accessing skb->len for tx statistics after usb_submit_urb() has
> been called:
> 
>   BUG: KASAN: slab-use-after-free in rtl8150_start_xmit+0x71f/0x760
>     drivers/net/usb/rtl8150.c:712
>   Read of size 4 at addr ffff88810eb7a930 by task kworker/0:4/5226
> 
> The URB completion handler write_bulk_callback() frees the skb via
> dev_kfree_skb_irq(dev->tx_skb). The URB may complete on another CPU
> in softirq context before usb_submit_urb() returns in the submitter,
> so by the time the submitter reads skb->len the skb has already been
> queued to the per-CPU completion_queue and freed by net_tx_action():
> 
>   CPU A (xmit)                      CPU B (USB completion softirq)
>   ------------                      ------------------------------
>   dev->tx_skb = skb;
>   usb_submit_urb()      --+
>                           |-------> write_bulk_callback()
>                           |           dev_kfree_skb_irq(dev->tx_skb)
>                           |         net_tx_action()
>                           |           napi_skb_cache_put()   <-- free
>   netdev->stats.tx_bytes  |
>     += skb->len;          <-- UAF read
> 
> Fix it by caching skb->len before submitting the URB and using the
> cached value when updating the tx_bytes counter. This mirrors the
> fix pattern used by other USB network drivers.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+3f46c095ac0ca048cb71@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/69e69ee7.050a0220.24bfd3.002b.GAE@google.com/
> Closes: https://syzkaller.appspot.com/bug?extid=3f46c095ac0ca048cb71
> Signed-off-by: Zhan Jun <zhanjun@uniontech.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

For future patches, please could you set the subject line correctly. See

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

    Andrew

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

* Re: [PATCH] net: usb: rtl8150: fix use-after-free in rtl8150_start_xmit()
  2026-04-21 11:04 [PATCH] net: usb: rtl8150: fix use-after-free in rtl8150_start_xmit() Morduan Zang
  2026-04-21 12:32 ` Andrew Lunn
@ 2026-04-21 20:05 ` Michal Pecio
  1 sibling, 0 replies; 3+ messages in thread
From: Michal Pecio @ 2026-04-21 20:05 UTC (permalink / raw)
  To: Morduan Zang
  Cc: petkan, davem, edumazet, kuba, pabeni, andrew+netdev, linux-usb,
	netdev, linux-kernel, syzkaller-bugs, Zhan Jun,
	syzbot+3f46c095ac0ca048cb71

On Tue, 21 Apr 2026 19:04:12 +0800, Morduan Zang wrote:
> From: Zhan Jun <zhanjun@uniontech.com>
> 
> syzbot reported a KASAN slab-use-after-free read in rtl8150_start_xmit()
> when accessing skb->len for tx statistics after usb_submit_urb() has
> been called:
> 
>   BUG: KASAN: slab-use-after-free in rtl8150_start_xmit+0x71f/0x760
>     drivers/net/usb/rtl8150.c:712
>   Read of size 4 at addr ffff88810eb7a930 by task kworker/0:4/5226
> 
> The URB completion handler write_bulk_callback() frees the skb via
> dev_kfree_skb_irq(dev->tx_skb). The URB may complete on another CPU
> in softirq context before usb_submit_urb() returns in the submitter,
> so by the time the submitter reads skb->len the skb has already been
> queued to the per-CPU completion_queue and freed by net_tx_action():
> 
>   CPU A (xmit)                      CPU B (USB completion softirq)
>   ------------                      ------------------------------
>   dev->tx_skb = skb;
>   usb_submit_urb()      --+
>                           |-------> write_bulk_callback()
>                           |           dev_kfree_skb_irq(dev->tx_skb)
>                           |         net_tx_action()
>                           |           napi_skb_cache_put()   <-- free
>   netdev->stats.tx_bytes  |
>     += skb->len;          <-- UAF read
> 
> Fix it by caching skb->len before submitting the URB and using the
> cached value when updating the tx_bytes counter.

Question:
Is it correct that ETH_ZLEN padding isn't counted in tx_bytes?

> This mirrors the fix pattern used by other USB network drivers.

Which ones? I looked at a few and they either:

- appear to have the same bug (kaweth)
- update stats on URB completion, right before freeing skb
- copy data out of skb, update stats, free skb before URB completion

Regards,
Michal

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

end of thread, other threads:[~2026-04-21 20:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-21 11:04 [PATCH] net: usb: rtl8150: fix use-after-free in rtl8150_start_xmit() Morduan Zang
2026-04-21 12:32 ` Andrew Lunn
2026-04-21 20:05 ` Michal Pecio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox