[syzbot] [net?] possible deadlock in team_del_slave (3)

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 480e035fc4c7 Merge tag 'drm-next-2024-03-13' of https://gi..
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=1662179e180000
kernel config: https://syzkaller.appspot.com/x/.config?x=1e5b814e91787669
dashboard link: https://syzkaller.appspot.com/bug?extid=705c61d60b091ef42c04
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1058e7b9180000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11919365180000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/5f73b6ef963d/disk-480e035f.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/46c949396aad/vmlinux-480e035f.xz
kernel image: https://storage.googleapis.com/syzbot-assets/e3b4d0f5a5f8/bzImage-480e035f.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+705c61...@syzkaller.appspotmail.com

======================================================
WARNING: possible circular locking dependency detected
6.8.0-syzkaller-08073-g480e035fc4c7 #0 Not tainted
------------------------------------------------------
syz-executor419/5074 is trying to acquire lock:
ffff888023dc4d20 (team->team_lock_key){+.+.}-{3:3}, at: team_del_slave+0x32/0x1d0 drivers/net/team/team.c:1988

but task is already holding lock:
ffff88802a210768 (&rdev->wiphy.mtx){+.+.}-{3:3}, at: nl80211_del_interface+0x11a/0x140 net/wireless/nl80211.c:4389

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&rdev->wiphy.mtx){+.+.}-{3:3}:
lock_acquire+0x1e4/0x530 kernel/locking/lockdep.c:5754
__mutex_lock_common kernel/locking/mutex.c:608 [inline]
__mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752
wiphy_lock include/net/cfg80211.h:5951 [inline]
ieee80211_open+0xe7/0x200 net/mac80211/iface.c:449
__dev_open+0x2d3/0x450 net/core/dev.c:1430
dev_open+0xae/0x1b0 net/core/dev.c:1466
team_port_add drivers/net/team/team.c:1214 [inline]
team_add_slave+0x9b3/0x2750 drivers/net/team/team.c:1974
do_set_master net/core/rtnetlink.c:2685 [inline]
do_setlink+0xe70/0x41f0 net/core/rtnetlink.c:2891
rtnl_setlink+0x40d/0x5a0 net/core/rtnetlink.c:3185
rtnetlink_rcv_msg+0x89b/0x10d0 net/core/rtnetlink.c:6595
netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2559
netlink_unicast_kernel net/netlink/af_netlink.c:1335 [inline]
netlink_unicast+0x7ea/0x980 net/netlink/af_netlink.c:1361
netlink_sendmsg+0x8e1/0xcb0 net/netlink/af_netlink.c:1905
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
____sys_sendmsg+0x525/0x7d0 net/socket.c:2584
___sys_sendmsg net/socket.c:2638 [inline]
__sys_sendmsg+0x2b0/0x3a0 net/socket.c:2667
do_syscall_64+0xfb/0x240
entry_SYSCALL_64_after_hwframe+0x6d/0x75

-> #0 (team->team_lock_key){+.+.}-{3:3}:
check_prev_add kernel/locking/lockdep.c:3134 [inline]
check_prevs_add kernel/locking/lockdep.c:3253 [inline]
validate_chain+0x18cb/0x58e0 kernel/locking/lockdep.c:3869
__lock_acquire+0x1346/0x1fd0 kernel/locking/lockdep.c:5137
lock_acquire+0x1e4/0x530 kernel/locking/lockdep.c:5754
__mutex_lock_common kernel/locking/mutex.c:608 [inline]
__mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752
team_del_slave+0x32/0x1d0 drivers/net/team/team.c:1988
team_device_event+0x200/0x5b0 drivers/net/team/team.c:3029
notifier_call_chain+0x18f/0x3b0 kernel/notifier.c:93
call_netdevice_notifiers_extack net/core/dev.c:1988 [inline]
call_netdevice_notifiers net/core/dev.c:2002 [inline]
unregister_netdevice_many_notify+0xd96/0x16d0 net/core/dev.c:11096
unregister_netdevice_many net/core/dev.c:11154 [inline]
unregister_netdevice_queue+0x303/0x370 net/core/dev.c:11033
unregister_netdevice include/linux/netdevice.h:3115 [inline]
_cfg80211_unregister_wdev+0x162/0x560 net/wireless/core.c:1206
ieee80211_if_remove+0x25d/0x3a0 net/mac80211/iface.c:2242
ieee80211_del_iface+0x19/0x30 net/mac80211/cfg.c:202
rdev_del_virtual_intf net/wireless/rdev-ops.h:62 [inline]
cfg80211_remove_virtual_intf+0x230/0x3f0 net/wireless/util.c:2847
genl_family_rcv_msg_doit net/netlink/genetlink.c:1113 [inline]
genl_family_rcv_msg net/netlink/genetlink.c:1193 [inline]
genl_rcv_msg+0xb14/0xec0 net/netlink/genetlink.c:1208
netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2559
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1217
netlink_unicast_kernel net/netlink/af_netlink.c:1335 [inline]
netlink_unicast+0x7ea/0x980 net/netlink/af_netlink.c:1361
netlink_sendmsg+0x8e1/0xcb0 net/netlink/af_netlink.c:1905
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
____sys_sendmsg+0x525/0x7d0 net/socket.c:2584
___sys_sendmsg net/socket.c:2638 [inline]
__sys_sendmsg+0x2b0/0x3a0 net/socket.c:2667
do_syscall_64+0xfb/0x240
entry_SYSCALL_64_after_hwframe+0x6d/0x75

other info that might help us debug this:

Possible unsafe locking scenario:

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

*** DEADLOCK ***

3 locks held by syz-executor419/5074:
#0: ffffffff8f3f1a30 (cb_lock){++++}-{3:3}, at: genl_rcv+0x19/0x40 net/netlink/genetlink.c:1216
#1: ffffffff8f38ce88 (rtnl_mutex){+.+.}-{3:3}, at: nl80211_pre_doit+0x5f/0x8b0 net/wireless/nl80211.c:16401
#2: ffff88802a210768 (&rdev->wiphy.mtx){+.+.}-{3:3}, at: nl80211_del_interface+0x11a/0x140 net/wireless/nl80211.c:4389

stack backtrace:
CPU: 1 PID: 5074 Comm: syz-executor419 Not tainted 6.8.0-syzkaller-08073-g480e035fc4c7 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/29/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
check_noncircular+0x36a/0x4a0 kernel/locking/lockdep.c:2187
check_prev_add kernel/locking/lockdep.c:3134 [inline]
check_prevs_add kernel/locking/lockdep.c:3253 [inline]
validate_chain+0x18cb/0x58e0 kernel/locking/lockdep.c:3869
__lock_acquire+0x1346/0x1fd0 kernel/locking/lockdep.c:5137
lock_acquire+0x1e4/0x530 kernel/locking/lockdep.c:5754
__mutex_lock_common kernel/locking/mutex.c:608 [inline]
__mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752
team_del_slave+0x32/0x1d0 drivers/net/team/team.c:1988
team_device_event+0x200/0x5b0 drivers/net/team/team.c:3029
notifier_call_chain+0x18f/0x3b0 kernel/notifier.c:93
call_netdevice_notifiers_extack net/core/dev.c:1988 [inline]
call_netdevice_notifiers net/core/dev.c:2002 [inline]
unregister_netdevice_many_notify+0xd96/0x16d0 net/core/dev.c:11096
unregister_netdevice_many net/core/dev.c:11154 [inline]
unregister_netdevice_queue+0x303/0x370 net/core/dev.c:11033
unregister_netdevice include/linux/netdevice.h:3115 [inline]
_cfg80211_unregister_wdev+0x162/0x560 net/wireless/core.c:1206
ieee80211_if_remove+0x25d/0x3a0 net/mac80211/iface.c:2242
ieee80211_del_iface+0x19/0x30 net/mac80211/cfg.c:202
rdev_del_virtual_intf net/wireless/rdev-ops.h:62 [inline]
cfg80211_remove_virtual_intf+0x230/0x3f0 net/wireless/util.c:2847
genl_family_rcv_msg_doit net/netlink/genetlink.c:1113 [inline]
genl_family_rcv_msg net/netlink/genetlink.c:1193 [inline]
genl_rcv_msg+0xb14/0xec0 net/netlink/genetlink.c:1208
netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2559
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1217
netlink_unicast_kernel net/netlink/af_netlink.c:1335 [inline]
netlink_unicast+0x7ea/0x980 net/netlink/af_netlink.c:1361
netlink_sendmsg+0x8e1/0xcb0 net/netlink/af_netlink.c:1905
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745
____sys_sendmsg+0x525/0x7d0 net/socket.c:2584
___sys_sendmsg net/socket.c:2638 [inline]
__sys_sendmsg+0x2b0/0x3a0 net/socket.c:2667
do_syscall_64+0xfb/0x240
entry_SYSCALL_64_after_hwframe+0x6d/0x75
RIP: 0033:0x7f963cb981a9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 d1 19 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffdde1419a8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007f963cbe53f6 RCX: 00007f963cb981a9
RDX: 0000000000000000 RSI: 0000000020000400 RDI: 0000000000000004
RBP: 00007f963cc17440 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000031
R13: 0000000000000003 R14: 0000000000050012 R15: 00007ffdde141a02
</TASK>
team0: Port device wlan0 removed


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

[Hillf Danton]

On Fri, 26 Apr 2024 04:59:32 -0700

ASSERT_RTNL();

ASSERT_RTNL();
lockdep_assert_wiphy(sdata->local->hw.wiphy);

Given ASSERT_RTNL() on both sides, difficult to understand the
deadlock reported.

[Jeongjun Park]

#syz test git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
possible deadlock in team_del_slave

bond0 (unregistering): (slave bond_slave_1): Releasing backup interface
bond0 (unregistering): Released all slaves

======================================================
WARNING: possible circular locking dependency detected

6.10.0-rc6-syzkaller-00061-ge9d22f7a6655 #0 Not tainted
------------------------------------------------------
kworker/u8:4/61 is trying to acquire lock:
ffff888023524d20 (team->team_lock_key#4){+.+.}-{3:3}, at: team_del_slave+0x32/0x1d0 drivers/net/team/team_core.c:1990

but task is already holding lock:

ffff8880226b0768 (&rdev->wiphy.mtx){+.+.}-{3:3}, at: wiphy_lock include/net/cfg80211.h:5966 [inline]
ffff8880226b0768 (&rdev->wiphy.mtx){+.+.}-{3:3}, at: ieee80211_remove_interfaces+0x12b/0x700 net/mac80211/iface.c:2280

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&rdev->wiphy.mtx){+.+.}-{3:3}:

lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5754

__mutex_lock_common kernel/locking/mutex.c:608 [inline]
__mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752

wiphy_lock include/net/cfg80211.h:5966 [inline]
ieee80211_open+0xe7/0x200 net/mac80211/iface.c:449
__dev_open+0x2d3/0x450 net/core/dev.c:1472
dev_open+0xae/0x1b0 net/core/dev.c:1508
team_port_add drivers/net/team/team_core.c:1216 [inline]
team_add_slave+0x9b3/0x2750 drivers/net/team/team_core.c:1976
do_set_master net/core/rtnetlink.c:2701 [inline]
do_setlink+0xe70/0x41f0 net/core/rtnetlink.c:2907
__rtnl_newlink net/core/rtnetlink.c:3696 [inline]
rtnl_newlink+0x180b/0x20a0 net/core/rtnetlink.c:3743
rtnetlink_rcv_msg+0x89b/0x1180 net/core/rtnetlink.c:6635
netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2564

netlink_unicast_kernel net/netlink/af_netlink.c:1335 [inline]
netlink_unicast+0x7ea/0x980 net/netlink/af_netlink.c:1361

netlink_sendmsg+0x8db/0xcb0 net/netlink/af_netlink.c:1905

sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0x221/0x270 net/socket.c:745

____sys_sendmsg+0x525/0x7d0 net/socket.c:2585
___sys_sendmsg net/socket.c:2639 [inline]
__sys_sendmsg+0x2b0/0x3a0 net/socket.c:2668
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f

-> #0 (team->team_lock_key#4){+.+.}-{3:3}:

check_prev_add kernel/locking/lockdep.c:3134 [inline]
check_prevs_add kernel/locking/lockdep.c:3253 [inline]

validate_chain+0x18e0/0x5900 kernel/locking/lockdep.c:3869
__lock_acquire+0x1346/0x1fd0 kernel/locking/lockdep.c:5137
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5754

__mutex_lock_common kernel/locking/mutex.c:608 [inline]
__mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752

team_del_slave+0x32/0x1d0 drivers/net/team/team_core.c:1990
team_device_event+0x200/0x5b0 drivers/net/team/team_core.c:2984
notifier_call_chain+0x19f/0x3e0 kernel/notifier.c:93
call_netdevice_notifiers_extack net/core/dev.c:2030 [inline]
call_netdevice_notifiers net/core/dev.c:2044 [inline]
unregister_netdevice_many_notify+0xd75/0x16b0 net/core/dev.c:11219
unregister_netdevice_many net/core/dev.c:11277 [inline]
unregister_netdevice_queue+0x303/0x370 net/core/dev.c:11156
unregister_netdevice include/linux/netdevice.h:3119 [inline]
_cfg80211_unregister_wdev+0x162/0x560 net/wireless/core.c:1206
ieee80211_remove_interfaces+0x4db/0x700 net/mac80211/iface.c:2305
ieee80211_unregister_hw+0x5d/0x2c0 net/mac80211/main.c:1658
mac80211_hwsim_del_radio+0x2c2/0x4c0 drivers/net/wireless/virtual/mac80211_hwsim.c:5576
hwsim_exit_net+0x5c1/0x670 drivers/net/wireless/virtual/mac80211_hwsim.c:6453
ops_exit_list net/core/net_namespace.c:173 [inline]
cleanup_net+0x802/0xcc0 net/core/net_namespace.c:640
process_one_work kernel/workqueue.c:3248 [inline]
process_scheduled_works+0xa2c/0x1830 kernel/workqueue.c:3329
worker_thread+0x86d/0xd50 kernel/workqueue.c:3409
kthread+0x2f0/0x390 kernel/kthread.c:389
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244

other info that might help us debug this:

Possible unsafe locking scenario:

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

lock(team->team_lock_key#4);
lock(&rdev->wiphy.mtx);
lock(team->team_lock_key#4);

*** DEADLOCK ***

5 locks held by kworker/u8:4/61:
#0: ffff888015ed5948 ((wq_completion)netns){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3223 [inline]
#0: ffff888015ed5948 ((wq_completion)netns){+.+.}-{0:0}, at: process_scheduled_works+0x90a/0x1830 kernel/workqueue.c:3329
#1: ffffc900015c7d00 (net_cleanup_work){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3224 [inline]
#1: ffffc900015c7d00 (net_cleanup_work){+.+.}-{0:0}, at: process_scheduled_works+0x945/0x1830 kernel/workqueue.c:3329
#2: ffffffff8f5da690 (pernet_ops_rwsem){++++}-{3:3}, at: cleanup_net+0x16a/0xcc0 net/core/net_namespace.c:594
#3: ffffffff8f5e6ec8 (rtnl_mutex){+.+.}-{3:3}, at: ieee80211_unregister_hw+0x55/0x2c0 net/mac80211/main.c:1651
#4: ffff8880226b0768 (&rdev->wiphy.mtx){+.+.}-{3:3}, at: wiphy_lock include/net/cfg80211.h:5966 [inline]
#4: ffff8880226b0768 (&rdev->wiphy.mtx){+.+.}-{3:3}, at: ieee80211_remove_interfaces+0x12b/0x700 net/mac80211/iface.c:2280

stack backtrace:
CPU: 0 PID: 61 Comm: kworker/u8:4 Not tainted 6.10.0-rc6-syzkaller-00061-ge9d22f7a6655 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/07/2024
Workqueue: netns cleanup_net

Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
check_noncircular+0x36a/0x4a0 kernel/locking/lockdep.c:2187
check_prev_add kernel/locking/lockdep.c:3134 [inline]
check_prevs_add kernel/locking/lockdep.c:3253 [inline]

validate_chain+0x18e0/0x5900 kernel/locking/lockdep.c:3869
__lock_acquire+0x1346/0x1fd0 kernel/locking/lockdep.c:5137
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5754

__mutex_lock_common kernel/locking/mutex.c:608 [inline]
__mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752

team_del_slave+0x32/0x1d0 drivers/net/team/team_core.c:1990
team_device_event+0x200/0x5b0 drivers/net/team/team_core.c:2984
notifier_call_chain+0x19f/0x3e0 kernel/notifier.c:93
call_netdevice_notifiers_extack net/core/dev.c:2030 [inline]
call_netdevice_notifiers net/core/dev.c:2044 [inline]
unregister_netdevice_many_notify+0xd75/0x16b0 net/core/dev.c:11219
unregister_netdevice_many net/core/dev.c:11277 [inline]
unregister_netdevice_queue+0x303/0x370 net/core/dev.c:11156
unregister_netdevice include/linux/netdevice.h:3119 [inline]
_cfg80211_unregister_wdev+0x162/0x560 net/wireless/core.c:1206
ieee80211_remove_interfaces+0x4db/0x700 net/mac80211/iface.c:2305
ieee80211_unregister_hw+0x5d/0x2c0 net/mac80211/main.c:1658
mac80211_hwsim_del_radio+0x2c2/0x4c0 drivers/net/wireless/virtual/mac80211_hwsim.c:5576
hwsim_exit_net+0x5c1/0x670 drivers/net/wireless/virtual/mac80211_hwsim.c:6453
ops_exit_list net/core/net_namespace.c:173 [inline]
cleanup_net+0x802/0xcc0 net/core/net_namespace.c:640
process_one_work kernel/workqueue.c:3248 [inline]
process_scheduled_works+0xa2c/0x1830 kernel/workqueue.c:3329
worker_thread+0x86d/0xd50 kernel/workqueue.c:3409
kthread+0x2f0/0x390 kernel/kthread.c:389
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
</TASK>
team0: Port device wlan1 removed
hsr_slave_0: left promiscuous mode
hsr_slave_1: left promiscuous mode
batman_adv: batadv0: Interface deactivated: batadv_slave_0
batman_adv: batadv0: Removing interface: batadv_slave_0
batman_adv: batadv0: Interface deactivated: batadv_slave_1
batman_adv: batadv0: Removing interface: batadv_slave_1
veth1_macvtap: left promiscuous mode
veth0_macvtap: left promiscuous mode
veth1_vlan: left promiscuous mode
veth0_vlan: left promiscuous mode
team0 (unregistering): Port device team_slave_1 removed
team0 (unregistering): Port device team_slave_0 removed
netdevsim netdevsim4 netdevsim3 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
netdevsim netdevsim4 netdevsim2 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
netdevsim netdevsim4 netdevsim1 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
netdevsim netdevsim4 netdevsim0 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
netdevsim netdevsim3 netdevsim3 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
netdevsim netdevsim3 netdevsim2 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
netdevsim netdevsim3 netdevsim1 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
netdevsim netdevsim3 netdevsim0 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
netdevsim netdevsim2 netdevsim3 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
netdevsim netdevsim2 netdevsim2 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
netdevsim netdevsim2 netdevsim1 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
netdevsim netdevsim2 netdevsim0 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
bridge_slave_1: left allmulticast mode
bridge_slave_1: left promiscuous mode
bridge0: port 2(bridge_slave_1) entered disabled state
bridge_slave_0: left allmulticast mode
bridge_slave_0: left promiscuous mode
bridge0: port 1(bridge_slave_0) entered disabled state
bridge_slave_1: left allmulticast mode
bridge_slave_1: left promiscuous mode
bridge0: port 2(bridge_slave_1) entered disabled state
bridge_slave_0: left allmulticast mode
bridge_slave_0: left promiscuous mode
bridge0: port 1(bridge_slave_0) entered disabled state
bridge_slave_1: left allmulticast mode
bridge_slave_1: left promiscuous mode
bridge0: port 2(bridge_slave_1) entered disabled state
bridge_slave_0: left allmulticast mode
bridge_slave_0: left promiscuous mode
bridge0: port 1(bridge_slave_0) entered disabled state
bridge_slave_1: left allmulticast mode
bridge_slave_1: left promiscuous mode
bridge0: port 2(bridge_slave_1) entered disabled state
bridge_slave_0: left allmulticast mode
bridge_slave_0: left promiscuous mode
bridge0: port 1(bridge_slave_0) entered disabled state
bond0 (unregistering): (slave bond_slave_0): Releasing backup interface
bond0 (unregistering): (slave bond_slave_1): Releasing backup interface
bond0 (unregistering): Released all slaves
bond0 (unregistering): (slave bond_slave_0): Releasing backup interface
bond0 (unregistering): (slave bond_slave_1): Releasing backup interface
bond0 (unregistering): Released all slaves
bond0 (unregistering): (slave bond_slave_0): Releasing backup interface
bond0 (unregistering): (slave bond_slave_1): Releasing backup interface
bond0 (unregistering): Released all slaves
bond0 (unregistering): (slave bond_slave_0): Releasing backup interface
bond0 (unregistering): (slave bond_slave_1): Releasing backup interface
bond0 (unregistering): Released all slaves
team0: Port device wlan1 removed
team0: Port device wlan1 removed
team0: Port device wlan1 removed
team0: Port device wlan1 removed
hsr_slave_0: left promiscuous mode
hsr_slave_1: left promiscuous mode
batman_adv: batadv0: Interface deactivated: batadv_slave_0
batman_adv: batadv0: Removing interface: batadv_slave_0
batman_adv: batadv0: Interface deactivated: batadv_slave_1
batman_adv: batadv0: Removing interface: batadv_slave_1
hsr_slave_0: left promiscuous mode
hsr_slave_1: left promiscuous mode
batman_adv: batadv0: Interface deactivated: batadv_slave_0
batman_adv: batadv0: Removing interface: batadv_slave_0
batman_adv: batadv0: Interface deactivated: batadv_slave_1
batman_adv: batadv0: Removing interface: batadv_slave_1
hsr_slave_0: left promiscuous mode
hsr_slave_1: left promiscuous mode
batman_adv: batadv0: Interface deactivated: batadv_slave_0
batman_adv: batadv0: Removing interface: batadv_slave_0
batman_adv: batadv0: Interface deactivated: batadv_slave_1
batman_adv: batadv0: Removing interface: batadv_slave_1
hsr_slave_0: left promiscuous mode
hsr_slave_1: left promiscuous mode
batman_adv: batadv0: Interface deactivated: batadv_slave_0
batman_adv: batadv0: Removing interface: batadv_slave_0
batman_adv: batadv0: Interface deactivated: batadv_slave_1
batman_adv: batadv0: Removing interface: batadv_slave_1
veth1_macvtap: left promiscuous mode
veth0_macvtap: left promiscuous mode
veth1_vlan: left promiscuous mode
veth0_vlan: left promiscuous mode
veth1_macvtap: left promiscuous mode
veth0_macvtap: left promiscuous mode
veth1_vlan: left promiscuous mode
veth0_vlan: left promiscuous mode
veth1_macvtap: left promiscuous mode
veth0_macvtap: left promiscuous mode
veth1_vlan: left promiscuous mode
veth0_vlan: left promiscuous mode
veth1_macvtap: left promiscuous mode
veth0_macvtap: left promiscuous mode
veth1_vlan: left promiscuous mode
veth0_vlan: left promiscuous mode
team0 (unregistering): Port device team_slave_1 removed
team0 (unregistering): Port device team_slave_0 removed
team0 (unregistering): Port device team_slave_1 removed
team0 (unregistering): Port device team_slave_0 removed
team0 (unregistering): Port device team_slave_1 removed
team0 (unregistering): Port device team_slave_0 removed
team0 (unregistering): Port device team_slave_1 removed
team0 (unregistering): Port device team_slave_0 removed


Tested on:

commit: e9d22f7a Merge tag 'linux_kselftest-fixes-6.10-rc7' of..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14efde81980000
kernel config: https://syzkaller.appspot.com/x/.config?x=864caee5f78cab51

dashboard link: https://syzkaller.appspot.com/bug?extid=705c61d60b091ef42c04
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40

Note: no patches were applied.

[Jeongjun Park]

#syz test git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

---
drivers/net/team/team_core.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index ab1935a4aa2c..3ac82df876b0 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -1970,11 +1970,12 @@ static int team_add_slave(struct net_device *dev, struct net_device *port_dev,
struct netlink_ext_ack *extack)
{
struct team *team = netdev_priv(dev);
- int err;
+ int err, locked;

- mutex_lock(&team->lock);
+ locked = mutex_trylock(&team->lock);
err = team_port_add(team, port_dev, extack);
- mutex_unlock(&team->lock);
+ if (locked)
+ mutex_unlock(&team->lock);

if (!err)
netdev_change_features(dev);
@@ -1985,11 +1986,12 @@ static int team_add_slave(struct net_device *dev, struct net_device *port_dev,
static int team_del_slave(struct net_device *dev, struct net_device *port_dev)
{
struct team *team = netdev_priv(dev);
- int err;
+ int err, locked;

- mutex_lock(&team->lock);
+ locked = mutex_trylock(&team->lock);
err = team_port_del(team, port_dev);
- mutex_unlock(&team->lock);
+ if (locked)
+ mutex_unlock(&team->lock);

if (err)
return err;
--

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+705c61...@syzkaller.appspotmail.com

Tested on:

commit: e9d22f7a Merge tag 'linux_kselftest-fixes-6.10-rc7' of..
git tree: upstream

console output: https://syzkaller.appspot.com/x/log.txt?x=16dbf4e1980000

kernel config: https://syzkaller.appspot.com/x/.config?x=864caee5f78cab51
dashboard link: https://syzkaller.appspot.com/bug?extid=705c61d60b091ef42c04
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40

patch: https://syzkaller.appspot.com/x/patch.diff?x=16fc5485980000

Note: testing is done by a robot and is best-effort only.

[Jeongjun Park]

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

lock(team->team_lock_key#4);
lock(&rdev->wiphy.mtx);
lock(team->team_lock_key#4);

Deadlock occurs due to the above scenario. Therefore,
modify the code as shown in the patch below to prevent deadlock.

Regards,
Jeongjun Park.

Reported-and-tested-by: syzbot+705c61...@syzkaller.appspotmail.com
Fixes: 61dc3461b954 ("team: convert overall spinlock to mutex")
Signed-off-by: Jeongjun Park <aha3...@gmail.com>

[Jeongjun Park]

#syz test git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

---

drivers/net/team/team_core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index ab1935a4aa2c..43d7c73b25aa 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -1972,7 +1972,8 @@ static int team_add_slave(struct net_device *dev, struct net_device *port_dev,
struct team *team = netdev_priv(dev);
int err;

- mutex_lock(&team->lock);
+ if (!mutex_trylock(&team->lock))
+ return -EBUSY;

err = team_port_add(team, port_dev, extack);

mutex_unlock(&team->lock);

@@ -1987,7 +1988,8 @@ static int team_del_slave(struct net_device *dev, struct net_device *port_dev)

struct team *team = netdev_priv(dev);

int err;

- mutex_lock(&team->lock);
+ if (!mutex_trylock(&team->lock))
+ return -EBUSY;
err = team_port_del(team, port_dev);
mutex_unlock(&team->lock);

--

[Jeongjun Park]

>
> On Wed, Jul 03, 2024 at 11:51:59PM +0900, Jeongjun Park wrote:
> >        CPU0                    CPU1
> >        ----                    ----
> >   lock(&rdev->wiphy.mtx);
> >                                lock(team->team_lock_key#4);
> >                                lock(&rdev->wiphy.mtx);
> >   lock(team->team_lock_key#4);
> >
> > Deadlock occurs due to the above scenario. Therefore,
> > modify the code as shown in the patch below to prevent deadlock.
> >
> > Regards,
> > Jeongjun Park.
>

> The commit message should contain the patch description only (without
> salutations, etc.).

>
> >
> > Reported-and-tested-by: syzbot+705c61...@syzkaller.appspotmail.com
> > Fixes: 61dc3461b954 ("team: convert overall spinlock to mutex")
> > Signed-off-by: Jeongjun Park <aha3...@gmail.com>
> > ---
> >  drivers/net/team/team_core.c | 14 ++++++++------

> >  1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
> > index ab1935a4aa2c..3ac82df876b0 100644
> > --- a/drivers/net/team/team_core.c
> > +++ b/drivers/net/team/team_core.c
> > @@ -1970,11 +1970,12 @@ static int team_add_slave(struct net_device *dev, struct net_device *port_dev,
> >                           struct netlink_ext_ack *extack)
> >  {

> >         struct team *team = netdev_priv(dev);

> > -       int err;
> > +       int err, locked;
> >
> > -       mutex_lock(&team->lock);
> > +       locked = mutex_trylock(&team->lock);

> >         err = team_port_add(team, port_dev, extack);

> > -       mutex_unlock(&team->lock);
> > +       if (locked)
> > +               mutex_unlock(&team->lock);
>

> This is not correct usage of 'mutex_trylock()' API. In such a case you
> could as well remove the lock completely from that part of code.
> If "mutex_trylock()" returns false it means the mutex cannot be taken
> (because it was already taken by other thread), so you should not modify
> the resources that were expected to be protected by the mutex.
> In other words, there is a risk of modifying resources using
> "team_port_add()" by several threads at a time.
>
> >
> >         if (!err)
> >                 netdev_change_features(dev);
> > @@ -1985,11 +1986,12 @@ static int team_add_slave(struct net_device *dev, struct net_device *port_dev,

> >  static int team_del_slave(struct net_device *dev, struct net_device *port_dev)
> >  {
> >         struct team *team = netdev_priv(dev);

> > -       int err;
> > +       int err, locked;
> >
> > -       mutex_lock(&team->lock);
> > +       locked = mutex_trylock(&team->lock);
> >         err = team_port_del(team, port_dev);
> > -       mutex_unlock(&team->lock);
> > +       if (locked)
> > +               mutex_unlock(&team->lock);
>

> The same story as in case of "team_add_slave()".

>
> >
> >         if (err)
> >                 return err;
> > --
> >
>

> The patch does not seem to be a correct solution to remove a deadlock.
> Most probably a synchronization design needs an inspection.
> If you really want to use "mutex_trylock()" API, please consider several
> attempts of taking the mutex, but never modify the protected resources when
> the mutex is not taken successfully.
>

Thanks for your comment. I rewrote the patch based on those comments.
This time, we modified it to return an error so that resources are not
modified when a race situation occurs. We would appreciate your
feedback on what this patch would be like.

> Thanks,
> Michal
>
>

Regards,
Jeongjun Park

[Eric Dumazet]

Failing team_del_slave() is not an option. It will add various issues.

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+705c61...@syzkaller.appspotmail.com

Tested on:

commit: e9d22f7a Merge tag 'linux_kselftest-fixes-6.10-rc7' of..
git tree: upstream

console output: https://syzkaller.appspot.com/x/log.txt?x=14125485980000

kernel config: https://syzkaller.appspot.com/x/.config?x=864caee5f78cab51
dashboard link: https://syzkaller.appspot.com/bug?extid=705c61d60b091ef42c04
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40

patch: https://syzkaller.appspot.com/x/patch.diff?x=1489b399980000

[Jiri Pirko]

I wonder, since we already rely on rtnl in lots of team code, perhaps we
can remove team->lock completely and convert the rest of the code to be
protected by rtnl lock as well.

[Jeongjun Park]

>
> On Wed, Jul 03, 2024 at 11:51:59PM +0900, Jeongjun Park wrote:

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

[Jeongjun Park]

#syz test git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

---

drivers/net/team/team_core.c | 32 +++++++++++++++++++++++---------
1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index ab1935a4aa2c..a12366fd420c 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -1142,31 +1142,37 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
char *portname = port_dev->name;
int err;

+ rtnl_lock();
+
if (port_dev->flags & IFF_LOOPBACK) {
NL_SET_ERR_MSG(extack, "Loopback device can't be added as a team port");
netdev_err(dev, "Device %s is loopback device. Loopback devices can't be added as a team port\n",
portname);
- return -EINVAL;
+ err = -EINVAL;
+ goto err_out;
}

if (netif_is_team_port(port_dev)) {
NL_SET_ERR_MSG(extack, "Device is already a port of a team device");
netdev_err(dev, "Device %s is already a port "
"of a team device\n", portname);
- return -EBUSY;
+ err = -EBUSY;
+ goto err_out;
}

if (dev == port_dev) {
NL_SET_ERR_MSG(extack, "Cannot enslave team device to itself");
netdev_err(dev, "Cannot enslave team device to itself\n");
- return -EINVAL;
+ err = -EINVAL;
+ goto err_out;
}

if (netdev_has_upper_dev(dev, port_dev)) {
NL_SET_ERR_MSG(extack, "Device is already an upper device of the team interface");
netdev_err(dev, "Device %s is already an upper device of the team interface\n",
portname);
- return -EBUSY;
+ err = -EBUSY;
+ goto err_out;
}

if (port_dev->features & NETIF_F_VLAN_CHALLENGED &&
@@ -1174,7 +1180,8 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
NL_SET_ERR_MSG(extack, "Device is VLAN challenged and team device has VLAN set up");
netdev_err(dev, "Device %s is VLAN challenged and team device has VLAN set up\n",
portname);
- return -EPERM;
+ err = -EPERM;
+ goto err_out;
}

err = team_dev_type_check_change(dev, port_dev);
@@ -1185,13 +1192,16 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
NL_SET_ERR_MSG(extack, "Device is up. Set it down before adding it as a team port");
netdev_err(dev, "Device %s is up. Set it down before adding it as a team port\n",
portname);
- return -EBUSY;
+ err = -EBUSY;
+ goto err_out;
}

port = kzalloc(sizeof(struct team_port) + team->mode->port_priv_size,
GFP_KERNEL);
- if (!port)
- return -ENOMEM;
+ if (!port) {
+ err = -ENOMEM;
+ goto err_out;
+ }

port->dev = port_dev;
port->team = team;
@@ -1213,7 +1223,9 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
goto err_port_enter;
}

+ mutex_unlock(&team->lock);
err = dev_open(port_dev, extack);
+ mutex_lock(&team->lock);
if (err) {
netdev_dbg(dev, "Device %s opening failed\n",
portname);
@@ -1292,6 +1304,7 @@ static int team_port_add(struct team *team, struct net_device *port_dev,

netdev_info(dev, "Port device %s added\n", portname);

+ rtnl_unlock();
return 0;

err_set_slave_promisc:
@@ -1321,7 +1334,8 @@ static int team_port_add(struct team *team, struct net_device *port_dev,

err_set_mtu:
kfree(port);
-
+err_out:
+ rtnl_unlock();
return err;
}

--

[Jeongjun Park]

#syz test git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

---

drivers/net/team/team_core.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index ab1935a4aa2c..245566a1875d 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -1213,7 +1213,9 @@ static int team_port_add(struct team *team, struct net_device *port_dev,

goto err_port_enter;
}

+ mutex_unlock(&team->lock);
err = dev_open(port_dev, extack);
+ mutex_lock(&team->lock);
if (err) {
netdev_dbg(dev, "Device %s opening failed\n",
portname);

--