Re: [PATCH 2/2] wifi: ath10k: Add missing validation in ath10k_htt_rx_proc_rx_frag_ind_hl

[Jeff Johnson <jeff.johnson@oss.qualcomm.com>]

On 9/10/2026 1:06 AM, Rivaldi Hormat wrote:
> The ath10k_htt_rx_proc_rx_frag_ind_hl function processes fragment
> indications sent by the WiFi firmware. It performs pointer arithmetic
> without validating the skb length:
> 
> 1. skb_pull(skb, HTT_RX_FRAG_IND_INFO0_HEADER_LEN) without checking
>    that skb->len >= HTT_RX_FRAG_IND_INFO0_HEADER_LEN.
> 
> 2. hdr = (struct ieee80211_hdr *)((u8 *)rx_desc + rx_hl->fw_desc.len)
>    without checking that fw_desc.len does not exceed the remaining
>    skb length.
> 
> If the firmware sends a malformed fragment indication with a small
> payload, this can lead to an integer underflow in skb->len and an
> out-of-bounds read in hdr->addr1.
> 
> Fix this by adding the missing validation:
> - Validate skb->len before skb_pull.
> - Validate num_mpdu_ranges to be <= 1.
> - Validate tot_hdr_len against skb->len.
> - Validate fw_desc.len against remaining skb length.
> 
> This prevents out-of-bounds read if the firmware sends malformed data.

ok, at least this one has commit text. Did you write this since it looks a lot
like LLM-generated stuff we see. If you've used LLM you need to add an
Assisted-by tag

but also look at other commit text. you are putting in way too much detail.
describe the problem as if you are telling someone else how to fix the
problem. normally you would not quote specific lines of code that has a
problem, you'd just say ath10k_htt_rx_proc_rx_frag_ind_hl() doesn't handle
undersized packets, so fix that. The code diff itself tells us what the actual
changes are.

> 
> Signed-off-by: Rivaldi Hormat <rivaldihormat@gmail.com>
> ---
>  drivers/net/wireless/ath/ath10k/htt_rx.c | 30 ++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index ab2d373b4..4fabb9264 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -2775,6 +2775,13 @@ static bool ath10k_htt_rx_proc_rx_frag_ind_hl(struct ath10k_htt *htt,
>  	struct htt_resp *resp;
>  	size_t tot_hdr_len;
>  
> +
> +	/* FIX: Validate skb length before skb_pull */
> +	if (skb->len < HTT_RX_FRAG_IND_INFO0_HEADER_LEN) {
> +		ath10k_warn(ar, "Invalid skb len %d for RX_FRAG_IND\n", skb->len);
> +		return false;
> +	}
> +
>  	resp = (struct htt_resp *)(skb->data + HTT_RX_FRAG_IND_INFO0_HEADER_LEN);
>  	skb_pull(skb, HTT_RX_FRAG_IND_INFO0_HEADER_LEN);
>  	skb_trim(skb, skb->len - FCS_LEN);
> @@ -2792,6 +2799,13 @@ static bool ath10k_htt_rx_proc_rx_frag_ind_hl(struct ath10k_htt *htt,
>  	num_mpdu_ranges = MS(__le32_to_cpu(rx_hl->hdr.info1),
>  			     HTT_RX_INDICATION_INFO1_NUM_MPDU_RANGES);
>  
> +	/* FIX: Validate num_mpdu_ranges */
> +	if (num_mpdu_ranges > 1) {
> +		ath10k_warn(ar, "Invalid num_mpdu_ranges %d\n", num_mpdu_ranges);
> +		goto err;
> +	}
> +
> +
>  	tot_hdr_len = sizeof(struct htt_resp_hdr) +
>  		      sizeof(rx_hl->hdr) +
>  		      sizeof(rx_hl->ppdu) +
> @@ -2799,10 +2813,26 @@ static bool ath10k_htt_rx_proc_rx_frag_ind_hl(struct ath10k_htt *htt,
>  		      sizeof(rx_hl->fw_desc) +
>  		      sizeof(struct htt_rx_indication_mpdu_range) * num_mpdu_ranges;
>  
> +	/* FIX: Validate tot_hdr_len against skb length */
> +	if (tot_hdr_len > skb->len) {
> +		ath10k_warn(ar, "Invalid tot_hdr_len %zu > skb->len %u\n",
> +			    tot_hdr_len, skb->len);
> +		goto err;
> +	}
> +
> +
>  	tid =  MS(rx_hl->hdr.info0, HTT_RX_INDICATION_INFO0_EXT_TID);
>  	rx_desc = (struct htt_hl_rx_desc *)(skb->data + tot_hdr_len);
>  	rx_desc_info = __le32_to_cpu(rx_desc->info);
>  
> +
> +	/* FIX: Validate fw_desc.len against remaining skb length */
> +	if (rx_hl->fw_desc.len > skb->len - tot_hdr_len) {
> +		ath10k_warn(ar, "Invalid fw_desc.len %u > remaining skb len\n",
> +			    rx_hl->fw_desc.len);
> +		goto err;
> +	}
> +
>  	hdr = (struct ieee80211_hdr *)((u8 *)rx_desc + rx_hl->fw_desc.len);
>  
>  	if (is_multicast_ether_addr(hdr->addr1)) {