[PATCH] wifi: ath12k: validate MAC/PHY capability count before saving

[PATCH] wifi: ath12k: validate MAC/PHY capability count before saving

[Jiale Yao]

Firmware supplies the hardware mode count and the PHY bitmap for each
mode in the service-ready event.  ath12k_wmi_save_all_mac_phy_info()
uses those bitmaps to advance through svc_ext_info->mac_phy_info, but the
destination is a fixed array of ATH12K_MAX_MAC_PHY_CAP elements.

If the total number of advertised PHY entries exceeds that limit, the
loop writes beyond mac_phy_info and corrupts adjacent memory.  A mismatch
between the advertised total and the number of parsed capability TLVs can
also make the source pointer advance beyond its allocation.

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@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@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) {
+		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) {
-- 
2.34.1

Re: [PATCH] wifi: ath12k: validate MAC/PHY capability count before saving

[Baochen Qiang]

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@vger.kernel.org
> Signed-off-by: Jiale Yao <yaojiale02@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) {