Commit 54659ca
staging: rtl8723bs: remove possible deadlock when disconnect (v2)
when turning off a connection, lockdep complains with the
following warning (a modprobe has been done but the same
happens with a disconnection from NetworkManager,
it's enough to trigger a cfg80211_disconnect call):
[ 682.855867] ======================================================
[ 682.855877] WARNING: possible circular locking dependency detected
[ 682.855887] 5.14.0-rc6+ #16 Tainted: G C OE
[ 682.855898] ------------------------------------------------------
[ 682.855906] modprobe/1770 is trying to acquire lock:
[ 682.855916] ffffb6d000332b00 (&pxmitpriv->lock){+.-.}-{2:2},
at: rtw_free_stainfo+0x52/0x4a0 [r8723bs]
[ 682.856073]
but task is already holding lock:
[ 682.856081] ffffb6d0003336a8 (&pstapriv->sta_hash_lock){+.-.}-{2:2},
at: rtw_free_assoc_resources+0x48/0x110 [r8723bs]
[ 682.856207]
which lock already depends on the new lock.
[ 682.856215]
the existing dependency chain (in reverse order) is:
[ 682.856223]
-> #1 (&pstapriv->sta_hash_lock){+.-.}-{2:2}:
[ 682.856247] _raw_spin_lock_bh+0x34/0x40
[ 682.856265] rtw_get_stainfo+0x9a/0x110 [r8723bs]
[ 682.856389] rtw_xmit_classifier+0x27/0x130 [r8723bs]
[ 682.856515] rtw_xmitframe_enqueue+0xa/0x20 [r8723bs]
[ 682.856642] rtl8723bs_hal_xmit+0x3b/0xb0 [r8723bs]
[ 682.856752] rtw_xmit+0x4ef/0x890 [r8723bs]
[ 682.856879] _rtw_xmit_entry+0xba/0x350 [r8723bs]
[ 682.856981] dev_hard_start_xmit+0xee/0x320
[ 682.856999] sch_direct_xmit+0x8c/0x330
[ 682.857014] __dev_queue_xmit+0xba5/0xf00
[ 682.857030] packet_sendmsg+0x981/0x1b80
[ 682.857047] sock_sendmsg+0x5b/0x60
[ 682.857060] __sys_sendto+0xf1/0x160
[ 682.857073] __x64_sys_sendto+0x24/0x30
[ 682.857087] do_syscall_64+0x3a/0x80
[ 682.857102] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 682.857117]
-> #0 (&pxmitpriv->lock){+.-.}-{2:2}:
[ 682.857142] __lock_acquire+0xfd9/0x1b50
[ 682.857158] lock_acquire+0xb4/0x2c0
[ 682.857172] _raw_spin_lock_bh+0x34/0x40
[ 682.857185] rtw_free_stainfo+0x52/0x4a0 [r8723bs]
[ 682.857308] rtw_free_assoc_resources+0x53/0x110 [r8723bs]
[ 682.857415] cfg80211_rtw_disconnect+0x4b/0x70 [r8723bs]
[ 682.857522] cfg80211_disconnect+0x12e/0x2f0 [cfg80211]
[ 682.857759] cfg80211_leave+0x2b/0x40 [cfg80211]
[ 682.857961] cfg80211_netdev_notifier_call+0xa9/0x560 [cfg80211]
[ 682.858163] raw_notifier_call_chain+0x41/0x50
[ 682.858180] __dev_close_many+0x62/0x100
[ 682.858195] dev_close_many+0x7d/0x120
[ 682.858209] unregister_netdevice_many+0x416/0x680
[ 682.858225] unregister_netdevice_queue+0xab/0xf0
[ 682.858240] unregister_netdev+0x18/0x20
[ 682.858255] rtw_unregister_netdevs+0x28/0x40 [r8723bs]
[ 682.858360] rtw_dev_remove+0x24/0xd0 [r8723bs]
[ 682.858463] sdio_bus_remove+0x31/0xd0 [mmc_core]
[ 682.858532] device_release_driver_internal+0xf7/0x1d0
[ 682.858550] driver_detach+0x47/0x90
[ 682.858564] bus_remove_driver+0x77/0xd0
[ 682.858579] rtw_drv_halt+0xc/0x678 [r8723bs]
[ 682.858685] __x64_sys_delete_module+0x13f/0x250
[ 682.858699] do_syscall_64+0x3a/0x80
[ 682.858715] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 682.858729]
other info that might help us debug this:
[ 682.858737] Possible unsafe locking scenario:
[ 682.858744] CPU0 CPU1
[ 682.858751] ---- ----
[ 682.858758] lock(&pstapriv->sta_hash_lock);
[ 682.858772] lock(&pxmitpriv->lock);
[ 682.858786] lock(&pstapriv->sta_hash_lock);
[ 682.858799] lock(&pxmitpriv->lock);
[ 682.858812]
*** DEADLOCK ***
[ 682.858820] 5 locks held by modprobe/1770:
[ 682.858831] #0: ffff8d870697d980 (&dev->mutex){....}-{3:3},
at: device_release_driver_internal+0x1a/0x1d0
[ 682.858869] #1: ffffffffbdbbf1c8 (rtnl_mutex){+.+.}-{3:3},
at: unregister_netdev+0xe/0x20
[ 682.858906] #2: ffff8d87054ee5e8 (&rdev->wiphy.mtx){+.+.}-{3:3},
at: cfg80211_netdev_notifier_call+0x9e/0x560 [cfg80211]
[ 682.859131] #3: ffff8d870f2bc8f0 (&wdev->mtx){+.+.}-{3:3},
at: cfg80211_leave+0x20/0x40 [cfg80211]
[ 682.859354] #4: ffffb6d0003336a8 (&pstapriv->sta_hash_lock){+.-.}-{2:2},
at: rtw_free_assoc_resources+0x48/0x110 [r8723bs]
[ 682.859482]
stack backtrace:
[ 682.859491] CPU: 1 PID: 1770 Comm: modprobe Tainted: G
C OE 5.14.0-rc6+ #16
[ 682.859507] Hardware name: LENOVO 80NR/Madrid, BIOS DACN25WW 08/20/2015
[ 682.859517] Call Trace:
[ 682.859531] dump_stack_lvl+0x56/0x6f
[ 682.859551] check_noncircular+0xdb/0xf0
[ 682.859579] __lock_acquire+0xfd9/0x1b50
[ 682.859606] lock_acquire+0xb4/0x2c0
[ 682.859623] ? rtw_free_stainfo+0x52/0x4a0 [r8723bs]
[ 682.859752] ? mark_held_locks+0x48/0x70
[ 682.859769] ? rtw_free_stainfo+0x4a/0x4a0 [r8723bs]
[ 682.859898] _raw_spin_lock_bh+0x34/0x40
[ 682.859914] ? rtw_free_stainfo+0x52/0x4a0 [r8723bs]
[ 682.860039] rtw_free_stainfo+0x52/0x4a0 [r8723bs]
[ 682.860171] rtw_free_assoc_resources+0x53/0x110 [r8723bs]
[ 682.860286] cfg80211_rtw_disconnect+0x4b/0x70 [r8723bs]
[ 682.860397] cfg80211_disconnect+0x12e/0x2f0 [cfg80211]
[ 682.860629] cfg80211_leave+0x2b/0x40 [cfg80211]
[ 682.860836] cfg80211_netdev_notifier_call+0xa9/0x560 [cfg80211]
[ 682.861048] ? __lock_acquire+0x4dc/0x1b50
[ 682.861070] ? lock_is_held_type+0xa8/0x110
[ 682.861089] ? lock_is_held_type+0xa8/0x110
[ 682.861104] ? find_held_lock+0x2d/0x90
[ 682.861120] ? packet_notifier+0x173/0x300
[ 682.861141] ? lock_release+0xb3/0x250
[ 682.861160] ? packet_notifier+0x192/0x300
[ 682.861184] raw_notifier_call_chain+0x41/0x50
[ 682.861205] __dev_close_many+0x62/0x100
[ 682.861224] dev_close_many+0x7d/0x120
[ 682.861245] unregister_netdevice_many+0x416/0x680
[ 682.861264] ? find_held_lock+0x2d/0x90
[ 682.861284] unregister_netdevice_queue+0xab/0xf0
[ 682.861306] unregister_netdev+0x18/0x20
[ 682.861325] rtw_unregister_netdevs+0x28/0x40 [r8723bs]
[ 682.861434] rtw_dev_remove+0x24/0xd0 [r8723bs]
[ 682.861542] sdio_bus_remove+0x31/0xd0 [mmc_core]
[ 682.861615] device_release_driver_internal+0xf7/0x1d0
[ 682.861637] driver_detach+0x47/0x90
[ 682.861656] bus_remove_driver+0x77/0xd0
[ 682.861674] rtw_drv_halt+0xc/0x678 [r8723bs]
[ 682.861782] __x64_sys_delete_module+0x13f/0x250
[ 682.861801] ? lockdep_hardirqs_on_prepare+0xf3/0x170
[ 682.861817] ? syscall_enter_from_user_mode+0x20/0x70
[ 682.861836] do_syscall_64+0x3a/0x80
[ 682.861855] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 682.861873] RIP: 0033:0x7f6dbe85400b
[ 682.861890] Code: 73 01 c3 48 8b 0d 6d 1e 0c 00 f7 d8 64 89
01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa
b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 3d
1e 0c 00 f7 d8 64 89 01 48
[ 682.861906] RSP: 002b:00007ffe7a82f538 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
[ 682.861923] RAX: ffffffffffffffda RBX: 000055a64693bd20 RCX: 00007f6dbe85400b
[ 682.861935] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 000055a64693bd88
[ 682.861946] RBP: 000055a64693bd20 R08: 0000000000000000 R09: 0000000000000000
[ 682.861957] R10: 00007f6dbe8c7ac0 R11: 0000000000000206 R12: 000055a64693bd88
[ 682.861967] R13: 0000000000000000 R14: 000055a64693bd88 R15: 00007ffe7a831848
This happens because when we enqueue a frame for
transmission we do it under xmit_priv lock, then calling
rtw_get_stainfo (needed for enqueuing) takes sta_hash_lock
and this leads to the following lock dependency:
xmit_priv->lock -> sta_hash_lock
Turning off a connection will bring to call
rtw_free_assoc_resources which will set up
the inverse dependency:
sta_hash_lock -> xmit_priv_lock
This could lead to a deadlock as lockdep complains.
Fix it by removing the xmit_priv->lock around
rtw_xmitframe_enqueue call inside rtl8723bs_hal_xmit
and put it in a smaller critical section inside
rtw_xmit_classifier, the only place where
xmit_priv data are actually accessed.
Replace spin_{lock,unlock}_bh(pxmitpriv->lock)
in other tx paths leading to rtw_xmitframe_enqueue
call with spin_{lock,unlock}_bh(psta->sleep_q.lock)
- it's not clear why accessing a sleep_q was protected
by a spinlock on xmitpriv->lock.
This way is avoided the same faulty lock nesting
order.
Extra changes in v2 by Hans de Goede:
-Lift the taking of the struct __queue.lock spinlock out of
rtw_free_xmitframe_queue() into the callers this allows also
protecting a bunch of related state in rtw_free_stainfo():
-Protect psta->sleepq_len on rtw_free_xmitframe_queue(&psta->sleep_q);
-Protect struct tx_servq.tx_pending and tx_servq.qcnt when
calling rtw_free_xmitframe_queue(&tx_servq.sta_pending)
-This also allows moving the spin_lock_bh(&pxmitpriv->lock); to below
the sleep_q free-ing code, avoiding another ABBA locking issue
CC: Larry Finger <Larry.Finger@lwfinger.net>
Co-developed-by: Hans de Goede <hdegoede@redhat.com>
Tested-on: Lenovo Ideapad MiiX 300-10IBY
Signed-off-by: Fabio Aiuto <fabioaiuto83@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20210920145502.155454-1-hdegoede@redhat.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>1 parent 7bdedfe commit 54659ca
File tree
5 files changed
+24
-33
lines changed- drivers/staging/rtl8723bs
- core
- hal
5 files changed
+24
-33
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5919 | 5919 | | |
5920 | 5920 | | |
5921 | 5921 | | |
5922 | | - | |
5923 | 5922 | | |
5924 | 5923 | | |
5925 | 5924 | | |
| |||
5930 | 5929 | | |
5931 | 5930 | | |
5932 | 5931 | | |
5933 | | - | |
5934 | | - | |
| 5932 | + | |
5935 | 5933 | | |
5936 | 5934 | | |
5937 | 5935 | | |
| |||
5954 | 5952 | | |
5955 | 5953 | | |
5956 | 5954 | | |
5957 | | - | |
5958 | | - | |
| 5955 | + | |
5959 | 5956 | | |
5960 | 5957 | | |
5961 | 5958 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
957 | 957 | | |
958 | 958 | | |
959 | 959 | | |
960 | | - | |
961 | 960 | | |
962 | | - | |
963 | | - | |
| 961 | + | |
964 | 962 | | |
965 | 963 | | |
966 | 964 | | |
| |||
991 | 989 | | |
992 | 990 | | |
993 | 991 | | |
994 | | - | |
995 | | - | |
| 992 | + | |
996 | 993 | | |
997 | 994 | | |
998 | | - | |
999 | | - | |
| 995 | + | |
1000 | 996 | | |
1001 | 997 | | |
1002 | 998 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
294 | 294 | | |
295 | 295 | | |
296 | 296 | | |
297 | | - | |
298 | | - | |
| 297 | + | |
299 | 298 | | |
300 | 299 | | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
301 | 303 | | |
302 | 304 | | |
303 | | - | |
| 305 | + | |
304 | 306 | | |
305 | 307 | | |
306 | 308 | | |
307 | 309 | | |
308 | 310 | | |
309 | | - | |
| 311 | + | |
310 | 312 | | |
311 | 313 | | |
312 | | - | |
| 314 | + | |
313 | 315 | | |
314 | 316 | | |
315 | 317 | | |
316 | 318 | | |
317 | 319 | | |
318 | | - | |
| 320 | + | |
319 | 321 | | |
320 | 322 | | |
321 | | - | |
| 323 | + | |
322 | 324 | | |
323 | 325 | | |
324 | 326 | | |
325 | 327 | | |
326 | 328 | | |
327 | | - | |
| 329 | + | |
328 | 330 | | |
329 | 331 | | |
330 | | - | |
| 332 | + | |
331 | 333 | | |
332 | 334 | | |
333 | 335 | | |
334 | 336 | | |
335 | 337 | | |
336 | | - | |
| 338 | + | |
337 | 339 | | |
338 | 340 | | |
339 | 341 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1734 | 1734 | | |
1735 | 1735 | | |
1736 | 1736 | | |
1737 | | - | |
1738 | | - | |
1739 | 1737 | | |
1740 | 1738 | | |
1741 | 1739 | | |
1742 | 1740 | | |
1743 | 1741 | | |
1744 | 1742 | | |
1745 | | - | |
1746 | 1743 | | |
1747 | 1744 | | |
1748 | 1745 | | |
| |||
1797 | 1794 | | |
1798 | 1795 | | |
1799 | 1796 | | |
| 1797 | + | |
1800 | 1798 | | |
1801 | 1799 | | |
1802 | 1800 | | |
| |||
1814 | 1812 | | |
1815 | 1813 | | |
1816 | 1814 | | |
| 1815 | + | |
1817 | 1816 | | |
1818 | 1817 | | |
1819 | 1818 | | |
1820 | 1819 | | |
1821 | 1820 | | |
1822 | 1821 | | |
| 1822 | + | |
1823 | 1823 | | |
1824 | 1824 | | |
1825 | 1825 | | |
| |||
2202 | 2202 | | |
2203 | 2203 | | |
2204 | 2204 | | |
2205 | | - | |
2206 | 2205 | | |
2207 | 2206 | | |
2208 | 2207 | | |
2209 | | - | |
| 2208 | + | |
2210 | 2209 | | |
2211 | 2210 | | |
2212 | 2211 | | |
| |||
2307 | 2306 | | |
2308 | 2307 | | |
2309 | 2308 | | |
2310 | | - | |
| 2309 | + | |
2311 | 2310 | | |
2312 | 2311 | | |
2313 | 2312 | | |
| |||
2319 | 2318 | | |
2320 | 2319 | | |
2321 | 2320 | | |
2322 | | - | |
2323 | 2321 | | |
2324 | | - | |
| 2322 | + | |
2325 | 2323 | | |
2326 | 2324 | | |
2327 | 2325 | | |
| |||
2374 | 2372 | | |
2375 | 2373 | | |
2376 | 2374 | | |
2377 | | - | |
| 2375 | + | |
2378 | 2376 | | |
2379 | 2377 | | |
2380 | 2378 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
507 | 507 | | |
508 | 508 | | |
509 | 509 | | |
510 | | - | |
511 | 510 | | |
512 | | - | |
513 | 511 | | |
514 | 512 | | |
515 | 513 | | |
| |||
0 commit comments