Netdev List
 help / color / mirror / Atom feed
* [PATCH net v3 0/2] net: usb: cdc_ncm/cdc_mbim: fix netpoll lockup in tx_fixup
@ 2026-10-24 13:23 Nicolai Buchwitz
  2026-10-24 13:23 ` [PATCH net v3 1/2] net: usb: cdc_ncm: don't enable BHs in tx_fixup under netpoll Nicolai Buchwitz
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Nicolai Buchwitz @ 2026-10-24 13:23 UTC (permalink / raw)
  To: Oliver Neukum, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Sjur Brændeland, Alexey Orishko,
	Bjørn Mork, Greg Suarez
  Cc: Breno Leitao, linux-usb, netdev, linux-kernel, Nicolai Buchwitz,
	Mikkel Eskildsen

Mikkel reported lockups with netconsole on a cdc_ncm device. Further
analysis showed that netpoll calls ndo_start_xmit() with IRQs off but
BHs on while holding the txq lock, and the spin_unlock_bh() in the
tx_fixup paths runs pending softirqs. A transmit from one of them then
spins on that txq lock.

Patch 1 fixes cdc_ncm. Mikkel tested the v2 version of it. Patch 2
fixes the same code in cdc_mbim and is untested.

Changes in v3:
- Use plain spin_lock() instead of spin_lock_irqsave() (Jakub)
- Drop Tested-by and Reviewed-by tags since the locking changed
- Link to v2: https://lore.kernel.org/r/20261021-cdc-ncm-netpoll-v2-0-8818965bbb69@tipi-net.de

Changes in v2:
- Reverse xmas in patch 1
- Link to v1: https://lore.kernel.org/r/20261020-cdc-ncm-netpoll-v1-0-682184384b49@tipi-net.de

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
Nicolai Buchwitz (2):
      net: usb: cdc_ncm: don't enable BHs in tx_fixup under netpoll
      net: usb: cdc_mbim: don't enable BHs in tx_fixup under netpoll

 drivers/net/usb/cdc_mbim.c | 5 +++--
 drivers/net/usb/cdc_ncm.c  | 7 +++++--
 2 files changed, 8 insertions(+), 4 deletions(-)
---
base-commit: 044ad3c90d9d2df2c5cb3325ae17b4e9603a311f
change-id: 20261020-cdc-ncm-netpoll-d7a7f69dd2cc

Best regards,
--  
Nicolai Buchwitz <nb@tipi-net.de>


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

* [PATCH net v3 1/2] net: usb: cdc_ncm: don't enable BHs in tx_fixup under netpoll
  2026-10-24 13:23 [PATCH net v3 0/2] net: usb: cdc_ncm/cdc_mbim: fix netpoll lockup in tx_fixup Nicolai Buchwitz
@ 2026-10-24 13:23 ` Nicolai Buchwitz
  2026-10-24 13:23 ` [PATCH net v3 2/2] net: usb: cdc_mbim: " Nicolai Buchwitz
  2026-10-27  1:40 ` [PATCH net v3 0/2] net: usb: cdc_ncm/cdc_mbim: fix netpoll lockup in tx_fixup patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Nicolai Buchwitz @ 2026-10-24 13:23 UTC (permalink / raw)
  To: Oliver Neukum, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Sjur Brændeland, Alexey Orishko,
	Bjørn Mork, Greg Suarez
  Cc: Breno Leitao, linux-usb, netdev, linux-kernel, Nicolai Buchwitz,
	Mikkel Eskildsen

netpoll holds the txq lock and calls ndo_start_xmit() with IRQs off but
BHs on. The spin_unlock_bh() in cdc_ncm_tx_fixup() then runs pending
softirqs. If one of them transmits, it spins on the txq lock that
netpoll holds. With netconsole on a cdc_ncm device this ends in a soft
or hard lockup.

Every caller of cdc_ncm_tx_fixup() holds the txq lock with BHs or IRQs
off, so the softirq user of ctx->mtx cannot interrupt it. Use plain
spin_lock().

Fixes: c84ff1d6dff3 ("cdc_ncm: reduce driver latency in the data path")
Reported-by: Mikkel Eskildsen <mke@visotia.com>
Closes: https://lore.kernel.org/netdev/PRAP189MB185019A5A8D411482BB6E0CDA9982@PRAP189MB1850.EURP189.PROD.OUTLOOK.COM/
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/usb/cdc_ncm.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 35db38cb3e4a2..3bd7cb66f3fc3 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1535,14 +1535,17 @@ cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 	if (ctx == NULL)
 		goto error;
 
-	spin_lock_bh(&ctx->mtx);
+	/* BHs or IRQs are always off here. netpoll only disables IRQs, and
+	 * spin_unlock_bh() would then run softirqs with the txq lock held.
+	 */
+	spin_lock(&ctx->mtx);
 
 	if (ctx->is_ndp16)
 		skb_out = cdc_ncm_fill_tx_frame(dev, skb, cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN));
 	else
 		skb_out = cdc_ncm_fill_tx_frame(dev, skb, cpu_to_le32(USB_CDC_NCM_NDP32_NOCRC_SIGN));
 
-	spin_unlock_bh(&ctx->mtx);
+	spin_unlock(&ctx->mtx);
 	return skb_out;
 
 error:

-- 
2.53.0


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

* [PATCH net v3 2/2] net: usb: cdc_mbim: don't enable BHs in tx_fixup under netpoll
  2026-10-24 13:23 [PATCH net v3 0/2] net: usb: cdc_ncm/cdc_mbim: fix netpoll lockup in tx_fixup Nicolai Buchwitz
  2026-10-24 13:23 ` [PATCH net v3 1/2] net: usb: cdc_ncm: don't enable BHs in tx_fixup under netpoll Nicolai Buchwitz
@ 2026-10-24 13:23 ` Nicolai Buchwitz
  2026-10-27  1:40 ` [PATCH net v3 0/2] net: usb: cdc_ncm/cdc_mbim: fix netpoll lockup in tx_fixup patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Nicolai Buchwitz @ 2026-10-24 13:23 UTC (permalink / raw)
  To: Oliver Neukum, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Sjur Brændeland, Alexey Orishko,
	Bjørn Mork, Greg Suarez
  Cc: Breno Leitao, linux-usb, netdev, linux-kernel, Nicolai Buchwitz

cdc_mbim_tx_fixup() has the same spin_unlock_bh() as cdc_ncm_tx_fixup().
Under netpoll it runs pending softirqs while the txq lock is held, and
a transmit from one of them spins on that lock.

Use plain spin_lock() as in cdc_ncm.

Fixes: 9bf211a30b3f ("net: cdc_mbim: adding MBIM driver")
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/usb/cdc_mbim.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index a7010a0664c7d..8f5734ac35b8c 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -289,9 +289,10 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb
 		}
 	}
 
-	spin_lock_bh(&ctx->mtx);
+	/* BHs or IRQs are always off here, see cdc_ncm_tx_fixup() */
+	spin_lock(&ctx->mtx);
 	skb_out = cdc_ncm_fill_tx_frame(dev, skb, sign);
-	spin_unlock_bh(&ctx->mtx);
+	spin_unlock(&ctx->mtx);
 	return skb_out;
 
 error:

-- 
2.53.0


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

* Re: [PATCH net v3 0/2] net: usb: cdc_ncm/cdc_mbim: fix netpoll lockup in tx_fixup
  2026-10-24 13:23 [PATCH net v3 0/2] net: usb: cdc_ncm/cdc_mbim: fix netpoll lockup in tx_fixup Nicolai Buchwitz
  2026-10-24 13:23 ` [PATCH net v3 1/2] net: usb: cdc_ncm: don't enable BHs in tx_fixup under netpoll Nicolai Buchwitz
  2026-10-24 13:23 ` [PATCH net v3 2/2] net: usb: cdc_mbim: " Nicolai Buchwitz
@ 2026-10-27  1:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-27  1:40 UTC (permalink / raw)
  To: Nicolai Buchwitz
  Cc: oliver, andrew+netdev, davem, edumazet, kuba, pabeni,
	sjur.brandeland, alexey.orishko, bjorn, gsuarez, leitao,
	linux-usb, netdev, linux-kernel, mke

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sat, 24 Oct 2026 15:23:05 +0200 you wrote:
> Mikkel reported lockups with netconsole on a cdc_ncm device. Further
> analysis showed that netpoll calls ndo_start_xmit() with IRQs off but
> BHs on while holding the txq lock, and the spin_unlock_bh() in the
> tx_fixup paths runs pending softirqs. A transmit from one of them then
> spins on that txq lock.
> 
> Patch 1 fixes cdc_ncm. Mikkel tested the v2 version of it. Patch 2
> fixes the same code in cdc_mbim and is untested.
> 
> [...]

Here is the summary with links:
  - [net,v3,1/2] net: usb: cdc_ncm: don't enable BHs in tx_fixup under netpoll
    https://git.kernel.org/netdev/net/c/108ca34cf4b4
  - [net,v3,2/2] net: usb: cdc_mbim: don't enable BHs in tx_fixup under netpoll
    https://git.kernel.org/netdev/net/c/bfd7ced012c7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2026-10-27  1:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-24 13:23 [PATCH net v3 0/2] net: usb: cdc_ncm/cdc_mbim: fix netpoll lockup in tx_fixup Nicolai Buchwitz
2026-10-24 13:23 ` [PATCH net v3 1/2] net: usb: cdc_ncm: don't enable BHs in tx_fixup under netpoll Nicolai Buchwitz
2026-10-24 13:23 ` [PATCH net v3 2/2] net: usb: cdc_mbim: " Nicolai Buchwitz
2026-10-27  1:40 ` [PATCH net v3 0/2] net: usb: cdc_ncm/cdc_mbim: fix netpoll lockup in tx_fixup patchwork-bot+netdevbpf

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