[PATCH] wifi: ath12k: validate MAC/PHY capability count before saving
Baochen Qiang
baochen.qiang at oss.qualcomm.com
Mon Sep 28 20:32:33 PDT 2026
On 9/25/2026 8:17 PM, Jiale Yao wrote:
> Firmware supplies the hardware mode count and the PHY bitmap for each
> mode in the service-ready event
to be accurate, it is service ready ext event. we do have another event named service ready.
> A mismatch
> between the advertised total and the number of parsed capability TLVs can
> also make the source pointer advance beyond its allocation.
This claim doesn't hold. The source buffer mac_phy_caps is kzalloc'd for tot_phy_id
elements, and the loop's mac_phy_cap++ runs exactly tot_phy_id times total, so the source
pointer reads at most tot_phy_id entries — precisely the allocation bound, never past it.
The n_mac_phy_caps > tot_phy_id case is already rejected at parse time in
ath12k_wmi_mac_phy_caps_parse(). The only mismatch that can actually reach
save_all_mac_phy_info() is n_mac_phy_caps < tot_phy_id, in which case the loop reads
zeroed-but-allocated source entries and populates mac_phy_info with all-zero PHY info — a
correctness bug, not a source-buffer overrun.
>
> Validate both counts before writing any entries and propagate the error to
> the service-ready parser.
>
> Fixes: 062ade23991e ("wifi: ath12k: parse and save hardware mode info from WMI_SERVICE_READY_EXT_EVENTID event for later use")
> Cc: stable at vger.kernel.org
> Signed-off-by: Jiale Yao <yaojiale02 at 163.com>
> ---
> drivers/net/wireless/ath/ath12k/wmi.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> index d5160af60e00..59ac17f0d48d 100644
> --- a/drivers/net/wireless/ath/ath12k/wmi.c
> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> @@ -5004,7 +5004,7 @@ ath12k_wmi_save_mac_phy_info(struct ath12k_base *ab,
> __le32_to_cpu(mac_phy_cap->high_5ghz_chan_freq);
> }
>
> -static void
> +static int
> ath12k_wmi_save_all_mac_phy_info(struct ath12k_base *ab,
> struct ath12k_wmi_svc_rdy_ext_parse *svc_rdy_ext)
> {
> @@ -5015,6 +5015,20 @@ ath12k_wmi_save_all_mac_phy_info(struct ath12k_base *ab,
> u32 hw_mode_id, phy_bit_map;
> u8 hw_idx;
>
> + if (svc_rdy_ext->tot_phy_id > ARRAY_SIZE(svc_ext_info->mac_phy_info)) {
> + ath12k_warn(ab, "too many PHY entries %u (max %zu)\n",
> + svc_rdy_ext->tot_phy_id,
> + ARRAY_SIZE(svc_ext_info->mac_phy_info));
> + return -EINVAL;
> + }
> +
> + if (svc_rdy_ext->n_mac_phy_caps != svc_rdy_ext->tot_phy_id) {
per comment above, should we use '<' instead of '!=' ?
Besides, I think the right place for such check should be in
ath12k_wmi_svc_rdy_ext_parse(), right before ath12k_wmi_save_all_mac_phy_info() and after
mac phy cap parse ?
> + ath12k_warn(ab, "invalid number of MAC/PHY caps %u, expected %u\n",
> + svc_rdy_ext->n_mac_phy_caps,
> + svc_rdy_ext->tot_phy_id);
> + return -EINVAL;
> + }
> +
> mac_phy_info = &svc_ext_info->mac_phy_info[0];
> mac_phy_cap = svc_rdy_ext->mac_phy_caps;
>
> @@ -5044,6 +5058,8 @@ ath12k_wmi_save_all_mac_phy_info(struct ath12k_base *ab,
> phy_bit_map >>= 1;
> }
> }
> +
> + return 0;
> }
>
> static int ath12k_wmi_svc_rdy_ext_parse(struct ath12k_base *ab,
> @@ -5094,7 +5110,9 @@ static int ath12k_wmi_svc_rdy_ext_parse(struct ath12k_base *ab,
> return ret;
> }
>
> - ath12k_wmi_save_all_mac_phy_info(ab, svc_rdy_ext);
> + ret = ath12k_wmi_save_all_mac_phy_info(ab, svc_rdy_ext);
> + if (ret)
> + return ret;
>
> svc_rdy_ext->mac_phy_done = true;
> } else if (!svc_rdy_ext->ext_hal_reg_done) {
More information about the ath12k
mailing list