[PATCH net-next] eth: bnxt: fix deadlock when xdp is attached or detached

[PATCH net-next] eth: bnxt: fix deadlock when xdp is attached or detached

[Taehee Yoo]

When xdp is attached or detached, dev->ndo_bpf() is called by
do_setlink(), and it acquires netdev_lock() if needed.
Unlike other drivers, the bnxt driver is protected by netdev_lock while
xdp is attached/detached because it sets dev->request_ops_lock to true.

So, the bnxt_xdp(), that is callback of ->ndo_bpf should not acquire
netdev_lock().
But the xdp_features_{set | clear}_redirect_target() was changed to
acquire netdev_lock() internally.
It causes a deadlock.
To fix this problem, bnxt driver should use
xdp_features_{set | clear}_redirect_target_locked() instead.

Splat looks like:
============================================
WARNING: possible recursive locking detected
6.15.0-rc6+ #1 Not tainted
--------------------------------------------
bpftool/1745 is trying to acquire lock:
ffff888131b85038 (&dev->lock){+.+.}-{4:4}, at: xdp_features_set_redirect_target+0x1f/0x80

but task is already holding lock:
ffff888131b85038 (&dev->lock){+.+.}-{4:4}, at: do_setlink.constprop.0+0x24e/0x35d0

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock(&dev->lock);
  lock(&dev->lock);

 *** DEADLOCK ***

 May be due to missing lock nesting notation

3 locks held by bpftool/1745:
 #0: ffffffffa56131c8 (rtnl_mutex){+.+.}-{4:4}, at: rtnl_setlink+0x1fe/0x570
 #1: ffffffffaafa75a0 (&net->rtnl_mutex){+.+.}-{4:4}, at: rtnl_setlink+0x236/0x570
 #2: ffff888131b85038 (&dev->lock){+.+.}-{4:4}, at: do_setlink.constprop.0+0x24e/0x35d0

stack backtrace:
CPU: 1 UID: 0 PID: 1745 Comm: bpftool Not tainted 6.15.0-rc6+ #1 PREEMPT(undef)
Hardware name: ASUS System Product Name/PRIME Z690-P D4, BIOS 0603 11/01/2021
Call Trace:
 <TASK>
 dump_stack_lvl+0x7a/0xd0
 print_deadlock_bug+0x294/0x3d0
 __lock_acquire+0x153b/0x28f0
 lock_acquire+0x184/0x340
 ? xdp_features_set_redirect_target+0x1f/0x80
 __mutex_lock+0x1ac/0x18a0
 ? xdp_features_set_redirect_target+0x1f/0x80
 ? xdp_features_set_redirect_target+0x1f/0x80
 ? __pfx_bnxt_rx_page_skb+0x10/0x10 [bnxt_en
 ? __pfx___mutex_lock+0x10/0x10
 ? __pfx_netdev_update_features+0x10/0x10
 ? bnxt_set_rx_skb_mode+0x284/0x540 [bnxt_en
 ? __pfx_bnxt_set_rx_skb_mode+0x10/0x10 [bnxt_en
 ? xdp_features_set_redirect_target+0x1f/0x80
 xdp_features_set_redirect_target+0x1f/0x80
 bnxt_xdp+0x34e/0x730 [bnxt_en 11cbcce8fa11cff1dddd7ef358d6219e4ca9add3]
 dev_xdp_install+0x3f4/0x830
 ? __pfx_bnxt_xdp+0x10/0x10 [bnxt_en 11cbcce8fa11cff1dddd7ef358d6219e4ca9add3]
 ? __pfx_dev_xdp_install+0x10/0x10
 dev_xdp_attach+0x560/0xf70
 dev_change_xdp_fd+0x22d/0x280
 do_setlink.constprop.0+0x2989/0x35d0
 ? __pfx_do_setlink.constprop.0+0x10/0x10
 ? lock_acquire+0x184/0x340
 ? find_held_lock+0x32/0x90
 ? rtnl_setlink+0x236/0x570
 ? rcu_is_watching+0x11/0xb0
 ? trace_contention_end+0xdc/0x120
 ? __mutex_lock+0x946/0x18a0
 ? __pfx___mutex_lock+0x10/0x10
 ? __lock_acquire+0xa95/0x28f0
 ? rcu_is_watching+0x11/0xb0
 ? rcu_is_watching+0x11/0xb0
 ? cap_capable+0x172/0x350
 rtnl_setlink+0x2cd/0x570

Fixes: 03df156dd3a6 ("xdp: double protect netdev->xdp_flags with netdev->lock")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---

This is a bugfix patch but target branch is net-next because the cause
commit is not yet merged to net.

 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
index e675611777b5..4a6d8cb9f970 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
@@ -425,9 +425,9 @@ static int bnxt_xdp_set(struct bnxt *bp, struct bpf_prog *prog)
 
 	if (prog) {
 		bnxt_set_rx_skb_mode(bp, true);
-		xdp_features_set_redirect_target(dev, true);
+		xdp_features_set_redirect_target_locked(dev, true);
 	} else {
-		xdp_features_clear_redirect_target(dev);
+		xdp_features_clear_redirect_target_locked(dev);
 		bnxt_set_rx_skb_mode(bp, false);
 	}
 	bp->tx_nr_rings_xdp = tx_xdp;
-- 
2.34.1

Re: [PATCH net-next] eth: bnxt: fix deadlock when xdp is attached or detached

[Simon Horman]

On Tue, May 20, 2025 at 07:11:55AM +0000, Taehee Yoo wrote:
> When xdp is attached or detached, dev->ndo_bpf() is called by
> do_setlink(), and it acquires netdev_lock() if needed.
> Unlike other drivers, the bnxt driver is protected by netdev_lock while
> xdp is attached/detached because it sets dev->request_ops_lock to true.
> 
> So, the bnxt_xdp(), that is callback of ->ndo_bpf should not acquire
> netdev_lock().
> But the xdp_features_{set | clear}_redirect_target() was changed to
> acquire netdev_lock() internally.
> It causes a deadlock.
> To fix this problem, bnxt driver should use
> xdp_features_{set | clear}_redirect_target_locked() instead.
> 
> Splat looks like:
> ============================================
> WARNING: possible recursive locking detected
> 6.15.0-rc6+ #1 Not tainted
> --------------------------------------------
> bpftool/1745 is trying to acquire lock:
> ffff888131b85038 (&dev->lock){+.+.}-{4:4}, at: xdp_features_set_redirect_target+0x1f/0x80
> 
> but task is already holding lock:
> ffff888131b85038 (&dev->lock){+.+.}-{4:4}, at: do_setlink.constprop.0+0x24e/0x35d0
> 
> other info that might help us debug this:
>  Possible unsafe locking scenario:
> 
>        CPU0
>        ----
>   lock(&dev->lock);
>   lock(&dev->lock);
> 
>  *** DEADLOCK ***

...

> 
> Fixes: 03df156dd3a6 ("xdp: double protect netdev->xdp_flags with netdev->lock")
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> ---
> 
> This is a bugfix patch but target branch is net-next because the cause
> commit is not yet merged to net.

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

Re: [PATCH net-next] eth: bnxt: fix deadlock when xdp is attached or detached

[Michael Chan]

[-- Attachment #1: Type: text/plain, Size: 754 bytes --]

On Tue, May 20, 2025 at 12:12 AM Taehee Yoo <ap420073@gmail.com> wrote:
>
> When xdp is attached or detached, dev->ndo_bpf() is called by
> do_setlink(), and it acquires netdev_lock() if needed.
> Unlike other drivers, the bnxt driver is protected by netdev_lock while
> xdp is attached/detached because it sets dev->request_ops_lock to true.
>
> So, the bnxt_xdp(), that is callback of ->ndo_bpf should not acquire
> netdev_lock().
> But the xdp_features_{set | clear}_redirect_target() was changed to
> acquire netdev_lock() internally.
> It causes a deadlock.
> To fix this problem, bnxt driver should use
> xdp_features_{set | clear}_redirect_target_locked() instead.

Thanks.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4196 bytes --]

Re: [PATCH net-next] eth: bnxt: fix deadlock when xdp is attached or detached

[patchwork-bot+netdevbpf]

Hello:

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

On Tue, 20 May 2025 07:11:55 +0000 you wrote:
> When xdp is attached or detached, dev->ndo_bpf() is called by
> do_setlink(), and it acquires netdev_lock() if needed.
> Unlike other drivers, the bnxt driver is protected by netdev_lock while
> xdp is attached/detached because it sets dev->request_ops_lock to true.
> 
> So, the bnxt_xdp(), that is callback of ->ndo_bpf should not acquire
> netdev_lock().
> But the xdp_features_{set | clear}_redirect_target() was changed to
> acquire netdev_lock() internally.
> It causes a deadlock.
> To fix this problem, bnxt driver should use
> xdp_features_{set | clear}_redirect_target_locked() instead.
> 
> [...]

Here is the summary with links:
  - [net-next] eth: bnxt: fix deadlock when xdp is attached or detached
    https://git.kernel.org/netdev/net-next/c/db807e5ef8ee

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