When a devlink instance is unregistered the following happens (among
other things):
t0 - The instance is marked with 'DEVLINK_UNREGISTERING'.
t1 - Blocking until an RCU grace period passes.
t2 - The 'DEVLINK_UNREGISTERING' mark is cleared from the instance.
When iterating over devlink instances (f.e., when requesting a dump of
available instances) and encountering an instance that is currently
being unregistered, the current code will loop around until the
'DEVLINK_UNREGISTERING' mark is cleared.
The iteration over devlink instances happens in an RCU critical section,
so if the instance that is currently being unregistered was encountered
between t0 and t1, the system will deadlock and RCU stalls will be
reported [1]. The task unregistering the instance will forever wait for an
RCU grace period to pass and the task iterating over the instances will
forever wait for the mark to be cleared.
The issue can be reliably reproduced by increasing the time window
between t0 and t1 (used a 60 seconds sleep) and running the following
reproducer [2].
Fix by skipping over instances that are currently being unregistered.
[1]
rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
rcu: Tasks blocked on level-0 rcu_node (CPUs 0-7): P344
(detected by 4, t=26002 jiffies, g=5773, q=12 ncpus=8)
task:devlink state:R running task stack:25568 pid:344 ppid:260 flags:0x00004002
[...]
Call Trace:
xa_get_mark+0x184/0x3e0
devlinks_xa_find_get.constprop.0+0xc6/0x2e0
devlink_nl_cmd_get_dumpit+0x105/0x3f0
netlink_dump+0x568/0xff0
__netlink_dump_start+0x651/0x900
genl_family_rcv_msg_dumpit+0x201/0x340
genl_rcv_msg+0x573/0x780
netlink_rcv_skb+0x15f/0x430
genl_rcv+0x29/0x40
netlink_unicast+0x546/0x800
netlink_sendmsg+0x958/0xe60
__sys_sendto+0x3a2/0x480
__x64_sys_sendto+0xe1/0x1b0
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x68/0xd2
[2]
# echo 10 > /sys/bus/netdevsim/new_device
# echo 10 > /sys/bus/netdevsim/del_device &
# devlink dev
Fixes: c2368b19807a ("net: devlink: introduce "unregistering" mark and use it during devlinks iteration")
Reported-by: Vivek Reddy Karri <vkarri@xxxxxxxxxx>
Signed-off-by: Ido Schimmel <idosch@xxxxxxxxxx>
---
I read the stable rules and I am not providing an "upstream commit ID"
since the code in upstream has been reworked, making this fix
irrelevant. The only affected stable kernel is 6.1.y.
---
net/devlink/leftover.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index 032c7af065cd..c6f781a08d06 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -301,6 +301,9 @@ devlinks_xa_find_get(struct net *net, unsigned long *indexp, xa_mark_t filter,
if (!devlink)
goto unlock;
+ /* For a possible retry, the xa_find_after() should be always used */
+ xa_find_fn = xa_find_after;
+
/* In case devlink_unregister() was already called and "unregistering"
* mark was set, do not allow to get a devlink reference here.
* This prevents live-lock of devlink_unregister() wait for completion.
@@ -308,8 +311,6 @@ devlinks_xa_find_get(struct net *net, unsigned long *indexp, xa_mark_t filter,
if (xa_get_mark(&devlinks, *indexp, DEVLINK_UNREGISTERING))
goto retry;
- /* For a possible retry, the xa_find_after() should be always used */
- xa_find_fn = xa_find_after;
if (!devlink_try_get(devlink))
goto retry;
if (!net_eq(devlink_net(devlink), net)) {
--
2.46.1
