Re: [PATCH] wifi: ath11k: fix use after free in ath11k_dp_rx_msdu_coalesce.
Baochen Qiang <[email protected]>
| Newsgroups | org.infradead.lists.ath11k,org.kernel.vger.linux-wireless |
|---|---|
| Message-ID | <[email protected]> |
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 <[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... I also prefer a cached flag as it is much cleaner. > > /jeff >