Re: [PATCH v2] wifi: ath9k: reject short WMI command responses

[Jeff Johnson]

On 8/14/2026 12:58 AM, Pengpeng Hou wrote:
> ath9k_wmi_rsp_callback() removes the validated WMI header and copies
> the number of bytes expected by the waiting command into its response
> buffer. A device response shorter than that expectation can therefore
> be read beyond the skb payload.
> 
> Record -EMSGSIZE for a short response, complete the waiter, and return
> the stored status from the command path. This also prevents callers from
> consuming stale response bytes as a successful reply.
> 
> Fixes: fb9987d0f748 ("ath9k_htc: Support for AR9271 chipset.")
> Assisted-by: Codex:gpt-5
> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>
> ---
> Changes since v1: https://lore.kernel.org/all/20260704011405.55089-1-pengpeng@xxxxxxxxxxx/
> - return a stored response error to the waiting command
> - retain acceptance of replies longer than the requested prefix
> - recheck callback synchronization under the existing WMI lock
> 
> The WMI callback and waiter synchronization were reviewed statically; no
> ath9k firmware fault injection was performed.
> 
>  drivers/net/wireless/ath/ath9k/wmi.c | 17 +++++++++++++++--
>  drivers/net/wireless/ath/ath9k/wmi.h |  1 +
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/wmi.c b/drivers/net/wireless/ath/ath9k/wmi.c
> index 284e8c13b043..06d9975f9717 100644
> --- a/drivers/net/wireless/ath/ath9k/wmi.c
> +++ b/drivers/net/wireless/ath/ath9k/wmi.c
> @@ -206,8 +206,16 @@ static void ath9k_wmi_rsp_callback(struct wmi *wmi, struct sk_buff *skb)
>  {
>  	skb_pull(skb, sizeof(struct wmi_cmd_hdr));
>  
> -	if (wmi->cmd_rsp_buf != NULL && wmi->cmd_rsp_len != 0)
> +	if (wmi->cmd_rsp_buf && wmi->cmd_rsp_len) {
> +		if (skb->len < wmi->cmd_rsp_len) {
> +			wmi->cmd_rsp_status = -EMSGSIZE;

my review agent notes that if this path is taken that there is no debug
message alerting this. see my suggestion on dealing with this below...

> +			goto complete;
> +		}
>  		memcpy(wmi->cmd_rsp_buf, skb->data, wmi->cmd_rsp_len);
> +	}
> +	wmi->cmd_rsp_status = 0;
> +
> +complete:
>  
>  	complete(&wmi->cmd_wait);
>  }
> @@ -300,6 +308,7 @@ static int ath9k_wmi_cmd_issue(struct wmi *wmi,
>  	/* record the rsp buffer and length */
>  	wmi->cmd_rsp_buf = rsp_buf;
>  	wmi->cmd_rsp_len = rsp_len;
> +	wmi->cmd_rsp_status = 0;
>  
>  	wmi->last_seq_id = wmi->tx_seq_id;
>  	spin_unlock_irqrestore(&wmi->wmi_lock, flags);
> @@ -356,9 +365,13 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id,
>  		return -ETIMEDOUT;
>  	}
>  
> +	spin_lock_irqsave(&wmi->wmi_lock, flags);
> +	ret = wmi->cmd_rsp_status;
> +	spin_unlock_irqrestore(&wmi->wmi_lock, flags);

in order to have a debug message on length error we could drop the following
"mutex_unlock() and return ret" so that all paths go past out:

> +
>  	mutex_unlock(&wmi->op_mutex);
>  
> -	return 0;
> +	return ret;
>  
>  out:

and here log on error:
	if (ret)
		ath_dbg(...)

>  	ath_dbg(common, WMI, "WMI failure for: %s\n", wmi_cmd_to_name(cmd_id));

suggest adding the errno to the log message since now there are multiple
reasons it can fail

> diff --git a/drivers/net/wireless/ath/ath9k/wmi.h b/drivers/net/wireless/ath/ath9k/wmi.h
> index 5c3b710b8f31..0bb9d31f1a1c 100644
> --- a/drivers/net/wireless/ath/ath9k/wmi.h
> +++ b/drivers/net/wireless/ath/ath9k/wmi.h
> @@ -157,6 +157,7 @@ struct wmi {
>  	u16 tx_seq_id;
>  	u8 *cmd_rsp_buf;
>  	u32 cmd_rsp_len;
> +	int cmd_rsp_status;
>  	bool stopped;
>  
>  	struct list_head pending_tx_events;