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

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

[Pengpeng Hou]

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@iscas.ac.cn>
---
Changes since v1: https://lore.kernel.org/all/20260704011405.55089-1-pengpeng@iscas.ac.cn/
- 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;
+			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);
+
 	mutex_unlock(&wmi->op_mutex);
 
-	return 0;
+	return ret;
 
 out:
 	ath_dbg(common, WMI, "WMI failure for: %s\n", wmi_cmd_to_name(cmd_id));
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;
-- 
2.50.1 (Apple Git-155)

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@iscas.ac.cn>
> ---
> Changes since v1: https://lore.kernel.org/all/20260704011405.55089-1-pengpeng@iscas.ac.cn/
> - 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;

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

[Toke Høiland-Jørgensen]

Jeff Johnson <jeff.johnson@oss.qualcomm.com> writes:

> 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@iscas.ac.cn>
>> ---
>> Changes since v1: https://lore.kernel.org/all/20260704011405.55089-1-pengpeng@iscas.ac.cn/
>> - 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...

I think you're being pedantic here. I'm pretty sure there's no real
hardware that does this; merging this is meant to protect against a
USB device pretending to be an ath9k_htc and trying to exploit the
kernel. So I think it's fine to merge as-is.

-Toke

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

[Jeff Johnson]

On 9/17/2026 1:56 AM, Toke Høiland-Jørgensen wrote:
> Jeff Johnson <jeff.johnson@oss.qualcomm.com> writes:
> 
>> 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@iscas.ac.cn>
>>> ---
>>> Changes since v1: https://lore.kernel.org/all/20260704011405.55089-1-pengpeng@iscas.ac.cn/
>>> - 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...
> 
> I think you're being pedantic here. I'm pretty sure there's no real
> hardware that does this; merging this is meant to protect against a
> USB device pretending to be an ath9k_htc and trying to exploit the
> kernel. So I think it's fine to merge as-is.
And I'm fine with that.

/jeff

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

[Jeff Johnson]

On Fri, 14 Aug 2026 15:58:37 +0800, 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.
> 
> [...]

Applied, thanks!

[1/1] wifi: ath9k: reject short WMI command responses
      commit: 93ce8ea70451e800601bc9444aa33043de15d67b

Best regards,
-- 
Jeff Johnson <jeff.johnson@oss.qualcomm.com>

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

[Mehmet Fide]

Hi,

this change breaks ath9k_htc with the AR9271 firmware from linux-firmware
(ath9k_htc/htc_9271-1.4.0.fw, "FW Version: 1.4"). The firmware answers
most commands with an empty payload, while the driver passes a response
buffer for every command: cmd_rsp in WMI_CMD()/WMI_CMD_BUF(), the 4-byte
value of WMI_REG_WRITE_CMDID, the 4- or 12-byte buffer of
WMI_REG_RMW_CMDID. With the new check all of these calls return
-EMSGSIZE, and bringing the interface up fails (hostapd: "Could not set
interface wlan0 flags (UP): Message too long").

Seen on a Colibri VF50 (Vybrid, armv7) with an AR9271 USB adapter on
6.12.113, which carries this commit as 03ef61d05aa2. A log line in
ath9k_wmi_rsp_callback() for every rejected response shows, during boot
and interface up:

  WMI_ATH_INIT_CMDID          0 bytes, 1 expected
  WMI_START_RECV_CMDID        0 bytes, 1 expected
  WMI_FLUSH_RECV_CMDID        0 bytes, 1 expected
  WMI_SET_MODE_CMDID          0 bytes, 1 expected
  WMI_REG_WRITE_CMDID         0 bytes, 4 expected
  WMI_TARGET_IC_UPDATE_CMDID  0 bytes, 1 expected
  WMI_REG_RMW_CMDID           0 bytes, 12 or 4 expected

With the commit reverted on the same kernel, logging instead of rejecting,
the same commands keep answering with 0 bytes, and so do
WMI_DISABLE_INTR_CMDID, WMI_ENABLE_INTR_CMDID, WMI_DRAIN_TXQ_ALL_CMDID,
WMI_STOP_RECV_CMDID, WMI_NODE_CREATE/REMOVE_CMDID and
WMI_VAP_CREATE/REMOVE_CMDID. WMI_REG_READ_CMDID is the only command in
the log that returns data.

So an empty reply is how this firmware answers a command without a
result, and rejecting it takes the device down. I have not tried other
firmware versions. I can test a fix on this hardware.

#regzbot introduced: 93ce8ea70451

Thanks,
Mehmet

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

[Toke Høiland-Jørgensen]

Mehmet Fide <mehmet.fide@gmail.com> writes:

> Hi,
>
> this change breaks ath9k_htc with the AR9271 firmware from linux-firmware
> (ath9k_htc/htc_9271-1.4.0.fw, "FW Version: 1.4"). The firmware answers
> most commands with an empty payload, while the driver passes a response
> buffer for every command: cmd_rsp in WMI_CMD()/WMI_CMD_BUF(), the 4-byte
> value of WMI_REG_WRITE_CMDID, the 4- or 12-byte buffer of
> WMI_REG_RMW_CMDID. With the new check all of these calls return
> -EMSGSIZE, and bringing the interface up fails (hostapd: "Could not set
> interface wlan0 flags (UP): Message too long").

Thank you for the detailed report! And apologies for the regression;
seems I need to do better at pushing back on untested slop patches :(

Given the lateness of the RC, I'll send a revert directly to Linus,
hopefully he can merge that ahead of the 7.3 release.

-Toke