Re: [PATCH 4/5] mac80211: run late dequeue late tx handlers without holding fq->lock

[Wen Gong <quic_wgong@quicinc.com>]

I will send a patch for it to avoid the potential user-after-free risk.

On 12/7/2022 2:30 PM, Wen Gong wrote:
> Hi Johannes,
>
> do you know it?
>
> On 12/5/2022 5:46 PM, Wen Gong wrote:
>> On 3/17/2019 1:06 AM, Felix Fietkau wrote:
>>> Reduces lock contention on enqueue/dequeue of iTXQ packets
>>>
>>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>>> ---
>>>   net/mac80211/tx.c | 10 ++++++++--
>>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
>>> index 8127e43e12b1..f85344c9af62 100644
>>> --- a/net/mac80211/tx.c
>>> +++ b/net/mac80211/tx.c
>>> @@ -3544,6 +3544,7 @@ struct sk_buff *ieee80211_tx_dequeue(struct 
>>> ieee80211_hw *hw,
>>>       ieee80211_tx_result r;
>>>       struct ieee80211_vif *vif = txq->vif;
>>>   +begin:
>>>       spin_lock_bh(&fq->lock);
>> Maybe use-after-free will happened?
>>
>> You can see ieee80211_tx_dequeue() in tx.c as below, after 
>> ieee80211_free_txskb(), it will goto begin,
>> If goto out happened in below check, then the skb which is freed will 
>> be returned, and use-after-free will happen.
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/tree/net/mac80211/tx.c?id=ded4698b58cb23c22b0dcbd829ced19ce4e6ce02#n3538 
>>
>> begin:
>>     spin_lock_bh(&fq->lock);
>>
>>     if (test_bit(IEEE80211_TXQ_STOP, &txqi->flags) ||
>>         test_bit(IEEE80211_TXQ_STOP_NETIF_TX, &txqi->flags))
>>         goto out;
>>
>>     if (vif->txqs_stopped[ieee80211_ac_from_tid(txq->tid)]) {
>>         set_bit(IEEE80211_TXQ_STOP_NETIF_TX, &txqi->flags);
>>         goto out;
>>     }
>>
>>     /* Make sure fragments stay together. */
>>     skb = __skb_dequeue(&txqi->frags);
>>     if (skb)
>>         goto out;
>>
>>     skb = fq_tin_dequeue(fq, tin, fq_tin_dequeue_func);
>>     if (!skb)
>>         goto out;
>>
>>     spin_unlock_bh(&fq->lock);
>>
>> Maybe "skb = NULL;" should be added after "begin:".
>>
>> ...
>>