[PATCH] wifi: ath11k: initialize CFR locks before capability checks

[PATCH] wifi: ath11k: initialize CFR locks before capability checks

[Runyu Xiao]

ath11k_cfr_init() returns before initializing cfr->lock when CFR
capability is absent.  However, station removal still calls
ath11k_cfr_decrement_peer_count(), which unconditionally takes this
lock.  This leaves a production path using an uninitialized spinlock.

Initialize cfr->lock for every radio before the capability check.  Keep
the CFR ring and lookup-table lock initialization conditional, since
those objects are only used after CFR capability setup.

Fixes: 9b2e3b4ebec7 ("wifi: ath11k: Add initialization and deinitialization sequence for CFR module")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/net/wireless/ath/ath11k/cfr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/cfr.c b/drivers/net/wireless/ath/ath11k/cfr.c
index a91f25fb6..0708ca28b 100644
--- a/drivers/net/wireless/ath/ath11k/cfr.c
+++ b/drivers/net/wireless/ath/ath11k/cfr.c
@@ -956,6 +956,9 @@ int ath11k_cfr_init(struct ath11k_base *ab)
 	struct ath11k *ar;
 	int i, ret;
 
+	for (i = 0; i < ab->num_radios; i++)
+		spin_lock_init(&ab->pdevs[i].ar->cfr.lock);
+
 	if (!test_bit(WMI_TLV_SERVICE_CFR_CAPTURE_SUPPORT, ab->wmi_ab.svc_map) ||
 	    !ab->hw_params.cfr_support)
 		return 0;
@@ -971,7 +974,6 @@ int ath11k_cfr_init(struct ath11k_base *ab)
 
 		idr_init(&cfr->rx_ring.bufs_idr);
 		spin_lock_init(&cfr->rx_ring.idr_lock);
-		spin_lock_init(&cfr->lock);
 		spin_lock_init(&cfr->lut_lock);
 
 		num_lut_entries = min_t(u32, CFR_MAX_LUT_ENTRIES, db_cap.min_elem);
-- 
2.34.1

Re: [PATCH] wifi: ath11k: initialize CFR locks before capability checks

[Vasanthakumar Thiagarajan]

On 10/9/2026 11:33 AM, Runyu Xiao wrote:
> ath11k_cfr_init() returns before initializing cfr->lock when CFR
> capability is absent.  However, station removal still calls
> ath11k_cfr_decrement_peer_count(), which unconditionally takes this
> lock.  This leaves a production path using an uninitialized spinlock.
> 
> Initialize cfr->lock for every radio before the capability check.  Keep
> the CFR ring and lookup-table lock initialization conditional, since
> those objects are only used after CFR capability setup.
> 
> Fixes: 9b2e3b4ebec7 ("wifi: ath11k: Add initialization and deinitialization sequence for CFR module")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
>   drivers/net/wireless/ath/ath11k/cfr.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/cfr.c b/drivers/net/wireless/ath/ath11k/cfr.c
> index a91f25fb6..0708ca28b 100644
> --- a/drivers/net/wireless/ath/ath11k/cfr.c
> +++ b/drivers/net/wireless/ath/ath11k/cfr.c
> @@ -956,6 +956,9 @@ int ath11k_cfr_init(struct ath11k_base *ab)
>   	struct ath11k *ar;
>   	int i, ret;
>   
> +	for (i = 0; i < ab->num_radios; i++)
> +		spin_lock_init(&ab->pdevs[i].ar->cfr.lock);
> +
>   	if (!test_bit(WMI_TLV_SERVICE_CFR_CAPTURE_SUPPORT, ab->wmi_ab.svc_map) ||
>   	    !ab->hw_params.cfr_support)
>   		return 0;
> @@ -971,7 +974,6 @@ int ath11k_cfr_init(struct ath11k_base *ab)
>   
>   		idr_init(&cfr->rx_ring.bufs_idr);
>   		spin_lock_init(&cfr->rx_ring.idr_lock);
> -		spin_lock_init(&cfr->lock);
>   		spin_lock_init(&cfr->lut_lock);
>   
>   		num_lut_entries = min_t(u32, CFR_MAX_LUT_ENTRIES, db_cap.min_elem);


NAK. This can never happen.

Re: [PATCH] wifi: ath11k: initialize CFR locks before capability checks

[Jeff Johnson]

On 10/8/2026 11:03 PM, Runyu Xiao wrote:
> ath11k_cfr_init() returns before initializing cfr->lock when CFR
> capability is absent.  However, station removal still calls
> ath11k_cfr_decrement_peer_count(), which unconditionally takes this
> lock.  This leaves a production path using an uninitialized spinlock.

If this is really happening...

> 
> Initialize cfr->lock for every radio before the capability check.  Keep
> the CFR ring and lookup-table lock initialization conditional, since
> those objects are only used after CFR capability setup.

...then this is a poor solution.

And my review agent agreed:

- Initializing the locks unconditionally at the top of ath11k_cfr_init()
before the early-exit check would work mechanically, but it's wrong
semantically — callers should not need to reason about which subset of struct
fields is safe to use at any given time.

It also ruled out:
- Guarding the call site in mac.c with a capability check would duplicate the
firmware/hw-support logic outside cfr.c, and there could be other callers in
the future.
- Moving the locks to ath11k_ar_init() would be over-engineering; cfr->lock
only protects CFR state.

And concluded:

The fix: guard ath11k_cfr_decrement_peer_count() with cfr->enabled

In cfr.c, add an early return at the top of ath11k_cfr_decrement_peer_count():

void ath11k_cfr_decrement_peer_count(struct ath11k *ar,
                                     struct ath11k_sta *arsta)
{
      struct ath11k_cfr *cfr = &ar->cfr;

      if (!cfr->enabled)
              return;

      spin_lock_bh(&cfr->lock);

      if (arsta->cfr_capture.cfr_enable)
              cfr->cfr_enabled_peer_cnt--;

      spin_unlock_bh(&cfr->lock);
}

Why this is the right approach:

- cfr->enabled is set to true only after spin_lock_init(&cfr->lock) succeeds
and the ring allocation succeeds (cfr.c:997). It stays false in all early-exit
paths: the firmware/hardware capability check at line 959, the
ath11k_dbring_get_cap() failure continue at line 970, and the ring alloc
failure at line 991.
- The check is already used analogously at line 937 in the relay flush path,
so it's an established pattern in this file.
- This avoids touching ath11k_cfr_init() or the call site in mac.c, keeping
the fix minimal and localized to the function that has the precondition.

/jeff