[PATCH ath-next] wifi: ath12k: accept num_users equal to HTT_PPDU_STATS_MAX_USERS

[PATCH ath-next] wifi: ath12k: accept num_users equal to HTT_PPDU_STATS_MAX_USERS

[Goran Sustek]

ath12k_htt_pull_ppdu_stats() rejects a PPDU stats message when the
common TLV reports num_users >= HTT_PPDU_STATS_MAX_USERS. The check
guards a loop that iterates i < num_users over user_stats[], which has
HTT_PPDU_STATS_MAX_USERS entries, so num_users equal to
HTT_PPDU_STATS_MAX_USERS stays in bounds. Commit 1bc44a505a22 ("wifi:
ath12k: fix possible out-of-bound read in ath12k_htt_pull_ppdu_stats()")
rejects that value, which is off by one.

QCN9274 firmware WLAN.WBE.1.6-01243 sends num_users 8 in messages that
otherwise parse cleanly. One AP logged the resulting warning 468 times
between 2026-08-17 and 2026-11-01, always with the value 8, and never
logged "Failed to parse tlv":

  HTT PPDU STATS event has unexpected num_users 8, should be smaller than 8

Accept num_users == HTT_PPDU_STATS_MAX_USERS.

Rejecting a message has no functional effect: the PPDU entry stays on
the list, its per-user statistics are applied when it is evicted, and
the only caller ignores the return value. A larger value is therefore
not worth a warning either, so report it at debug level, as done in
"wifi: ath12k: downgrade log level for CE buffer enqueue failure".

Fixes: 1bc44a505a22 ("wifi: ath12k: fix possible out-of-bound read in ath12k_htt_pull_ppdu_stats()")
Signed-off-by: Goran Sustek <gsustek@gmail.com>
---
Notes for reviewers:

The check guards two loops that can never run. Both are gated on
ppdu_info->tlv_bitmap, frame_type, delay_ba and bar_num_users, and the
first also on the per-user delay_ba; none of these is written anywhere
in ath12k, not since the initial driver (2022). The parser that would
fill them (per-user common TLV, MU-BAR user count) was never ported, so
ath12k_copy_to_delay_stats(), ath12k_copy_to_bar() and the peer's
ppdu_stats_delayba/delayba_flag are unreachable as well.

If you prefer, I have a patch ready that removes the loops, the helpers,
the fields and this check instead (about 90 lines, no functional
change). Keeping them only makes sense if delayed block ack accounting
is going to be implemented. In that case the BAR restore loop also
needs a bound (it iterates bar_num_users over user_stats[] unchecked),
and note that the per-user slot finder only uses
HTT_PPDU_STATS_MAX_USERS - 1 slots.

Compile tested only (W=1).

 drivers/net/wireless/ath/ath12k/dp_htt.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/dp_htt.c b/drivers/net/wireless/ath/ath12k/dp_htt.c
index 72069640bd38..9a409dd44d98 100644
--- a/drivers/net/wireless/ath/ath12k/dp_htt.c
+++ b/drivers/net/wireless/ath/ath12k/dp_htt.c
@@ -487,12 +487,12 @@ static int ath12k_htt_pull_ppdu_stats(struct ath12k_base *ab,
 		goto exit;
 	}
 
-	if (ppdu_info->ppdu_stats.common.num_users >= HTT_PPDU_STATS_MAX_USERS) {
+	if (ppdu_info->ppdu_stats.common.num_users > HTT_PPDU_STATS_MAX_USERS) {
 		spin_unlock_bh(&dp_pdev->ppdu_list_lock);
-		ath12k_warn(ab,
-			    "HTT PPDU STATS event has unexpected num_users %u, should be smaller than %u\n",
-			    ppdu_info->ppdu_stats.common.num_users,
-			    HTT_PPDU_STATS_MAX_USERS);
+		ath12k_dbg(ab, ATH12K_DBG_DP_HTT,
+			   "HTT PPDU STATS event has unexpected num_users %u, should not exceed %u\n",
+			   ppdu_info->ppdu_stats.common.num_users,
+			   HTT_PPDU_STATS_MAX_USERS);
 		ret = -EINVAL;
 		goto exit;
 	}

base-commit: a3ddbb45768717b5c9f923489387cf55c838d1ad
-- 
2.56.0

Re: [PATCH ath-next] wifi: ath12k: accept num_users equal to HTT_PPDU_STATS_MAX_USERS

[Jeff Johnson]

On 11/1/2026 12:37 PM, Goran Sustek wrote:
> ath12k_htt_pull_ppdu_stats() rejects a PPDU stats message when the
> common TLV reports num_users >= HTT_PPDU_STATS_MAX_USERS. The check
> guards a loop that iterates i < num_users over user_stats[], which has
> HTT_PPDU_STATS_MAX_USERS entries, so num_users equal to
> HTT_PPDU_STATS_MAX_USERS stays in bounds. Commit 1bc44a505a22 ("wifi:
> ath12k: fix possible out-of-bound read in ath12k_htt_pull_ppdu_stats()")
> rejects that value, which is off by one.
> 
> QCN9274 firmware WLAN.WBE.1.6-01243 sends num_users 8 in messages that
> otherwise parse cleanly. One AP logged the resulting warning 468 times
> between 2026-08-17 and 2026-11-01, always with the value 8, and never
> logged "Failed to parse tlv":
> 
>   HTT PPDU STATS event has unexpected num_users 8, should be smaller than 8
> 
> Accept num_users == HTT_PPDU_STATS_MAX_USERS.
> 
> Rejecting a message has no functional effect: the PPDU entry stays on
> the list, its per-user statistics are applied when it is evicted, and
> the only caller ignores the return value. A larger value is therefore
> not worth a warning either, so report it at debug level, as done in
> "wifi: ath12k: downgrade log level for CE buffer enqueue failure".
> 
> Fixes: 1bc44a505a22 ("wifi: ath12k: fix possible out-of-bound read in ath12k_htt_pull_ppdu_stats()")
> Signed-off-by: Goran Sustek <gsustek@gmail.com>
> ---
> Notes for reviewers:
> 
> The check guards two loops that can never run. Both are gated on
> ppdu_info->tlv_bitmap, frame_type, delay_ba and bar_num_users, and the
> first also on the per-user delay_ba; none of these is written anywhere
> in ath12k, not since the initial driver (2022). The parser that would
> fill them (per-user common TLV, MU-BAR user count) was never ported, so
> ath12k_copy_to_delay_stats(), ath12k_copy_to_bar() and the peer's
> ppdu_stats_delayba/delayba_flag are unreachable as well.
> 
> If you prefer, I have a patch ready that removes the loops, the helpers,
> the fields and this check instead (about 90 lines, no functional
> change). Keeping them only makes sense if delayed block ack accounting
> is going to be implemented. In that case the BAR restore loop also
> needs a bound (it iterates bar_num_users over user_stats[] unchecked),
> and note that the per-user slot finder only uses
> HTT_PPDU_STATS_MAX_USERS - 1 slots.
> 
> Compile tested only (W=1).
> 
>  drivers/net/wireless/ath/ath12k/dp_htt.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/dp_htt.c b/drivers/net/wireless/ath/ath12k/dp_htt.c
> index 72069640bd38..9a409dd44d98 100644
> --- a/drivers/net/wireless/ath/ath12k/dp_htt.c
> +++ b/drivers/net/wireless/ath/ath12k/dp_htt.c
> @@ -487,12 +487,12 @@ static int ath12k_htt_pull_ppdu_stats(struct ath12k_base *ab,
>  		goto exit;
>  	}
>  
> -	if (ppdu_info->ppdu_stats.common.num_users >= HTT_PPDU_STATS_MAX_USERS) {
> +	if (ppdu_info->ppdu_stats.common.num_users > HTT_PPDU_STATS_MAX_USERS) {
>  		spin_unlock_bh(&dp_pdev->ppdu_list_lock);
> -		ath12k_warn(ab,
> -			    "HTT PPDU STATS event has unexpected num_users %u, should be smaller than %u\n",
> -			    ppdu_info->ppdu_stats.common.num_users,
> -			    HTT_PPDU_STATS_MAX_USERS);
> +		ath12k_dbg(ab, ATH12K_DBG_DP_HTT,
> +			   "HTT PPDU STATS event has unexpected num_users %u, should not exceed %u\n",
> +			   ppdu_info->ppdu_stats.common.num_users,
> +			   HTT_PPDU_STATS_MAX_USERS);

please don't change the warning. this is flagging a discrepancy between the
driver and the firmware and should be reported if it occurs in the future
(i.e. if a future firmware supports more users but the driver isn't updated)

>  		ret = -EINVAL;
>  		goto exit;
>  	}
> 
> base-commit: a3ddbb45768717b5c9f923489387cf55c838d1ad

My review agent says there is lot of associated code that is also treating
HTT_PPDU_STATS_MAX_USERS inconsistently. Much of this is directly inherited
from ath11k. I'm posting these finding here so that the developers can further
evaluate what approach to take with this patch.

BCC: Qualcomm's ath12k internal review list

/jeff


❯ can you provide a list of references to the patched version of the ath12k
  code where num_users == 8 isn't properly handled?

Here's the corrected and complete list, now reflecting that the firmware
genuinely sends 8 users and the 8th is silently discarded. All sites are in
drivers/net/wireless/ath/ath12k/dp_htt.c unless noted.

1. The 8th user is silently dropped during parse (root cause)

- dp_htt.c:49 — ath12k_get_ppdu_user_index(), loop i <
HTT_PPDU_STATS_MAX_USERS - 1 (0..6). Can only allocate slots 0..6, so the 8th
user never gets a slot and the function returns -EINVAL for it.
- dp_htt.c:95-96, :115-116, :135-137 — the three parse cases (USR_RATE,
USR_COMPLTN_COMMON, USR_COMPLTN_ACK_BA_STATUS): cur_user =
ath12k_get_ppdu_user_index(...); if (cur_user < 0) return -EINVAL;. For the
8th user this returns -EINVAL, so its rate / completion / ack-BA TLVs are not
stored.
- dp_htt.c:176-178 — ath12k_dp_htt_tlv_iter(): if (ret == -ENOMEM) return ret;
— only -ENOMEM aborts. The -EINVAL from the 8th user is swallowed, the
iterator continues and returns 0 (:183). This is why the drop is silent and
:486 ("Failed to parse tlv") never fires — reconciling the commit's field
evidence with 8 real users.

2. The 8th user's stats are never reported (consumer side)

- dp_htt.c:365 — ath12k_htt_update_ppdu_stats(), loop user <
HTT_PPDU_STATS_MAX_USERS - 1 (0..6). At eviction this pushes per-peer TX stats
for users 0..6 only; index 7 is never consumed (:195 indexes user_stats[user]
with that capped counter). Even if slot 7 were populated, its stats would not
be reported.

3. Loops whose ceiling the patch widened past what sections 1–2 can fill

- dp_htt.c:504-512 — delay-BA backup loop, for (i = 0; i < common.num_users;
i++). The patch (:490) now lets num_users == 8 through, so i == 7 reads
user_stats[7] (:505 peer_id, :510 usr_stats) — a slot section 1 guarantees is
empty. In-bounds and benign today (write at :512 gated by usr_stats->delay_ba
at :511, which is 0 for the empty slot), but it's the concrete ceiling
mismatch: num_users admits 8 while the allocator fills only 7.
- dp_htt.c:519-527 — BAR restore loop, for (i = 0; i < bar_num_users; i++),
reading user_stats[7] at :520/:525. Driven by the separate bar_num_users
counter (dp_htt.h:1296), never bounded by the :490 guard before or after the
patch — same empty-slot-7 exposure, pre-existing and independent of this
change.

4. The patched guard itself

- dp_htt.c:490 — if (ppdu_info->ppdu_stats.common.num_users >
HTT_PPDU_STATS_MAX_USERS). Correctly stops rejecting num_users == 8, but it's
a necessary-not-sufficient part of "handling 8 users": without the - 1
removals in sections 1–2, accepting the message just means the 8th user is
discarded a bit later and more quietly instead of the whole message being
dropped.

Supporting declarations

- dp_htt.h:1282 — #define HTT_PPDU_STATS_MAX_USERS 8
- dp_htt.h:1287 — struct htt_ppdu_user_stats
user_stats[HTT_PPDU_STATS_MAX_USERS]; (slots 0..7)
- dp_htt.h:1115 — u8 num_users;
- dp_htt.h:1296 — u32 bar_num_users;

Excluded after checking (not this path)

- dp_mon.c:905-909, :1049-1053 — ppdu_info->num_users bounded by
HAL_MAX_UL_MU_USERS, separate monitor-mode struct.
- dp.c:355-417 — TCL bank-profile num_users, unrelated.
- debugfs_htt_stats.c:2166 / .h:1216 — a separate __le32 num_users display
field.

Bottom line: to actually store and report the 8th user, the minimal change is
removing - 1 at :49 and :365 (both in-bounds against the 8-slot array), on top
of the existing :490 relaxation. Sections 1–2 are the real gap; section 3 is
the cosmetic ceiling mismatch that disappears once slot 7 is legitimately
populated.