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
next 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