[PATCH] ath10k: fix wmi mgmt tx queue full due to race condition

[Kalle Valo]

Brian Norris <briannorris at chromium.org> writes:

> Hi,
>
> On Sun, Dec 20, 2020 at 5:53 PM Miaoqing Pan <miaoqing at codeaurora.org> wrote:
>>
>> Failed to transmit wmi management frames:
>>
>> [84977.840894] ath10k_snoc a000000.wifi: wmi mgmt tx queue is full
>> [84977.840913] ath10k_snoc a000000.wifi: failed to transmit packet, dropping: -28
>> [84977.840924] ath10k_snoc a000000.wifi: failed to submit frame: -28
>> [84977.840932] ath10k_snoc a000000.wifi: failed to transmit frame: -28
>>
>> This issue is caused by race condition between skb_dequeue and
>> __skb_queue_tail. The queue of ‘wmi_mgmt_tx_queue’ is protected by a
>> different lock: ar->data_lock vs list->lock, the result is no protection.
>
> Nice catch!
>
>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>> @@ -3763,23 +3763,16 @@ bool ath10k_mac_tx_frm_has_freq(struct ath10k *ar)
>>  static int ath10k_mac_tx_wmi_mgmt(struct ath10k *ar, struct sk_buff *skb)
>>  {
>>         struct sk_buff_head *q = &ar->wmi_mgmt_tx_queue;
>> -       int ret = 0;
>> -
>> -       spin_lock_bh(&ar->data_lock);
>>
>>         if (skb_queue_len(q) == ATH10K_MAX_NUM_MGMT_PENDING) {
>
> I believe you should be switching this to use skb_queue_len_lockless()
> too.

Why lockless? (reads documentation) Ah, is it due to memory
synchronisation now that we don't take the data_lock anymore?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches