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

[Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>]

On 5/5/2026 8:38 PM, Jeff Johnson wrote:
> 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...


IMO a cached flag is a little easier to read and harder to regress.

Also please add a Fixes tag. This appears to go back to the initial 
ath11k commit.

Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")


--
Ramesh