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

Jeff Johnson <[email protected]>
Newsgroups org.infradead.lists.ath11k,org.kernel.vger.linux-wireless
Message-ID <[email protected]>
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 <[email protected]>
> ---
>  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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.