Re: [PATCH v3 3/3] wifi: ath6kl: fix OOB read from firmware num_msg in TX complete handler

[Jeff Johnson]

On 4/21/2026 6:50 AM, Tristan Madani wrote:
> From: Tristan Madani <tristan@xxxxxxxxxxxxxxxxxxx>
> 
> The firmware-controlled num_msg field (u8, 0-255) drives the loop in
> ath6kl_wmi_tx_complete_event_rx() without validation against the buffer
> length. This allows out-of-bounds reads of up to 1020 bytes past the
> WMI event buffer when the firmware sends an inflated num_msg.
> 
> Add a check that the buffer is large enough to hold num_msg entries.
> 
> Fixes: bdcd81707973 ("Add ath6kl cleaned up driver")
> Signed-off-by: Tristan Madani <tristan@xxxxxxxxxxxxxxxxxxx>
> ---
> Changes in v3:
>   - Regenerated from wireless-next with proper git format-patch to
>     produce valid index hashes (v2 had post-processed index lines).
> 
> Changes in v2:
>   - No code changes from v1.
> 
>  drivers/net/wireless/ath/ath6kl/wmi.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
> index 1cafbac2938fe..f56722c5ef5f1 100644
> --- a/drivers/net/wireless/ath/ath6kl/wmi.c
> +++ b/drivers/net/wireless/ath/ath6kl/wmi.c
> @@ -484,6 +484,12 @@ static int ath6kl_wmi_tx_complete_event_rx(u8 *datap, int len)
>  
>  	evt = (struct wmi_tx_complete_event *) datap;
>  
> +	if (len < sizeof(*evt) ||
> +	    len < sizeof(*evt) + evt->num_msg * sizeof(struct tx_complete_msg_v1)) {
> +		ath6kl_dbg(ATH6KL_DBG_WMI, "tx complete: invalid len %d for %u msgs\n",
> +			   len, evt->num_msg);

In the case where the first test is true, the logging of evt->num_msg will
still overread the buffer.

I think the logic would be more clear if it follows the pattern in the 2/3
patch, first validate the fixed portion of the struct is available, and then
separately validate the variable portion of the struct is available:

	if (len < sizeof(*evt) {
		ath6kl_dbg(ATH6KL_DBG_WMI, "tx complete: invalid len %d\n",
			   len);
		return -EINVAL;
	}
		
	if (len < sizeof(*evt) + evt->num_msg * sizeof(struct tx_complete_msg_v1)) {
		ath6kl_dbg(ATH6KL_DBG_WMI, "tx complete: invalid len %d for %u msgs\n",
			   len, evt->num_msg);
		return -EINVAL;
	}

>  	ath6kl_dbg(ATH6KL_DBG_WMI, "comp: %d %d %d\n",
>  		   evt->num_msg, evt->msg_len, evt->msg_type);
>