linux-kernel - RE: [PATCH rtw v4 4/4] wifi: rtw89: avoid circular locking dependency in ser_state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0b56e5a8cd7048a19625764bc323ba46@realtek.com>
Date: Thu, 18 Sep 2025 05:52:41 +0000
From: Ping-Ke Shih <pkshih@...ltek.com>
To: Fedor Pchelkin <pchelkin@...ras.ru>,
        Zong-Zhe Yang
	<kevin_yang@...ltek.com>
CC: Bitterblue Smith <rtl8821cerfe2@...il.com>,
        Bernie Huang
	<phhuang@...ltek.com>,
        "linux-wireless@...r.kernel.org"
	<linux-wireless@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "lvc-project@...uxtesting.org"
	<lvc-project@...uxtesting.org>
Subject: RE: [PATCH rtw v4 4/4] wifi: rtw89: avoid circular locking dependency in ser_state_run()

Fedor Pchelkin <pchelkin@...ras.ru> wrote:
> Lockdep gives a splat [1] when ser_hdl_work item is executed.  It is
> scheduled at mac80211 workqueue via ieee80211_queue_work() and takes a
> wiphy lock inside.  However, this workqueue can be flushed when e.g.
> closing the interface and wiphy lock is already taken in that case.
> 
> Choosing wiphy_work_queue() for SER is likely not suitable.  Back on to
> the global workqueue.
> 
> [1]:
> 
>  WARNING: possible circular locking dependency detected
>  6.17.0-rc2 #17 Not tainted
>  ------------------------------------------------------
>  kworker/u32:1/61 is trying to acquire lock:
>  ffff88811bc00768 (&rdev->wiphy.mtx){+.+.}-{4:4}, at: ser_state_run+0x5e/0x180 [rtw89_core]
> 
>  but task is already holding lock:
>  ffffc9000048fd30 ((work_completion)(&ser->ser_hdl_work)){+.+.}-{0:0}, at:
> process_one_work+0x7b5/0x1450
> 
>  which lock already depends on the new lock.
> 
>  the existing dependency chain (in reverse order) is:
> 
>  -> #2 ((work_completion)(&ser->ser_hdl_work)){+.+.}-{0:0}:
>         process_one_work+0x7c6/0x1450
>         worker_thread+0x49e/0xd00
>         kthread+0x313/0x640
>         ret_from_fork+0x221/0x300
>         ret_from_fork_asm+0x1a/0x30
> 
>  -> #1 ((wq_completion)phy0){+.+.}-{0:0}:
>         touch_wq_lockdep_map+0x8e/0x180
>         __flush_workqueue+0x129/0x10d0
>         ieee80211_stop_device+0xa8/0x110
>         ieee80211_do_stop+0x14ce/0x2880
>         ieee80211_stop+0x13a/0x2c0
>         __dev_close_many+0x18f/0x510
>         __dev_change_flags+0x25f/0x670
>         netif_change_flags+0x7b/0x160
>         do_setlink.isra.0+0x1640/0x35d0
>         rtnl_newlink+0xd8c/0x1d30
>         rtnetlink_rcv_msg+0x700/0xb80
>         netlink_rcv_skb+0x11d/0x350
>         netlink_unicast+0x49a/0x7a0
>         netlink_sendmsg+0x759/0xc20
>         ____sys_sendmsg+0x812/0xa00
>         ___sys_sendmsg+0xf7/0x180
>         __sys_sendmsg+0x11f/0x1b0
>         do_syscall_64+0xbb/0x360
>         entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
>  -> #0 (&rdev->wiphy.mtx){+.+.}-{4:4}:
>         __lock_acquire+0x124c/0x1d20
>         lock_acquire+0x154/0x2e0
>         __mutex_lock+0x17b/0x12f0
>         ser_state_run+0x5e/0x180 [rtw89_core]
>         rtw89_ser_hdl_work+0x119/0x220 [rtw89_core]
>         process_one_work+0x82d/0x1450
>         worker_thread+0x49e/0xd00
>         kthread+0x313/0x640
>         ret_from_fork+0x221/0x300
>         ret_from_fork_asm+0x1a/0x30
> 
>  other info that might help us debug this:
> 
>  Chain exists of:
>    &rdev->wiphy.mtx --> (wq_completion)phy0 --> (work_completion)(&ser->ser_hdl_work)
> 
>   Possible unsafe locking scenario:
> 
>         CPU0                    CPU1
>         ----                    ----
>    lock((work_completion)(&ser->ser_hdl_work));
>                                 lock((wq_completion)phy0);
>                                 lock((work_completion)(&ser->ser_hdl_work));
>    lock(&rdev->wiphy.mtx);
> 
>   *** DEADLOCK ***
> 
>  2 locks held by kworker/u32:1/61:
>   #0: ffff888103835148 ((wq_completion)phy0){+.+.}-{0:0}, at: process_one_work+0xefa/0x1450
>   #1: ffffc9000048fd30 ((work_completion)(&ser->ser_hdl_work)){+.+.}-{0:0}, at:
> process_one_work+0x7b5/0x1450
> 
>  stack backtrace:
>  CPU: 0 UID: 0 PID: 61 Comm: kworker/u32:1 Not tainted 6.17.0-rc2 #17 PREEMPT(voluntary)
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS edk2-20250523-14.fc42 05/23/2025
>  Workqueue: phy0 rtw89_ser_hdl_work [rtw89_core]
>  Call Trace:
>   <TASK>
>   dump_stack_lvl+0x5d/0x80
>   print_circular_bug.cold+0x178/0x1be
>   check_noncircular+0x14c/0x170
>   __lock_acquire+0x124c/0x1d20
>   lock_acquire+0x154/0x2e0
>   __mutex_lock+0x17b/0x12f0
>   ser_state_run+0x5e/0x180 [rtw89_core]
>   rtw89_ser_hdl_work+0x119/0x220 [rtw89_core]
>   process_one_work+0x82d/0x1450
>   worker_thread+0x49e/0xd00
>   kthread+0x313/0x640
>   ret_from_fork+0x221/0x300
>   ret_from_fork_asm+0x1a/0x30
>   </TASK>
> 
> Found by Linux Verification Center (linuxtesting.org).
> 
> Fixes: ebfc9199df05 ("wifi: rtw89: add wiphy_lock() to work that isn't held wiphy_lock() yet")
> Signed-off-by: Fedor Pchelkin <pchelkin@...ras.ru>

Acked-by: Ping-Ke Shih <pkshih@...ltek.com>

By the way, you mark this patchset as 'rtw'. Does it mean this patchset is
urgent to you? If not, it will be more smooth (avoid possible merge conflict)
if it goes via 'rtw-next'. Let me know your preference.