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

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

[Willmar Knikker]

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);
 	}
 
 	return 0;
-- 
2.54.0

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

[Jeff Johnson]

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

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

[Rameshkumar Sundaram]

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

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

[Willmar Knikker]

On Tue, 05 May 2026 08:08:48 -0700
"Jeff Johnson" <jeff.johnson@oss.qualcomm.com> wrote:

> 
> 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

imo, i would agree that would be cleaner.
But as this is my first patch i did not want to make to big of a change
so reusing the pattern from above felt te safest.

> This issue is present since day 1...
> 
> Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")

Should i make v2 commit to add that tag? 

Thanks for taking the time.

	--- Willmar K.

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

[Baochen Qiang]

On 5/5/2026 11:08 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...

I also prefer a cached flag as it is much cleaner.

> 
> /jeff
> 

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

[Jeff Johnson]

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>

This issue is present since day 1...

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