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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.