Re: [PATCH] wifi: ath11k: fix use after free in ath11k_dp_rx_msdu_coalesce.

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

On 5/5/2026 7:30 AM, Willmar Knikker wrote:
> In ath11k_dp_rx_msdu_coalesce the loop uses ->is_continuation after
> the dev_kfree_skb_any. This can cause a use after free kfence.
> 
> Move the use after the dev_kfree_skb_any after use of ->is_continuation
> inline with the while in the while loop above this one.
> 
> Signed-off-by: Willmar Knikker <willmar@met-dubbel-l.nl>
> ---
>  drivers/net/wireless/ath/ath11k/dp_rx.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
> index fe79109adc70..02bd9787d6b4 100644
> --- a/drivers/net/wireless/ath/ath11k/dp_rx.c
> +++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
> @@ -1825,11 +1825,12 @@ static int ath11k_dp_rx_msdu_coalesce(struct ath11k *ar,
>  		skb_pull(skb, hal_rx_desc_sz);
>  		skb_copy_from_linear_data(skb, skb_put(first, buf_len),
>  					  buf_len);
> -		dev_kfree_skb_any(skb);
> -
>  		rem_len -= buf_len;
> -		if (!rxcb->is_continuation)
> +		if (!rxcb->is_continuation) {
> +			dev_kfree_skb_any(skb);
>  			break;
> +		}
> +		dev_kfree_skb_any(skb);

rather than repeating code imo it would be "better" to cache the flag before
freeing and then test the cached flag.

however as you note this proposed logic matches the logic already present in
the "Free up all buffers of the MSDU" portion of the function, so from that
perspective the proposal is consistent with that logic.

let's see if anyone else has an opinion...

/jeff