Skip to content

Commit 0335c03

Browse files
nbd168nbd168
committed
wifi: mt76: fix race condition related to checking tx queue fill status
When drv_tx calls race against local tx scheduling, the queue fill status checks can potentially race, leading to dma queue entries being overwritten. Fix this by deferring packets from drv_tx calls to the tx worker, in order to ensure that all regular queue tx comes from the same context. Reported-by: Ryder Lee <Ryder.Lee@mediatek.com> Signed-off-by: Felix Fietkau <nbd@nbd.name>
1 parent debd133 commit 0335c03

File tree

10 files changed

+155
-51
lines changed

10 files changed

+155
-51
lines changed

‎drivers/net/wireless/mediatek/mt76/mac80211.c‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,9 @@ mt76_phy_init(struct mt76_phy *phy, struct ieee80211_hw *hw)
415415
struct mt76_dev *dev = phy->dev;
416416
struct wiphy *wiphy = hw->wiphy;
417417

418+
INIT_LIST_HEAD(&phy->tx_list);
419+
spin_lock_init(&phy->tx_lock);
420+
418421
SET_IEEE80211_DEV(hw, dev->dev);
419422
SET_IEEE80211_PERM_ADDR(hw, phy->macaddr);
420423

@@ -689,6 +692,7 @@ int mt76_register_device(struct mt76_dev *dev, bool vht,
689692
int ret;
690693

691694
dev_set_drvdata(dev->dev, dev);
695+
mt76_wcid_init(&dev->global_wcid);
692696
ret = mt76_phy_init(phy, hw);
693697
if (ret)
694698
return ret;
@@ -744,6 +748,7 @@ void mt76_unregister_device(struct mt76_dev *dev)
744748
if (IS_ENABLED(CONFIG_MT76_LEDS))
745749
mt76_led_cleanup(&dev->phy);
746750
mt76_tx_status_check(dev, true);
751+
mt76_wcid_cleanup(dev, &dev->global_wcid);
747752
ieee80211_unregister_hw(hw);
748753
}
749754
EXPORT_SYMBOL_GPL(mt76_unregister_device);
@@ -1412,7 +1417,7 @@ mt76_sta_add(struct mt76_phy *phy, struct ieee80211_vif *vif,
14121417
wcid->phy_idx = phy->band_idx;
14131418
rcu_assign_pointer(dev->wcid[wcid->idx], wcid);
14141419

1415-
mt76_packet_id_init(wcid);
1420+
mt76_wcid_init(wcid);
14161421
out:
14171422
mutex_unlock(&dev->mutex);
14181423

@@ -1431,7 +1436,7 @@ void __mt76_sta_remove(struct mt76_dev *dev, struct ieee80211_vif *vif,
14311436
if (dev->drv->sta_remove)
14321437
dev->drv->sta_remove(dev, vif, sta);
14331438

1434-
mt76_packet_id_flush(dev, wcid);
1439+
mt76_wcid_cleanup(dev, wcid);
14351440

14361441
mt76_wcid_mask_clear(dev->wcid_mask, idx);
14371442
mt76_wcid_mask_clear(dev->wcid_phy_mask, idx);
@@ -1487,6 +1492,47 @@ void mt76_sta_pre_rcu_remove(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
14871492
}
14881493
EXPORT_SYMBOL_GPL(mt76_sta_pre_rcu_remove);
14891494

1495+
void mt76_wcid_init(struct mt76_wcid *wcid)
1496+
{
1497+
INIT_LIST_HEAD(&wcid->tx_list);
1498+
skb_queue_head_init(&wcid->tx_pending);
1499+
1500+
INIT_LIST_HEAD(&wcid->list);
1501+
idr_init(&wcid->pktid);
1502+
}
1503+
EXPORT_SYMBOL_GPL(mt76_wcid_init);
1504+
1505+
void mt76_wcid_cleanup(struct mt76_dev *dev, struct mt76_wcid *wcid)
1506+
{
1507+
struct mt76_phy *phy = dev->phys[wcid->phy_idx];
1508+
struct ieee80211_hw *hw;
1509+
struct sk_buff_head list;
1510+
struct sk_buff *skb;
1511+
1512+
mt76_tx_status_lock(dev, &list);
1513+
mt76_tx_status_skb_get(dev, wcid, -1, &list);
1514+
mt76_tx_status_unlock(dev, &list);
1515+
1516+
idr_destroy(&wcid->pktid);
1517+
1518+
spin_lock_bh(&phy->tx_lock);
1519+
1520+
if (!list_empty(&wcid->tx_list))
1521+
list_del_init(&wcid->tx_list);
1522+
1523+
spin_lock(&wcid->tx_pending.lock);
1524+
skb_queue_splice_tail_init(&wcid->tx_pending, &list);
1525+
spin_unlock(&wcid->tx_pending.lock);
1526+
1527+
spin_unlock_bh(&phy->tx_lock);
1528+
1529+
while ((skb = __skb_dequeue(&list)) != NULL) {
1530+
hw = mt76_tx_status_get_hw(dev, skb);
1531+
ieee80211_free_txskb(hw, skb);
1532+
}
1533+
}
1534+
EXPORT_SYMBOL_GPL(mt76_wcid_cleanup);
1535+
14901536
int mt76_get_txpower(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
14911537
int *dbm)
14921538
{

‎drivers/net/wireless/mediatek/mt76/mt76.h‎

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,9 @@ struct mt76_wcid {
334334
u32 tx_info;
335335
bool sw_iv;
336336

337+
struct list_head tx_list;
338+
struct sk_buff_head tx_pending;
339+
337340
struct list_head list;
338341
struct idr pktid;
339342

@@ -719,6 +722,8 @@ struct mt76_phy {
719722
unsigned long state;
720723
u8 band_idx;
721724

725+
spinlock_t tx_lock;
726+
struct list_head tx_list;
722727
struct mt76_queue *q_tx[__MT_TXQ_MAX];
723728

724729
struct cfg80211_chan_def chandef;
@@ -1598,22 +1603,7 @@ mt76_token_put(struct mt76_dev *dev, int token)
15981603
return txwi;
15991604
}
16001605

1601-
static inline void mt76_packet_id_init(struct mt76_wcid *wcid)
1602-
{
1603-
INIT_LIST_HEAD(&wcid->list);
1604-
idr_init(&wcid->pktid);
1605-
}
1606-
1607-
static inline void
1608-
mt76_packet_id_flush(struct mt76_dev *dev, struct mt76_wcid *wcid)
1609-
{
1610-
struct sk_buff_head list;
1611-
1612-
mt76_tx_status_lock(dev, &list);
1613-
mt76_tx_status_skb_get(dev, wcid, -1, &list);
1614-
mt76_tx_status_unlock(dev, &list);
1615-
1616-
idr_destroy(&wcid->pktid);
1617-
}
1606+
void mt76_wcid_init(struct mt76_wcid *wcid);
1607+
void mt76_wcid_cleanup(struct mt76_dev *dev, struct mt76_wcid *wcid);
16181608

16191609
#endif

‎drivers/net/wireless/mediatek/mt76/mt7603/main.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ mt7603_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
7070
mvif->sta.wcid.idx = idx;
7171
mvif->sta.wcid.hw_key_idx = -1;
7272
mvif->sta.vif = mvif;
73-
mt76_packet_id_init(&mvif->sta.wcid);
73+
mt76_wcid_init(&mvif->sta.wcid);
7474

7575
eth_broadcast_addr(bc_addr);
7676
mt7603_wtbl_init(dev, idx, mvif->idx, bc_addr);
@@ -110,7 +110,7 @@ mt7603_remove_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
110110
dev->mt76.vif_mask &= ~BIT_ULL(mvif->idx);
111111
mutex_unlock(&dev->mt76.mutex);
112112

113-
mt76_packet_id_flush(&dev->mt76, &mvif->sta.wcid);
113+
mt76_wcid_cleanup(&dev->mt76, &mvif->sta.wcid);
114114
}
115115

116116
void mt7603_init_edcca(struct mt7603_dev *dev)

‎drivers/net/wireless/mediatek/mt76/mt7615/main.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ static int mt7615_add_interface(struct ieee80211_hw *hw,
226226
mvif->sta.wcid.idx = idx;
227227
mvif->sta.wcid.phy_idx = mvif->mt76.band_idx;
228228
mvif->sta.wcid.hw_key_idx = -1;
229-
mt76_packet_id_init(&mvif->sta.wcid);
229+
mt76_wcid_init(&mvif->sta.wcid);
230230

231231
mt7615_mac_wtbl_update(dev, idx,
232232
MT_WTBL_UPDATE_ADM_COUNT_CLEAR);
@@ -279,7 +279,7 @@ static void mt7615_remove_interface(struct ieee80211_hw *hw,
279279
list_del_init(&msta->wcid.poll_list);
280280
spin_unlock_bh(&dev->mt76.sta_poll_lock);
281281

282-
mt76_packet_id_flush(&dev->mt76, &mvif->sta.wcid);
282+
mt76_wcid_cleanup(&dev->mt76, &mvif->sta.wcid);
283283
}
284284

285285
int mt7615_set_channel(struct mt7615_phy *phy)

‎drivers/net/wireless/mediatek/mt76/mt76x02_util.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ mt76x02_vif_init(struct mt76x02_dev *dev, struct ieee80211_vif *vif,
288288
mvif->idx = idx;
289289
mvif->group_wcid.idx = MT_VIF_WCID(idx);
290290
mvif->group_wcid.hw_key_idx = -1;
291-
mt76_packet_id_init(&mvif->group_wcid);
291+
mt76_wcid_init(&mvif->group_wcid);
292292

293293
mtxq = (struct mt76_txq *)vif->txq->drv_priv;
294294
rcu_assign_pointer(dev->mt76.wcid[MT_VIF_WCID(idx)], &mvif->group_wcid);
@@ -346,7 +346,7 @@ void mt76x02_remove_interface(struct ieee80211_hw *hw,
346346

347347
dev->mt76.vif_mask &= ~BIT_ULL(mvif->idx);
348348
rcu_assign_pointer(dev->mt76.wcid[mvif->group_wcid.idx], NULL);
349-
mt76_packet_id_flush(&dev->mt76, &mvif->group_wcid);
349+
mt76_wcid_cleanup(&dev->mt76, &mvif->group_wcid);
350350
}
351351
EXPORT_SYMBOL_GPL(mt76x02_remove_interface);
352352

‎drivers/net/wireless/mediatek/mt76/mt7915/main.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ static int mt7915_add_interface(struct ieee80211_hw *hw,
253253
mvif->sta.wcid.phy_idx = ext_phy;
254254
mvif->sta.wcid.hw_key_idx = -1;
255255
mvif->sta.wcid.tx_info |= MT_WCID_TX_INFO_SET;
256-
mt76_packet_id_init(&mvif->sta.wcid);
256+
mt76_wcid_init(&mvif->sta.wcid);
257257

258258
mt7915_mac_wtbl_update(dev, idx,
259259
MT_WTBL_UPDATE_ADM_COUNT_CLEAR);
@@ -314,7 +314,7 @@ static void mt7915_remove_interface(struct ieee80211_hw *hw,
314314
list_del_init(&msta->wcid.poll_list);
315315
spin_unlock_bh(&dev->mt76.sta_poll_lock);
316316

317-
mt76_packet_id_flush(&dev->mt76, &msta->wcid);
317+
mt76_wcid_cleanup(&dev->mt76, &msta->wcid);
318318
}
319319

320320
int mt7915_set_channel(struct mt7915_phy *phy)

‎drivers/net/wireless/mediatek/mt76/mt7921/main.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ mt7921_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
315315
mvif->sta.wcid.phy_idx = mvif->mt76.band_idx;
316316
mvif->sta.wcid.hw_key_idx = -1;
317317
mvif->sta.wcid.tx_info |= MT_WCID_TX_INFO_SET;
318-
mt76_packet_id_init(&mvif->sta.wcid);
318+
mt76_wcid_init(&mvif->sta.wcid);
319319

320320
mt7921_mac_wtbl_update(dev, idx,
321321
MT_WTBL_UPDATE_ADM_COUNT_CLEAR);

‎drivers/net/wireless/mediatek/mt76/mt792x_core.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ void mt792x_remove_interface(struct ieee80211_hw *hw,
115115
list_del_init(&msta->wcid.poll_list);
116116
spin_unlock_bh(&dev->mt76.sta_poll_lock);
117117

118-
mt76_packet_id_flush(&dev->mt76, &msta->wcid);
118+
mt76_wcid_cleanup(&dev->mt76, &msta->wcid);
119119
}
120120
EXPORT_SYMBOL_GPL(mt792x_remove_interface);
121121

‎drivers/net/wireless/mediatek/mt76/mt7996/main.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ static int mt7996_add_interface(struct ieee80211_hw *hw,
207207
mvif->sta.wcid.phy_idx = band_idx;
208208
mvif->sta.wcid.hw_key_idx = -1;
209209
mvif->sta.wcid.tx_info |= MT_WCID_TX_INFO_SET;
210-
mt76_packet_id_init(&mvif->sta.wcid);
210+
mt76_wcid_init(&mvif->sta.wcid);
211211

212212
mt7996_mac_wtbl_update(dev, idx,
213213
MT_WTBL_UPDATE_ADM_COUNT_CLEAR);
@@ -268,7 +268,7 @@ static void mt7996_remove_interface(struct ieee80211_hw *hw,
268268
list_del_init(&msta->wcid.poll_list);
269269
spin_unlock_bh(&dev->mt76.sta_poll_lock);
270270

271-
mt76_packet_id_flush(&dev->mt76, &msta->wcid);
271+
mt76_wcid_cleanup(&dev->mt76, &msta->wcid);
272272
}
273273

274274
int mt7996_set_channel(struct mt7996_phy *phy)

‎drivers/net/wireless/mediatek/mt76/tx.c‎

Lines changed: 88 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -329,40 +329,32 @@ void
329329
mt76_tx(struct mt76_phy *phy, struct ieee80211_sta *sta,
330330
struct mt76_wcid *wcid, struct sk_buff *skb)
331331
{
332-
struct mt76_dev *dev = phy->dev;
333332
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
334-
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
335-
struct mt76_queue *q;
336-
int qid = skb_get_queue_mapping(skb);
337333

338334
if (mt76_testmode_enabled(phy)) {
339335
ieee80211_free_txskb(phy->hw, skb);
340336
return;
341337
}
342338

343-
if (WARN_ON(qid >= MT_TXQ_PSD)) {
344-
qid = MT_TXQ_BE;
345-
skb_set_queue_mapping(skb, qid);
346-
}
347-
348-
if ((dev->drv->drv_flags & MT_DRV_HW_MGMT_TXQ) &&
349-
!(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) &&
350-
!ieee80211_is_data(hdr->frame_control) &&
351-
!ieee80211_is_bufferable_mmpdu(skb)) {
352-
qid = MT_TXQ_PSD;
353-
}
339+
if (WARN_ON(skb_get_queue_mapping(skb) >= MT_TXQ_PSD))
340+
skb_set_queue_mapping(skb, MT_TXQ_BE);
354341

355342
if (wcid && !(wcid->tx_info & MT_WCID_TX_INFO_SET))
356343
ieee80211_get_tx_rates(info->control.vif, sta, skb,
357344
info->control.rates, 1);
358345

359346
info->hw_queue |= FIELD_PREP(MT_TX_HW_QUEUE_PHY, phy->band_idx);
360-
q = phy->q_tx[qid];
361347

362-
spin_lock_bh(&q->lock);
363-
__mt76_tx_queue_skb(phy, qid, skb, wcid, sta, NULL);
364-
dev->queue_ops->kick(dev, q);
365-
spin_unlock_bh(&q->lock);
348+
spin_lock_bh(&wcid->tx_pending.lock);
349+
__skb_queue_tail(&wcid->tx_pending, skb);
350+
spin_unlock_bh(&wcid->tx_pending.lock);
351+
352+
spin_lock_bh(&phy->tx_lock);
353+
if (list_empty(&wcid->tx_list))
354+
list_add_tail(&wcid->tx_list, &phy->tx_list);
355+
spin_unlock_bh(&phy->tx_lock);
356+
357+
mt76_worker_schedule(&phy->dev->tx_worker);
366358
}
367359
EXPORT_SYMBOL_GPL(mt76_tx);
368360

@@ -593,10 +585,86 @@ void mt76_txq_schedule(struct mt76_phy *phy, enum mt76_txq_id qid)
593585
}
594586
EXPORT_SYMBOL_GPL(mt76_txq_schedule);
595587

588+
static int
589+
mt76_txq_schedule_pending_wcid(struct mt76_phy *phy, struct mt76_wcid *wcid)
590+
{
591+
struct mt76_dev *dev = phy->dev;
592+
struct ieee80211_sta *sta;
593+
struct mt76_queue *q;
594+
struct sk_buff *skb;
595+
int ret = 0;
596+
597+
spin_lock(&wcid->tx_pending.lock);
598+
while ((skb = skb_peek(&wcid->tx_pending)) != NULL) {
599+
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
600+
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
601+
int qid = skb_get_queue_mapping(skb);
602+
603+
if ((dev->drv->drv_flags & MT_DRV_HW_MGMT_TXQ) &&
604+
!(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) &&
605+
!ieee80211_is_data(hdr->frame_control) &&
606+
!ieee80211_is_bufferable_mmpdu(skb))
607+
qid = MT_TXQ_PSD;
608+
609+
q = phy->q_tx[qid];
610+
if (mt76_txq_stopped(q)) {
611+
ret = -1;
612+
break;
613+
}
614+
615+
__skb_unlink(skb, &wcid->tx_pending);
616+
spin_unlock(&wcid->tx_pending.lock);
617+
618+
sta = wcid_to_sta(wcid);
619+
spin_lock(&q->lock);
620+
__mt76_tx_queue_skb(phy, qid, skb, wcid, sta, NULL);
621+
dev->queue_ops->kick(dev, q);
622+
spin_unlock(&q->lock);
623+
624+
spin_lock(&wcid->tx_pending.lock);
625+
}
626+
spin_unlock(&wcid->tx_pending.lock);
627+
628+
return ret;
629+
}
630+
631+
static void mt76_txq_schedule_pending(struct mt76_phy *phy)
632+
{
633+
if (list_empty(&phy->tx_list))
634+
return;
635+
636+
local_bh_disable();
637+
rcu_read_lock();
638+
639+
spin_lock(&phy->tx_lock);
640+
while (!list_empty(&phy->tx_list)) {
641+
struct mt76_wcid *wcid = NULL;
642+
int ret;
643+
644+
wcid = list_first_entry(&phy->tx_list, struct mt76_wcid, tx_list);
645+
list_del_init(&wcid->tx_list);
646+
647+
spin_unlock(&phy->tx_lock);
648+
ret = mt76_txq_schedule_pending_wcid(phy, wcid);
649+
spin_lock(&phy->tx_lock);
650+
651+
if (ret) {
652+
if (list_empty(&wcid->tx_list))
653+
list_add_tail(&wcid->tx_list, &phy->tx_list);
654+
break;
655+
}
656+
}
657+
spin_unlock(&phy->tx_lock);
658+
659+
rcu_read_unlock();
660+
local_bh_enable();
661+
}
662+
596663
void mt76_txq_schedule_all(struct mt76_phy *phy)
597664
{
598665
int i;
599666

667+
mt76_txq_schedule_pending(phy);
600668
for (i = 0; i <= MT_TXQ_BK; i++)
601669
mt76_txq_schedule(phy, i);
602670
}

0 commit comments

Comments
 (0)