[PATCH v2 1/2] wifi: mt76: mt7996: use spin_lock_irqsave for reg_lock

[PATCH v2 1/2] wifi: mt76: mt7996: use spin_lock_irqsave for reg_lock

[Valera Kozlov]

mt7996_rr()/wr()/rmw() and mt7996_memcpy_fromio() take dev->reg_lock
via spin_lock_bh() when accessing remapped registers. This is not
safe against hard-IRQ context, which can lead to race conditions and
kernel panics on some platforms.

MediaTek's own vendor tree independently found and fixed this same
issue, reporting it as causing AP panics via Asynchronous SError
Interrupt during normal WiFi operation:
https://github.com/mediatek/mtk-openwrt-feeds/commit/3edfc0cbf7dc666a0b051b54f51634d6ff2a3be6

Switch to spin_lock_irqsave()/spin_unlock_irqrestore() to make the
locking safe against hard-IRQ context.

Signed-off-by: Valera Kozlov <valera.kozlov19@gmail.com>
---
 .../net/wireless/mediatek/mt76/mt7996/mmio.c  | 20 +++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mmio.c b/drivers/net/wireless/mediatek/mt76/mt7996/mmio.c
index fbfbb96a742f..4e335e601581 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mmio.c
@@ -391,29 +391,31 @@ void mt7996_memcpy_fromio(struct mt7996_dev *dev, void *buf, u32 offset,
 			  size_t len)
 {
 	u32 addr = __mt7996_reg_addr(dev, offset);
+	unsigned long flags;
 
 	if (addr != INVALID_REG_ADDR) {
 		memcpy_fromio(buf, dev->mt76.mmio.regs + addr, len);
 		return;
 	}
 
-	spin_lock_bh(&dev->reg_lock);
+	spin_lock_irqsave(&dev->reg_lock, flags);
 	memcpy_fromio(buf, dev->mt76.mmio.regs +
 			   __mt7996_reg_remap_addr(dev, offset), len);
-	spin_unlock_bh(&dev->reg_lock);
+	spin_unlock_irqrestore(&dev->reg_lock, flags);
 }
 
 static u32 mt7996_rr(struct mt76_dev *mdev, u32 offset)
 {
 	struct mt7996_dev *dev = container_of(mdev, struct mt7996_dev, mt76);
 	u32 addr = __mt7996_reg_addr(dev, offset), val;
+	unsigned long flags;
 
 	if (addr != INVALID_REG_ADDR)
 		return dev->bus_ops->rr(mdev, addr);
 
-	spin_lock_bh(&dev->reg_lock);
+	spin_lock_irqsave(&dev->reg_lock, flags);
 	val = dev->bus_ops->rr(mdev, __mt7996_reg_remap_addr(dev, offset));
-	spin_unlock_bh(&dev->reg_lock);
+	spin_unlock_irqrestore(&dev->reg_lock, flags);
 
 	return val;
 }
@@ -422,28 +424,30 @@ static void mt7996_wr(struct mt76_dev *mdev, u32 offset, u32 val)
 {
 	struct mt7996_dev *dev = container_of(mdev, struct mt7996_dev, mt76);
 	u32 addr = __mt7996_reg_addr(dev, offset);
+	unsigned long flags;
 
 	if (addr != INVALID_REG_ADDR) {
 		dev->bus_ops->wr(mdev, addr, val);
 		return;
 	}
 
-	spin_lock_bh(&dev->reg_lock);
+	spin_lock_irqsave(&dev->reg_lock, flags);
 	dev->bus_ops->wr(mdev, __mt7996_reg_remap_addr(dev, offset), val);
-	spin_unlock_bh(&dev->reg_lock);
+	spin_unlock_irqrestore(&dev->reg_lock, flags);
 }
 
 static u32 mt7996_rmw(struct mt76_dev *mdev, u32 offset, u32 mask, u32 val)
 {
 	struct mt7996_dev *dev = container_of(mdev, struct mt7996_dev, mt76);
 	u32 addr = __mt7996_reg_addr(dev, offset);
+	unsigned long flags;
 
 	if (addr != INVALID_REG_ADDR)
 		return dev->bus_ops->rmw(mdev, addr, mask, val);
 
-	spin_lock_bh(&dev->reg_lock);
+	spin_lock_irqsave(&dev->reg_lock, flags);
 	val = dev->bus_ops->rmw(mdev, __mt7996_reg_remap_addr(dev, offset), mask, val);
-	spin_unlock_bh(&dev->reg_lock);
+	spin_unlock_irqrestore(&dev->reg_lock, flags);
 
 	return val;
 }
-- 
2.53.0

[PATCH v2 2/2] wifi: mt76: mt7996: route disassoc frames to ALTXQ

[Valera Kozlov]

If AP sends deauth or disassoc to a peer station while the station is
in power-save mode, the frame may queue in the normal ACQ and lead to
PLE hang, since the station won't wake up to receive it.

Deauth frames are already routed to MT_TXQ_PSD / ALTX0. Extend the
same handling to disassoc frames, and additionally route any frame to
a station that hasn't completed authorization (tx_info without
MT_WCID_TX_INFO_SET) to ALTX0 in mt7996_mac_write_txwi(), clearing the
flag again on MT76_STA_EVENT_DISASSOC so it gets re-armed on the next
association.

MediaTek's vendor tree has an equivalent fix, though written against
a newer MLO-aware version of this code that doesn't apply directly to
this tree:
https://github.com/mediatek/mtk-openwrt-feeds/commit/ed6924828479aaeadbb7c402ec390df26dcbd8bc

Signed-off-by: Valera Kozlov <valera.kozlov19@gmail.com>
---
 drivers/net/wireless/mediatek/mt76/mt7996/mac.c  | 2 +-
 drivers/net/wireless/mediatek/mt76/mt7996/main.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
index a9558381ad91..decf6ea15777 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
@@ -932,7 +932,7 @@ void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
 	} else if (beacon) {
 		p_fmt = MT_TX_TYPE_FW;
 		q_idx = MT_LMAC_BCN0;
-	} else if (qid >= MT_TXQ_PSD) {
+	} else if (qid >= MT_TXQ_PSD || (!(wcid->tx_info & MT_WCID_TX_INFO_SET))) {
 		p_fmt = MT_TX_TYPE_CT;
 		q_idx = MT_LMAC_ALTX0;
 	} else {
diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/main.c b/drivers/net/wireless/mediatek/mt76/mt7996/main.c
index dd7a01c02a7c..32ba5cbe5062 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/main.c
@@ -1422,6 +1422,7 @@ mt7996_mac_sta_event(struct mt7996_dev *dev, struct ieee80211_vif *vif,
 				goto unlock;
 			break;
 		case MT76_STA_EVENT_DISASSOC:
+			msta_link->wcid.tx_info &= ~MT_WCID_TX_INFO_SET;
 			for (i = 0; i < ARRAY_SIZE(msta_link->twt.flow); i++)
 				mt7996_mac_twt_teardown_flow(dev, link,
 							     msta_link, i);
-- 
2.53.0

Re: [PATCH v2 2/2] wifi: mt76: mt7996: route disassoc frames to ALTXQ

[Valera Kozlov]

Hi Felix,

While rebasing this against mt76-fixes as requested, I noticed
ece175a8a7af ("wifi: mt76: use ALTX queue for packets to disassociated
stations") already covers the same problem via MT_WCID_TX_INFO_SET,
and with broader scope than my patch (any station that hasn't
completed authorization, not just the disassoc-frame case I was
checking for).

After rebasing, my patch's diff against tx.c and mac.c came out empty,
and the one remaining line in mt7996/main.c was a redundant second
clear of the same flag already cleared by your commit a few lines
below.

I'm withdrawing this patch (2/2) as superseded by ece175a8a7af.
Patch 1/2 (spin_lock_irqsave for reg_lock) is unrelated and still
stands.

Thanks,
Valera