123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170 |
- From fe2b1226656afae56702d1d84c6900f6b67df297 Mon Sep 17 00:00:00 2001
- From: Heiner Kallweit <[email protected]>
- Date: Fri, 1 Dec 2023 11:23:22 +0100
- Subject: [PATCH] leds: trigger: netdev: fix RTNL handling to prevent potential
- deadlock
- When working on LED support for r8169 I got the following lockdep
- warning. Easiest way to prevent this scenario seems to be to take
- the RTNL lock before the trigger_data lock in set_device_name().
- ======================================================
- WARNING: possible circular locking dependency detected
- 6.7.0-rc2-next-20231124+ #2 Not tainted
- ------------------------------------------------------
- bash/383 is trying to acquire lock:
- ffff888103aa1c68 (&trigger_data->lock){+.+.}-{3:3}, at: netdev_trig_notify+0xec/0x190 [ledtrig_netdev]
- but task is already holding lock:
- ffffffff8cddf808 (rtnl_mutex){+.+.}-{3:3}, at: rtnl_lock+0x12/0x20
- which lock already depends on the new lock.
- the existing dependency chain (in reverse order) is:
- -> #1 (rtnl_mutex){+.+.}-{3:3}:
- __mutex_lock+0x9b/0xb50
- mutex_lock_nested+0x16/0x20
- rtnl_lock+0x12/0x20
- set_device_name+0xa9/0x120 [ledtrig_netdev]
- netdev_trig_activate+0x1a1/0x230 [ledtrig_netdev]
- led_trigger_set+0x172/0x2c0
- led_trigger_write+0xf1/0x140
- sysfs_kf_bin_write+0x5d/0x80
- kernfs_fop_write_iter+0x15d/0x210
- vfs_write+0x1f0/0x510
- ksys_write+0x6c/0xf0
- __x64_sys_write+0x14/0x20
- do_syscall_64+0x3f/0xf0
- entry_SYSCALL_64_after_hwframe+0x6c/0x74
- -> #0 (&trigger_data->lock){+.+.}-{3:3}:
- __lock_acquire+0x1459/0x25a0
- lock_acquire+0xc8/0x2d0
- __mutex_lock+0x9b/0xb50
- mutex_lock_nested+0x16/0x20
- netdev_trig_notify+0xec/0x190 [ledtrig_netdev]
- call_netdevice_register_net_notifiers+0x5a/0x100
- register_netdevice_notifier+0x85/0x120
- netdev_trig_activate+0x1d4/0x230 [ledtrig_netdev]
- led_trigger_set+0x172/0x2c0
- led_trigger_write+0xf1/0x140
- sysfs_kf_bin_write+0x5d/0x80
- kernfs_fop_write_iter+0x15d/0x210
- vfs_write+0x1f0/0x510
- ksys_write+0x6c/0xf0
- __x64_sys_write+0x14/0x20
- do_syscall_64+0x3f/0xf0
- entry_SYSCALL_64_after_hwframe+0x6c/0x74
- other info that might help us debug this:
- Possible unsafe locking scenario:
- CPU0 CPU1
- ---- ----
- lock(rtnl_mutex);
- lock(&trigger_data->lock);
- lock(rtnl_mutex);
- lock(&trigger_data->lock);
- *** DEADLOCK ***
- 8 locks held by bash/383:
- #0: ffff888103ff33f0 (sb_writers#3){.+.+}-{0:0}, at: ksys_write+0x6c/0xf0
- #1: ffff888103aa1e88 (&of->mutex){+.+.}-{3:3}, at: kernfs_fop_write_iter+0x114/0x210
- #2: ffff8881036f1890 (kn->active#82){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x11d/0x210
- #3: ffff888108e2c358 (&led_cdev->led_access){+.+.}-{3:3}, at: led_trigger_write+0x30/0x140
- #4: ffffffff8cdd9e10 (triggers_list_lock){++++}-{3:3}, at: led_trigger_write+0x75/0x140
- #5: ffff888108e2c270 (&led_cdev->trigger_lock){++++}-{3:3}, at: led_trigger_write+0xe3/0x140
- #6: ffffffff8cdde3d0 (pernet_ops_rwsem){++++}-{3:3}, at: register_netdevice_notifier+0x1c/0x120
- #7: ffffffff8cddf808 (rtnl_mutex){+.+.}-{3:3}, at: rtnl_lock+0x12/0x20
- stack backtrace:
- CPU: 0 PID: 383 Comm: bash Not tainted 6.7.0-rc2-next-20231124+ #2
- Hardware name: Default string Default string/Default string, BIOS ADLN.M6.SODIMM.ZB.CY.015 08/08/2023
- Call Trace:
- <TASK>
- dump_stack_lvl+0x5c/0xd0
- dump_stack+0x10/0x20
- print_circular_bug+0x2dd/0x410
- check_noncircular+0x131/0x150
- __lock_acquire+0x1459/0x25a0
- lock_acquire+0xc8/0x2d0
- ? netdev_trig_notify+0xec/0x190 [ledtrig_netdev]
- __mutex_lock+0x9b/0xb50
- ? netdev_trig_notify+0xec/0x190 [ledtrig_netdev]
- ? __this_cpu_preempt_check+0x13/0x20
- ? netdev_trig_notify+0xec/0x190 [ledtrig_netdev]
- ? __cancel_work_timer+0x11c/0x1b0
- ? __mutex_lock+0x123/0xb50
- mutex_lock_nested+0x16/0x20
- ? mutex_lock_nested+0x16/0x20
- netdev_trig_notify+0xec/0x190 [ledtrig_netdev]
- call_netdevice_register_net_notifiers+0x5a/0x100
- register_netdevice_notifier+0x85/0x120
- netdev_trig_activate+0x1d4/0x230 [ledtrig_netdev]
- led_trigger_set+0x172/0x2c0
- ? preempt_count_add+0x49/0xc0
- led_trigger_write+0xf1/0x140
- sysfs_kf_bin_write+0x5d/0x80
- kernfs_fop_write_iter+0x15d/0x210
- vfs_write+0x1f0/0x510
- ksys_write+0x6c/0xf0
- __x64_sys_write+0x14/0x20
- do_syscall_64+0x3f/0xf0
- entry_SYSCALL_64_after_hwframe+0x6c/0x74
- RIP: 0033:0x7f269055d034
- Code: c7 00 16 00 00 00 b8 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 80 3d 35 c3 0d 00 00 74 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 48 83 ec 28 48 89 54 24 18 48
- RSP: 002b:00007ffddb7ef748 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
- RAX: ffffffffffffffda RBX: 0000000000000007 RCX: 00007f269055d034
- RDX: 0000000000000007 RSI: 000055bf5f4af3c0 RDI: 0000000000000001
- RBP: 000055bf5f4af3c0 R08: 0000000000000073 R09: 0000000000000001
- R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000007
- R13: 00007f26906325c0 R14: 00007f269062ff20 R15: 0000000000000000
- </TASK>
- Fixes: d5e01266e7f5 ("leds: trigger: netdev: add additional specific link speed mode")
- Cc: [email protected]
- Signed-off-by: Heiner Kallweit <[email protected]>
- Reviewed-by: Andrew Lunn <[email protected]>
- Acked-by: Lee Jones <[email protected]>
- Link: https://lore.kernel.org/r/[email protected]
- Signed-off-by: Jakub Kicinski <[email protected]>
- ---
- drivers/leds/trigger/ledtrig-netdev.c | 11 +++++++----
- 1 file changed, 7 insertions(+), 4 deletions(-)
- --- a/drivers/leds/trigger/ledtrig-netdev.c
- +++ b/drivers/leds/trigger/ledtrig-netdev.c
- @@ -235,6 +235,11 @@ static int set_device_name(struct led_ne
- {
- cancel_delayed_work_sync(&trigger_data->work);
-
- + /*
- + * Take RTNL lock before trigger_data lock to prevent potential
- + * deadlock with netdev notifier registration.
- + */
- + rtnl_lock();
- mutex_lock(&trigger_data->lock);
-
- if (trigger_data->net_dev) {
- @@ -254,16 +259,14 @@ static int set_device_name(struct led_ne
- trigger_data->carrier_link_up = false;
- trigger_data->link_speed = SPEED_UNKNOWN;
- trigger_data->duplex = DUPLEX_UNKNOWN;
- - if (trigger_data->net_dev != NULL) {
- - rtnl_lock();
- + if (trigger_data->net_dev)
- get_device_state(trigger_data);
- - rtnl_unlock();
- - }
-
- trigger_data->last_activity = 0;
-
- set_baseline_state(trigger_data);
- mutex_unlock(&trigger_data->lock);
- + rtnl_unlock();
-
- return 0;
- }
|