public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: Kohei Enju <enjuk@amazon.com>
To: <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<bpf@vger.kernel.org>
Cc: "David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	Kuniyuki Iwashima <kuniyu@amazon.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	"Ahmed Zaki" <ahmed.zaki@intel.com>,
	Stanislav Fomichev <sdf@fomichev.me>,
	"Alexander Lobakin" <aleksander.lobakin@intel.com>,
	Kohei Enju <kohei.enju@gmail.com>, Kohei Enju <enjuk@amazon.com>
Subject: [PATCH net-next v1] dev: remove netdev_lock() and netdev_lock_ops() in register_netdevice().
Date: Sun, 9 Mar 2025 05:37:18 +0900	[thread overview]
Message-ID: <20250308203835.60633-2-enjuk@amazon.com> (raw)

ieee80211_register_hw() takes the wiphy lock, and then
register_netdevice() calls netdev_lock(), since commit 5fda3f35349b ("net:
make netdev_lock() protect netdev->reg_state").
On the other hand, do_setlink() calls netdev_lock() and then
ieee80211_change_mac() takes the wiphy lock, after commit df43d8bf1031
("net: replace dev_addr_sem with netdev instance lock").
This causes the circular locking dependency.

Like netdev_lock(), netdev_lock_ops() introduced in commit 97246d6d21c2
("net: hold netdev instance lock during ndo_bpf") would also cause the
same type of issue.

Both netdev_lock() and netdev_lock_ops() are called before
list_netdevice() in register_netdevice().
No other context can access the struct net_device, so we don't need these
locks in this context.
Remove them.

WARNING: possible circular locking dependency detected
6.14.0-rc5-next-20250307 #44 Not tainted

NetworkManager/8289 is trying to acquire lock:
ffff88810fb30768 (&rdev->wiphy.mtx){+.+.}-{4:4}, at: ieee80211_change_mac+0x110/0x1450

but task is already holding lock:
ffff88810fb48d30 (&dev->lock){+.+.}-{4:4}, at: do_setlink.isra.0+0x326c/0x3f40

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #1 (&dev->lock){+.+.}-{4:4}:
       lock_acquire+0x1b2/0x550
       __mutex_lock+0x1a2/0x14b0
       register_netdevice+0x12dc/0x2000
       cfg80211_register_netdevice+0x149/0x330
       ieee80211_if_add+0xcfe/0x1880
       ieee80211_register_hw+0x3655/0x3f60
       mac80211_hwsim_new_radio+0x2760/0x53a0
       init_mac80211_hwsim+0x6c6/0x7d0
       do_one_initcall+0x11a/0x6d0
       kernel_init_freeable+0x6dd/0x770
       kernel_init+0x1f/0x1e0
       ret_from_fork+0x45/0x80
       ret_from_fork_asm+0x1a/0x30

-> #0 (&rdev->wiphy.mtx){+.+.}-{4:4}:
       check_prev_add+0x1af/0x23e0
       __lock_acquire+0x2d55/0x49a0
       lock_acquire+0x1b2/0x550
       __mutex_lock+0x1a2/0x14b0
       ieee80211_change_mac+0x110/0x1450
       netif_set_mac_address+0x30a/0x4a0
       do_setlink.isra.0+0x77a/0x3f40
       rtnl_newlink+0x11ef/0x2370
       rtnetlink_rcv_msg+0x95b/0xe90
       netlink_rcv_skb+0x16b/0x440
       netlink_unicast+0x532/0x7f0
       netlink_sendmsg+0x8ca/0xda0
       ____sys_sendmsg+0x9cf/0xb60
       ___sys_sendmsg+0x11a/0x1a0
       __sys_sendmsg+0x136/0x1e0
       do_syscall_64+0x74/0x190
       entry_SYSCALL_64_after_hwframe+0x76/0x7e

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&dev->lock);
                               lock(&rdev->wiphy.mtx);
                               lock(&dev->lock);
  lock(&rdev->wiphy.mtx);

 *** DEADLOCK ***

Fixes: df43d8bf1031 ("net: replace dev_addr_sem with netdev instance lock")
Signed-off-by: Kohei Enju <enjuk@amazon.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/core/dev.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index c4cc33f73629..df9661961558 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11003,17 +11003,11 @@ int register_netdevice(struct net_device *dev)
 		goto err_ifindex_release;
 
 	ret = netdev_register_kobject(dev);
-
-	netdev_lock(dev);
 	WRITE_ONCE(dev->reg_state, ret ? NETREG_UNREGISTERED : NETREG_REGISTERED);
-	netdev_unlock(dev);
-
 	if (ret)
 		goto err_uninit_notify;
 
-	netdev_lock_ops(dev);
 	__netdev_update_features(dev);
-	netdev_unlock_ops(dev);
 
 	/*
 	 *	Default initial state at registry is that the
-- 
2.48.1


             reply	other threads:[~2025-03-08 20:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-08 20:37 Kohei Enju [this message]
2025-03-08 21:18 ` [PATCH net-next v1] dev: remove netdev_lock() and netdev_lock_ops() in register_netdevice() Jakub Kicinski
2025-03-08 22:41   ` Stanislav Fomichev
2025-03-09 21:41     ` Stanislav Fomichev
2025-03-08 22:41   ` Jakub Kicinski
2025-03-08 22:59     ` Stanislav Fomichev
2025-03-08 23:51       ` Stanislav Fomichev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250308203835.60633-2-enjuk@amazon.com \
    --to=enjuk@amazon.com \
    --cc=ahmed.zaki@intel.com \
    --cc=aleksander.lobakin@intel.com \
    --cc=bigeasy@linutronix.de \
    --cc=bpf@vger.kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kohei.enju@gmail.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@amazon.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox