wifi: mt76: fix potential deadlock caused by rx_lock

PATCH In review Shayne Chen 2 participants 10 messages Feb 2

Subsystems (2)

Arm MediaTek MediaTek MT76 wireless LAN driver

lore

Share & tools

lore

Shayne Chen

author reviewer

2026-02-02 07:54 UTC #0 lore raw

PATCH 1/5 wifi: mt76: fix potential deadlock caused by rx_lock

A deadlock will occur if both of the following conditions are met, because they each attempt to acquire the rx_lock:
- mac80211 receives an unexpected BAR control frame, which triggers a BA deletion
- A transmission failure happens due to an abnormality in DMA

Since ieee80211_tx_status_ext() is primarily used to address AQL issues, avoid potential deadlocks by restricting calls to ieee80211_tx_status_ext only for data frames.

Fixes 94e4f5794627 ("mt76: dma: use ieee80211_tx_status_ext to free packets when tx fails")

Reviewed-by Money Wang <[email protected]>

Signed-off-by Shayne Chen <[email protected]>

---
 drivers/net/wireless/mediatek/mt76/dma.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

drivers/net/wireless/mediatek/mt76/dma.c +12 -3

index 2d133ace7c33..928709f4a206 100644
@@ -648,6 +648,8 @@ mt76_dma_tx_queue_skb(struct mt76_phy *phy, struct mt76_queue *q,
 		.skb = skb,
 	};
 	struct mt76_dev *dev = phy->dev;
+	struct ieee80211_tx_info *info;
+	struct ieee80211_hdr *hdr;
 	struct ieee80211_hw *hw;
 	int len, n = 0, ret = -ENOMEM;
 	struct mt76_txwi_cache *t;

@@ -736,9 +738,16 @@ mt76_dma_tx_queue_skb(struct mt76_phy *phy, struct mt76_queue *q,
 free_skb:
 	status.skb = tx_info.skb;
 	hw = mt76_tx_status_get_hw(dev, tx_info.skb);
-	spin_lock_bh(&dev->rx_lock);
-	ieee80211_tx_status_ext(hw, &status);
-	spin_unlock_bh(&dev->rx_lock);
+	hdr = (struct ieee80211_hdr *)tx_info.skb->data;
+	info = IEEE80211_SKB_CB(tx_info.skb);
+	if ((info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) ||
+	    ieee80211_is_data(hdr->frame_control)) {
+		spin_lock_bh(&dev->rx_lock);
+		ieee80211_tx_status_ext(hw, &status);
+		spin_unlock_bh(&dev->rx_lock);
+	} else {
+		ieee80211_free_txskb(hw, tx_info.skb);
+	}
 
 	return ret;
 }

Signature · 1 line

2.51.0

Shayne Chen

author reviewer

2026-02-02 07:54 UTC #1 lore raw

PATCH 2/5 wifi: mt76: mt7996: fix wrong DMAD length when using MAC TXP

The struct mt76_connac_fw_txp is used for HIF TXP. Change to use the struct mt76_connac_hw_txp to fix the wrong DMAD length for MAC TXP.

Fixes cb6ebbdffef2 ("wifi: mt76: mt7996: support writing MAC TXD for AddBA Request")

Signed-off-by Shayne Chen <[email protected]>

---
 drivers/net/wireless/mediatek/mt76/mt7996/mac.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/net/wireless/mediatek/mt76/mt7996/mac.c +4 -2

index 77a036ac043c..3b09eff216c3 100644
@@ -1099,10 +1099,10 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
 	 * req
 	 */
 	if (le32_to_cpu(ptr[7]) & MT_TXD7_MAC_TXD) {
-		u32 val;
+		u32 val, mac_txp_size = sizeof(struct mt76_connac_hw_txp);
 
 		ptr = (__le32 *)(txwi + MT_TXD_SIZE);
-		memset((void *)ptr, 0, sizeof(struct mt76_connac_fw_txp));
+		memset((void *)ptr, 0, mac_txp_size);
 
 		val = FIELD_PREP(MT_TXP0_TOKEN_ID0, id) |
 		      MT_TXP0_TOKEN_ID0_VALID_MASK;

@@ -1121,6 +1121,8 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
 				  tx_info->buf[1].addr >> 32);
 #endif
 		ptr[3] = cpu_to_le32(val);
+
+		tx_info->buf[0].len = MT_TXD_SIZE + mac_txp_size;
 	} else {
 		struct mt76_connac_txp_common *txp;

Signature · 1 line

2.51.0

Shayne Chen

author reviewer

2026-02-02 07:54 UTC #2 lore raw

PATCH 3/5 wifi: mt76: mt7996: fix struct mt7996_mcu_uni_event

In reply to Shayne Chen

From: StanleyYP Wang <[email protected]>

The cid field is defined as a two-byte value in the firmware.

Fixes 98686cd21624 ("wifi: mt76: mt7996: add driver for MediaTek Wi-Fi 7 (802.11be) devices")

Signed-off-by StanleyYP Wang <[email protected]>

Signed-off-by Shayne Chen <[email protected]>

---
 drivers/net/wireless/mediatek/mt76/mt7996/mcu.c | 2 +-
 drivers/net/wireless/mediatek/mt76/mt7996/mcu.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

drivers/net/wireless/mediatek/mt76/mt7996/mcu.c +1 -1

index 8e1c8e1d6a99..285cd83e7117 100644
@@ -242,7 +242,7 @@ mt7996_mcu_parse_response(struct mt76_dev *mdev, int cmd,
 		event = (struct mt7996_mcu_uni_event *)skb->data;
 		ret = le32_to_cpu(event->status);
 		/* skip invalid event */
-		if (mcu_cmd != event->cid)
+		if (mcu_cmd != le16_to_cpu(event->cid))
 			ret = -EAGAIN;
 	} else {
 		skb_pull(skb, sizeof(struct mt7996_mcu_rxd));

drivers/net/wireless/mediatek/mt76/mt7996/mcu.h +2 -2

index d9fb49f7b01b..d70540982983 100644
@@ -25,8 +25,8 @@ struct mt7996_mcu_rxd {
 };
 
 struct mt7996_mcu_uni_event {
-	u8 cid;
-	u8 __rsv[3];
+	__le16 cid;
+	u8 __rsv[2];
 	__le32 status; /* 0: success, others: fail */
 } __packed;

Signature · 1 line

2.51.0

Shayne Chen

author reviewer

2026-02-02 07:54 UTC #3 lore raw

PATCH 4/5 wifi: mt76: avoid to set ACK for MCU command if wait_resp is not set

In reply to Shayne Chen

From: StanleyYP Wang <[email protected]>

When wait_resp is not set but the ACK option is enabled in the MCU TXD, the ACK event is enqueued to the MCU event queue without being dequeued by the original MCU command request.

Any orphaned ACK events will only be removed from the queue when another MCU command requests a response. Due to sequence index mismatches, these events are discarded one by one until a matching sequence index is found.

However, if several MCU commands that do not require a response continue to fill up the event queue, there is a risk that when an MCU command with wait_resp enabled is issued, it may dequeue the wrong event skb, especially if the queue contains events with all possible sequence indices.

Signed-off-by StanleyYP Wang <[email protected]>

Signed-off-by Shayne Chen <[email protected]>

---
 drivers/net/wireless/mediatek/mt76/mcu.c        |  2 +-
 drivers/net/wireless/mediatek/mt76/mt7996/mcu.c | 11 +++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

drivers/net/wireless/mediatek/mt76/mcu.c +1 -1

index 535c3d8a9cc0..cbfb3bbec503 100644
@@ -98,7 +98,7 @@ int mt76_mcu_skb_send_and_get_msg(struct mt76_dev *dev, struct sk_buff *skb,
 	/* orig skb might be needed for retry, mcu_skb_send_msg consumes it */
 	if (orig_skb)
 		skb_get(orig_skb);
-	ret = dev->mcu_ops->mcu_skb_send_msg(dev, skb, cmd, &seq);
+	ret = dev->mcu_ops->mcu_skb_send_msg(dev, skb, cmd, wait_resp ? &seq : NULL);
 	if (ret < 0)
 		goto out;

drivers/net/wireless/mediatek/mt76/mt7996/mcu.c +5 -6

index 285cd83e7117..68d698033e43 100644
@@ -322,13 +322,12 @@ mt7996_mcu_send_message(struct mt76_dev *mdev, struct sk_buff *skb,
 		uni_txd->pkt_type = MCU_PKT_ID;
 		uni_txd->seq = seq;
 
-		if (cmd & __MCU_CMD_FIELD_QUERY)
-			uni_txd->option = MCU_CMD_UNI_QUERY_ACK;
-		else
-			uni_txd->option = MCU_CMD_UNI_EXT_ACK;
+		uni_txd->option = MCU_CMD_UNI;
+		if (!(cmd & __MCU_CMD_FIELD_QUERY))
+			uni_txd->option |= MCU_CMD_SET;
 
-		if (mcu_cmd == MCU_UNI_CMD_SDO)
-			uni_txd->option &= ~MCU_CMD_ACK;
+		if (wait_seq)
+			uni_txd->option |= MCU_CMD_ACK;
 
 		if ((cmd & __MCU_CMD_FIELD_WA) && (cmd & __MCU_CMD_FIELD_WM))
 			uni_txd->s2d_index = MCU_S2D_H2CN;

Signature · 1 line

2.51.0

Shayne Chen

author reviewer

2026-02-02 07:54 UTC #4 lore raw

PATCH 5/5 wifi: mt76: mt7996: fix queue pause after scan due to wrong channel switch reason

In reply to Shayne Chen

From: StanleyYP Wang <[email protected]>

Previously, we use the IEEE80211_CONF_IDLE flag to avoid setting the parking channel with the CH_SWITCH_NORMAL reason, which could trigger TX emission before bootup CAC.

However, we found that this flag can be set after triggering scanning on a connected station interface, and the reason CH_SWITCH_SCAN_BYPASS_DPD will be used when switching back to the operating channel, which makes the firmware failed to resume paused AC queues.

Seems that we should avoid relying on this flag after switching to single multi-radio architecture. Instead, replace it with MT76_STATE_RUNNING.

Signed-off-by StanleyYP Wang <[email protected]>

Signed-off-by Shayne Chen <[email protected]>

---
 drivers/net/wireless/mediatek/mt76/mt7996/mcu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/wireless/mediatek/mt76/mt7996/mcu.c +1 -1

index 68d698033e43..1d5ea28e7b9b 100644
@@ -3925,7 +3925,7 @@ int mt7996_mcu_set_chan_info(struct mt7996_phy *phy, u16 tag)
 	if (phy->mt76->hw->conf.flags & IEEE80211_CONF_MONITOR)
 		req.switch_reason = CH_SWITCH_NORMAL;
 	else if (phy->mt76->offchannel ||
-		 phy->mt76->hw->conf.flags & IEEE80211_CONF_IDLE)
+		 !test_bit(MT76_STATE_RUNNING, &phy->mt76->state))
 		req.switch_reason = CH_SWITCH_SCAN_BYPASS_DPD;
 	else if (!cfg80211_reg_can_beacon(phy->mt76->hw->wiphy, chandef,
 					  NL80211_IFTYPE_AP))

Signature · 1 line

2.51.0

Felix Fietkau

maintainer

2026-02-02 09:01 UTC #5 lore raw

Re: [PATCH 5/5] wifi: mt76: mt7996: fix queue pause after scan due to wrong channel switch reason

Quoted 6 lines · Shayne Chen

From: StanleyYP Wang <[email protected]>
Previously, we use the IEEE80211_CONF_IDLE flag to avoid setting the parking channel with the CH_SWITCH_NORMAL reason, which could trigger TX emission before bootup CAC.

Quoted 7 lines

However, we found that this flag can be set after triggering scanning on a connected station interface, and the reason CH_SWITCH_SCAN_BYPASS_DPD will be used when switching back to the operating channel, which makes the firmware failed to resume paused AC queues.
Seems that we should avoid relying on this flag after switching to single multi-radio architecture. Instead, replace it with MT76_STATE_RUNNING.

I don't see how the conditions are comparable at all. I also don't see how this function can be called with MT76_STATE_RUNNING unset.

Maybe a better replacement would be to check for a chanctx on the phy?

- Felix

Shayne Chen

author reviewer

2026-02-02 11:52 UTC #6 lore raw

Re: [PATCH 5/5] wifi: mt76: mt7996: fix queue pause after scan due to wrong channel switch reason

Quoted 17 lines · Felix Fietkau

Quoted 16 lines · Shayne Chen
From: StanleyYP Wang <[email protected]>
Previously, we use the IEEE80211_CONF_IDLE flag to avoid setting the
parking channel with the CH_SWITCH_NORMAL reason, which could trigger TX
emission before bootup CAC.
However, we found that this flag can be set after triggering scanning on a
connected station interface, and the reason CH_SWITCH_SCAN_BYPASS_DPD will
be used when switching back to the operating channel, which makes the
firmware failed to resume paused AC queues.

Quoted 9 lines

Quoted 4 lines
Seems that we should avoid relying on this flag after switching to single
multi-radio architecture. Instead, replace it with MT76_STATE_RUNNING.
I don't see how the conditions are comparable at all. I also don't see
how this function can be called with MT76_STATE_RUNNING unset.

The condition is used to prevent mt7996_mcu_set_chan_info() (in mt7996_run()) from triggering TX emission.

Quoted 3 lines

Maybe a better replacement would be to check for a chanctx on the phy?

Will do some tests on this and send v2.

Thanks,
Shayne

Quoted 1 line

- Felix

Felix Fietkau

maintainer

2026-02-02 12:22 UTC #7 lore raw

Re: [PATCH 5/5] wifi: mt76: mt7996: fix queue pause after scan due to wrong channel switch reason

Quoted 23 lines · Shayne Chen

Quoted 22 lines · Felix Fietkau
Quoted 20 lines · Shayne Chen
From: StanleyYP Wang <[email protected]>
Previously, we use the IEEE80211_CONF_IDLE flag to avoid setting the
parking channel with the CH_SWITCH_NORMAL reason, which could trigger TX
emission before bootup CAC.
However, we found that this flag can be set after triggering scanning on a
connected station interface, and the reason CH_SWITCH_SCAN_BYPASS_DPD will
be used when switching back to the operating channel, which makes the
firmware failed to resume paused AC queues.
Seems that we should avoid relying on this flag after switching to single
multi-radio architecture. Instead, replace it with MT76_STATE_RUNNING.

Quoted 5 lines

Quoted 3 lines
I don't see how the conditions are comparable at all. I also don't see how this function can be called with MT76_STATE_RUNNING unset.
The condition is used to prevent mt7996_mcu_set_chan_info() (in mt7996_run()) from triggering TX emission.

Makes sense.

Quoted 4 lines

Quoted 3 lines
Maybe a better replacement would be to check for a chanctx on the phy?
Will do some tests on this and send v2.

Thanks.

- Felix

Felix Fietkau

maintainer

2026-02-02 08:53 UTC #8 lore raw

In reply to Shayne Chen

Quoted 9 lines · Shayne Chen

A deadlock will occur if both of the following conditions are met, because they each attempt to acquire the rx_lock:
- mac80211 receives an unexpected BAR control frame, which triggers a BA deletion
- A transmission failure happens due to an abnormality in DMA
Since ieee80211_tx_status_ext() is primarily used to address AQL issues, avoid potential deadlocks by restricting calls to ieee80211_tx_status_ext only for data frames.

First of all, ieee80211_tx_status_ext is not primarily used to address AQL, ieee80211_free_txskb handles it as well. The reason for it is tx status handling, e.g. for management frames sent by hostapd that require an ACK status report, so limiting the status calls for data frames seems wrong to me.

I don't really understand how the scenario you're describing leads to a deadlock. From my understanding, if something in the mac80211 rx path triggers a tx, it should end up calling mt76_tx(), which queues the skb to wcid->tx_list and triggers the tx worker. So the actual dma tx callls are expected to come from the worker kthread. How does this lead to a deadlock on rx_lock?

- Felix

Shayne Chen

author reviewer

2026-02-02 11:46 UTC #9 lore raw

Quoted 22 lines · Felix Fietkau

Quoted 11 lines · Shayne Chen
A deadlock will occur if both of the following conditions are met, because they each attempt to acquire the rx_lock:
- mac80211 receives an unexpected BAR control frame, which triggers a BA deletion
- A transmission failure happens due to an abnormality in DMA
Since ieee80211_tx_status_ext() is primarily used to address AQL issues,
avoid potential deadlocks by restricting calls to ieee80211_tx_status_ext
only for data frames.
First of all, ieee80211_tx_status_ext is not primarily used to address
AQL, ieee80211_free_txskb handles it as well. The reason for it is tx status handling, e.g. for management frames sent by hostapd that require
an ACK status report, so limiting the status calls for data frames seems
wrong to me.

Quoted 9 lines

I don't really understand how the scenario you're describing leads to a
deadlock. From my understanding, if something in the mac80211 rx path triggers a tx, it should end up calling mt76_tx(), which queues the skb
to wcid->tx_list and triggers the tx worker. So the actual dma tx callls
are expected to come from the worker kthread.
How does this lead to a deadlock on rx_lock?

Hi Felix,

Thanks for the explanation.
I've re-checked the codebase used by the customer when the issue was reported, and I found that the wcid->tx_list structure was not present in that version. So yes, this problem should not occur in the current codebase.

Will drop this patch in v2.

Thanks,
Shayne

Quoted 2 lines

- Felix