The WMI receive handlers read cmd_id from skb->data before checking that
the skb contains a complete struct wmi_cmd_hdr. The outer HTC payload
check does not establish the nested WMI header length.

Centralize current-item header validation with pskb_may_pull() and use it
in every WMI receive variant before parsing cmd_id.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/net/wireless/ath/ath10k/wmi-tlv.c |  8 ++--
 drivers/net/wireless/ath/ath10k/wmi.c     | 46 +++++++++++++----------
 drivers/net/wireless/ath/ath10k/wmi.h     |  2 +
 3 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index 01f2d1fa9d7d..345401b97ffb 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -638,15 +638,13 @@ static int ath10k_wmi_tlv_event_peer_delete_resp(struct ath10k *ar,
 
 static void ath10k_wmi_tlv_op_rx(struct ath10k *ar, struct sk_buff *skb)
 {
-	struct wmi_cmd_hdr *cmd_hdr;
 	enum wmi_tlv_event_id id;
 	bool consumed;
+	u32 cmd_id;
 
-	cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
-	id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID);
-
-	if (skb_pull(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
+	if (!ath10k_wmi_pull_cmd_hdr(skb, &cmd_id))
 		goto out;
+	id = MS(cmd_id, WMI_CMD_HDR_CMD_ID);
 
 	trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index e57588c19c80..5b64b5f9bff7 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -5873,16 +5873,28 @@ static inline void ath10k_wmi_queue_set_coverage_class_work(struct ath10k *ar)
 	}
 }
 
+bool ath10k_wmi_pull_cmd_hdr(struct sk_buff *skb, u32 *cmd_id)
+{
+	const struct wmi_cmd_hdr *cmd_hdr;
+
+	if (!pskb_may_pull(skb, sizeof(*cmd_hdr)))
+		return false;
+
+	cmd_hdr = (const void *)skb->data;
+	*cmd_id = __le32_to_cpu(cmd_hdr->cmd_id);
+	skb_pull(skb, sizeof(*cmd_hdr));
+
+	return true;
+}
+
 static void ath10k_wmi_op_rx(struct ath10k *ar, struct sk_buff *skb)
 {
-	struct wmi_cmd_hdr *cmd_hdr;
 	enum wmi_event_id id;
+	u32 cmd_id;
 
-	cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
-	id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID);
-
-	if (skb_pull(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
+	if (!ath10k_wmi_pull_cmd_hdr(skb, &cmd_id))
 		goto out;
+	id = MS(cmd_id, WMI_CMD_HDR_CMD_ID);
 
 	trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
 
@@ -6002,15 +6014,13 @@ static void ath10k_wmi_op_rx(struct ath10k *ar, struct sk_buff *skb)
 
 static void ath10k_wmi_10_1_op_rx(struct ath10k *ar, struct sk_buff *skb)
 {
-	struct wmi_cmd_hdr *cmd_hdr;
 	enum wmi_10x_event_id id;
 	bool consumed;
+	u32 cmd_id;
 
-	cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
-	id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID);
-
-	if (skb_pull(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
+	if (!ath10k_wmi_pull_cmd_hdr(skb, &cmd_id))
 		goto out;
+	id = MS(cmd_id, WMI_CMD_HDR_CMD_ID);
 
 	trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
 
@@ -6133,15 +6143,13 @@ static void ath10k_wmi_10_1_op_rx(struct ath10k *ar, struct sk_buff *skb)
 
 static void ath10k_wmi_10_2_op_rx(struct ath10k *ar, struct sk_buff *skb)
 {
-	struct wmi_cmd_hdr *cmd_hdr;
 	enum wmi_10_2_event_id id;
 	bool consumed;
+	u32 cmd_id;
 
-	cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
-	id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID);
-
-	if (skb_pull(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
+	if (!ath10k_wmi_pull_cmd_hdr(skb, &cmd_id))
 		goto out;
+	id = MS(cmd_id, WMI_CMD_HDR_CMD_ID);
 
 	trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
 
@@ -6282,15 +6290,13 @@ static void ath10k_wmi_10_2_op_rx(struct ath10k *ar, struct sk_buff *skb)
 
 static void ath10k_wmi_10_4_op_rx(struct ath10k *ar, struct sk_buff *skb)
 {
-	struct wmi_cmd_hdr *cmd_hdr;
 	enum wmi_10_4_event_id id;
 	bool consumed;
+	u32 cmd_id;
 
-	cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
-	id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID);
-
-	if (!skb_pull(skb, sizeof(struct wmi_cmd_hdr)))
+	if (!ath10k_wmi_pull_cmd_hdr(skb, &cmd_id))
 		goto out;
+	id = MS(cmd_id, WMI_CMD_HDR_CMD_ID);
 
 	trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 7f50a1de6b97..7c787173b187 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -7440,6 +7440,7 @@ struct ath10k_vif;
 struct ath10k_fw_stats_pdev;
 struct ath10k_fw_stats_peer;
 struct ath10k_fw_stats;
+struct sk_buff;
 
 int ath10k_wmi_attach(struct ath10k *ar);
 void ath10k_wmi_detach(struct ath10k *ar);
@@ -7449,6 +7450,7 @@ int ath10k_wmi_wait_for_unified_ready(struct ath10k *ar);
 
 struct sk_buff *ath10k_wmi_alloc_skb(struct ath10k *ar, u32 len);
 int ath10k_wmi_connect(struct ath10k *ar);
+bool ath10k_wmi_pull_cmd_hdr(struct sk_buff *skb, u32 *cmd_id);
 
 int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, u32 cmd_id);
 int ath10k_wmi_cmd_send_nowait(struct ath10k *ar, struct sk_buff *skb,
-- 
2.43.0

On 7/21/2026 9:20 PM, Pengpeng Hou wrote:

> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> index e57588c19c80..5b64b5f9bff7 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -5873,16 +5873,28 @@ static inline void ath10k_wmi_queue_set_coverage_class_work(struct ath10k *ar)
>  	}
>  }
>  
> +bool ath10k_wmi_pull_cmd_hdr(struct sk_buff *skb, u32 *cmd_id)
> +{
> +	const struct wmi_cmd_hdr *cmd_hdr;
> +
> +	if (!pskb_may_pull(skb, sizeof(*cmd_hdr)))
> +		return false;
> +
> +	cmd_hdr = (const void *)skb->data;
> +	*cmd_id = __le32_to_cpu(cmd_hdr->cmd_id);
> +	skb_pull(skb, sizeof(*cmd_hdr));

consider following the pattern from my ath12k patch instead:
https://patch.msgid.link/20260716-ath12k_wmi_op_rx-overread-v1-1-327a4b1c2372@oss.qualcomm.com

use skb_pull_data() instead of pskb_may_pull() + skb_pull()
consider masking out the id here instead of in each of the callers
(if all of the caller have the same masking)

> +
> +	return true;
> +}