From: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
To: Ziyi Guo <n7l8m4@u.northwestern.edu>, Jeff Johnson <jjohnson@kernel.org>
Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] wifi: ath10k: fix lock protection in ath10k_wmi_event_peer_sta_ps_state_chg()
Date: Mon, 26 Jan 2026 10:15:13 +0800 [thread overview]
Message-ID: <86af03f9-3823-4561-95ec-8b15ed88249c@oss.qualcomm.com> (raw)
In-Reply-To: <20260123175611.767731-1-n7l8m4@u.northwestern.edu>
On 1/24/2026 1:56 AM, Ziyi Guo wrote:
> ath10k_wmi_event_peer_sta_ps_state_chg() uses lockdep_assert_held() to
> assert that ar->data_lock should be held by the caller, but neither
> ath10k_wmi_10_2_op_rx() nor ath10k_wmi_10_4_op_rx() acquire this lock
> before calling this function.
>
> The field arsta->peer_ps_state is documented as protected by
> ar->data_lock in core.h, and other accessors (ath10k_peer_ps_state_disable,
> ath10k_dbg_sta_read_peer_ps_state) properly acquire this lock.
there is another instance in ath10k_sta_state() where the lock is not acquired, but I
guess that is OK since during the NOTEXIST -> NONE transition no race is expected.
>
> Add spin_lock_bh()/spin_unlock_bh() around the peer_ps_state update,
> and remove the lockdep_assert_held() to be aligned with new locking,
> following the pattern used by other WMI event handlers in the driver.
>
> Signed-off-by: Ziyi Guo <n7l8m4@u.northwestern.edu>
> ---
> v2:
> - Remove lockdep_assert_held() as suggested, since
> we are now taking the lock internally.
>
> drivers/net/wireless/ath/ath10k/wmi.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> index b4aad6604d6d..061a2fa8f00f 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -5289,7 +5289,6 @@ ath10k_wmi_event_peer_sta_ps_state_chg(struct ath10k *ar, struct sk_buff *skb)
> struct ath10k_sta *arsta;
> u8 peer_addr[ETH_ALEN];
>
> - lockdep_assert_held(&ar->data_lock);
>
> ev = (struct wmi_peer_sta_ps_state_chg_event *)skb->data;
> ether_addr_copy(peer_addr, ev->peer_macaddr.addr);
> @@ -5305,7 +5304,9 @@ ath10k_wmi_event_peer_sta_ps_state_chg(struct ath10k *ar, struct sk_buff *skb)
> }
>
> arsta = (struct ath10k_sta *)sta->drv_priv;
> + spin_lock_bh(&ar->data_lock);
> arsta->peer_ps_state = __le32_to_cpu(ev->peer_ps_state);
> + spin_unlock_bh(&ar->data_lock);
>
> exit:
> rcu_read_unlock();
Reviewed-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
next prev parent reply other threads:[~2026-01-26 2:15 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-23 17:56 Ziyi Guo
2026-01-26 2:15 ` Baochen Qiang [this message]
2026-01-28 16:36 ` Jeff Johnson
2026-01-30 15:15 ` Jeff Johnson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86af03f9-3823-4561-95ec-8b15ed88249c@oss.qualcomm.com \
--to=baochen.qiang@oss.qualcomm.com \
--cc=ath10k@lists.infradead.org \
--cc=jjohnson@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=n7l8m4@u.northwestern.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®