[PATCH v2] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()

[PATCH v2] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()

[Lorenzo Bianconi]

mt76_tx_status_skb_add() zeroes the mt76_tx_cb struct stored at
info->status.status_driver_data via memset(). Since info->control and
info->status are members of the same union in ieee80211_tx_info,
this overwrites info->control.flags.
In mt7996_tx_prepare_skb(), mt76_tx_status_skb_add() is called before
mt7996_mac_write_txwi(), which re-reads info->control.flags to extract
IEEE80211_TX_CTRL_MLO_LINK. Because the field has been zeroed, the
link_id always resolves to 0 for frames using global_wcid, leading to
incorrect TXWI configuration.
Fix this by passing link_id as an explicit parameter to
mt7996_mac_write_txwi(). In mt7996_tx_prepare_skb(), the link_id is
already extracted from info->control.flags before the destructive
mt76_tx_status_skb_add() call. For the beacon and inband discovery
callers in mcu.c, use link_conf->link_id directly.

Fixes: f0b0b239b8f36 ("wifi: mt76: mt7996: rework mt7996_mac_write_txwi() for MLO support")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
Changes in v2:
- Do not use link_id in mt7996_mac_write_txwi if it is IEEE80211_LINK_UNSPECIFIED
- In mt7996_mac_write_txwi() rely on link_id calculated in
  mt7996_tx_prepare_skb().
- Link to v1: https://lore.kernel.org/r/20260530-mt76_tx_status_skb_add-overwrite-fix-v1-1-e2c3151c391a@kernel.org
---
 drivers/net/wireless/mediatek/mt76/mt7996/mac.c    | 14 ++++----------
 drivers/net/wireless/mediatek/mt76/mt7996/mcu.c    |  5 +++--
 drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h |  3 ++-
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
index c98446057282..95b3078d9667 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
@@ -856,7 +856,8 @@ mt7996_mac_write_txwi_80211(struct mt7996_dev *dev, __le32 *txwi,
 void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
 			   struct sk_buff *skb, struct mt76_wcid *wcid,
 			   struct ieee80211_key_conf *key, int pid,
-			   enum mt76_txq_id qid, u32 changed)
+			   enum mt76_txq_id qid, u32 changed,
+			   unsigned int link_id)
 {
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
@@ -866,7 +867,6 @@ void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
 	bool is_8023 = info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP;
 	struct mt76_vif_link *mlink = NULL;
 	struct mt7996_vif *mvif;
-	unsigned int link_id;
 	u16 tx_count = 15;
 	u32 val;
 	bool inband_disc = !!(changed & (BSS_CHANGED_UNSOL_BCAST_PROBE_RESP |
@@ -874,17 +874,11 @@ void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
 	bool beacon = !!(changed & (BSS_CHANGED_BEACON |
 				    BSS_CHANGED_BEACON_ENABLED)) && (!inband_disc);
 
-	if (wcid != &dev->mt76.global_wcid)
-		link_id = wcid->link_id;
-	else
-		link_id = u32_get_bits(info->control.flags,
-				       IEEE80211_TX_CTRL_MLO_LINK);
-
 	mvif = vif ? (struct mt7996_vif *)vif->drv_priv : NULL;
 	if (mvif) {
 		if (wcid->offchannel)
 			mlink = rcu_dereference(mvif->mt76.offchannel_link);
-		if (!mlink)
+		if (!mlink && link_id != IEEE80211_LINK_UNSPECIFIED)
 			mlink = rcu_dereference(mvif->mt76.link[link_id]);
 	}
 
@@ -1096,7 +1090,7 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
 	/* Transmit non qos data by 802.11 header and need to fill txd by host*/
 	if (!is_8023 || pid >= MT_PACKET_ID_FIRST)
 		mt7996_mac_write_txwi(dev, txwi_ptr, tx_info->skb, wcid, key,
-				      pid, qid, 0);
+				      pid, qid, 0, link_id);
 
 	/* MT7996 and MT7992 require driver to provide the MAC TXP for AddBA
 	 * req
diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
index 8be40d60ad29..a14c63438923 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
@@ -3103,7 +3103,7 @@ mt7996_mcu_beacon_cont(struct mt7996_dev *dev,
 
 	buf = (u8 *)bcn + sizeof(*bcn);
 	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL, 0, 0,
-			      BSS_CHANGED_BEACON);
+			      BSS_CHANGED_BEACON, link_conf->link_id);
 
 	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
 }
@@ -3249,7 +3249,8 @@ int mt7996_mcu_beacon_inband_discov(struct mt7996_dev *dev,
 
 	buf = (u8 *)tlv + sizeof(*discov);
 
-	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL, 0, 0, changed);
+	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL, 0, 0,
+			      changed, link_conf->link_id);
 
 	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
index 0dc4198fcf8b..0d6488522ba7 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
@@ -874,7 +874,8 @@ void mt7996_mac_enable_nf(struct mt7996_dev *dev, u8 band);
 void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
 			   struct sk_buff *skb, struct mt76_wcid *wcid,
 			   struct ieee80211_key_conf *key, int pid,
-			   enum mt76_txq_id qid, u32 changed);
+			   enum mt76_txq_id qid, u32 changed,
+			   unsigned int link_id);
 void mt7996_mac_update_beacons(struct mt7996_phy *phy);
 void mt7996_mac_set_coverage_class(struct mt7996_phy *phy);
 void mt7996_mac_work(struct work_struct *work);

---
base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
change-id: 20260530-mt76_tx_status_skb_add-overwrite-fix-85818a9bb31f

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>

Re: [PATCH v2] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()

[Ryder Lee]

On Sun, 2026-05-31 at 10:55 +0200, Lorenzo Bianconi wrote:
> mt76_tx_status_skb_add() zeroes the mt76_tx_cb struct stored at
> info->status.status_driver_data via memset(). Since info->control and
> info->status are members of the same union in ieee80211_tx_info,
> this overwrites info->control.flags.
> In mt7996_tx_prepare_skb(), mt76_tx_status_skb_add() is called before
> mt7996_mac_write_txwi(), which re-reads info->control.flags to
> extract
> IEEE80211_TX_CTRL_MLO_LINK. Because the field has been zeroed, the
> link_id always resolves to 0 for frames using global_wcid, leading to
> incorrect TXWI configuration.
> Fix this by passing link_id as an explicit parameter to
> mt7996_mac_write_txwi(). In mt7996_tx_prepare_skb(), the link_id is
> already extracted from info->control.flags before the destructive
> mt76_tx_status_skb_add() call. For the beacon and inband discovery
> callers in mcu.c, use link_conf->link_id directly.
> 
> Fixes: f0b0b239b8f36 ("wifi: mt76: mt7996: rework
> mt7996_mac_write_txwi() for MLO support")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> Changes in v2:
> - Do not use link_id in mt7996_mac_write_txwi if it is
> IEEE80211_LINK_UNSPECIFIED
> - In mt7996_mac_write_txwi() rely on link_id calculated in
>   mt7996_tx_prepare_skb().
> - Link to v1:
> https://lore.kernel.org/r/20260530-mt76_tx_status_skb_add-overwrite-fix-v1-1-e2c3151c391a@kernel.org
>  
> ---
>  drivers/net/wireless/mediatek/mt76/mt7996/mac.c    | 14 ++++--------
> --
>  drivers/net/wireless/mediatek/mt76/mt7996/mcu.c    |  5 +++--
>  drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h |  3 ++-
>  3 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> index c98446057282..95b3078d9667 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> @@ -856,7 +856,8 @@ mt7996_mac_write_txwi_80211(struct mt7996_dev
> *dev, __le32 *txwi,
>  void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
>  			   struct sk_buff *skb, struct mt76_wcid
> *wcid,
>  			   struct ieee80211_key_conf *key, int pid,
> -			   enum mt76_txq_id qid, u32 changed)
> +			   enum mt76_txq_id qid, u32 changed,
> +			   unsigned int link_id)
>  {
>  	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb-
> >data;
>  	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> @@ -866,7 +867,6 @@ void mt7996_mac_write_txwi(struct mt7996_dev
> *dev, __le32 *txwi,
>  	bool is_8023 = info->flags &
> IEEE80211_TX_CTL_HW_80211_ENCAP;
>  	struct mt76_vif_link *mlink = NULL;
>  	struct mt7996_vif *mvif;
> -	unsigned int link_id;
>  	u16 tx_count = 15;
>  	u32 val;
>  	bool inband_disc = !!(changed &
> (BSS_CHANGED_UNSOL_BCAST_PROBE_RESP |
> @@ -874,17 +874,11 @@ void mt7996_mac_write_txwi(struct mt7996_dev
> *dev, __le32 *txwi,
>  	bool beacon = !!(changed & (BSS_CHANGED_BEACON |
>  				    BSS_CHANGED_BEACON_ENABLED)) &&
> (!inband_disc);
>  
> -	if (wcid != &dev->mt76.global_wcid)
> -		link_id = wcid->link_id;
> -	else
> -		link_id = u32_get_bits(info->control.flags,
> -				       IEEE80211_TX_CTRL_MLO_LINK);
> -
>  	mvif = vif ? (struct mt7996_vif *)vif->drv_priv : NULL;
>  	if (mvif) {
>  		if (wcid->offchannel)
>  			mlink = rcu_dereference(mvif-
> >mt76.offchannel_link);
> -		if (!mlink)
> +		if (!mlink && link_id != IEEE80211_LINK_UNSPECIFIED)
>  			mlink = rcu_dereference(mvif-
> >mt76.link[link_id]);
>  	}
>  
> @@ -1096,7 +1090,7 @@ int mt7996_tx_prepare_skb(struct mt76_dev
> *mdev, void *txwi_ptr,
>  	/* Transmit non qos data by 802.11 header and need to fill
> txd by host*/
>  	if (!is_8023 || pid >= MT_PACKET_ID_FIRST)
>  		mt7996_mac_write_txwi(dev, txwi_ptr, tx_info->skb,
> wcid, key,
> -				      pid, qid, 0);
> +				      pid, qid, 0, link_id);
>  
>  	/* MT7996 and MT7992 require driver to provide the MAC TXP
> for AddBA
>  	 * req
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> index 8be40d60ad29..a14c63438923 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> @@ -3103,7 +3103,7 @@ mt7996_mcu_beacon_cont(struct mt7996_dev *dev,
>  
>  	buf = (u8 *)bcn + sizeof(*bcn);
>  	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL,
> 0, 0,
> -			      BSS_CHANGED_BEACON);
> +			      BSS_CHANGED_BEACON, link_conf-
> >link_id);
>  
>  	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
>  }
> @@ -3249,7 +3249,8 @@ int mt7996_mcu_beacon_inband_discov(struct
> mt7996_dev *dev,
>  
>  	buf = (u8 *)tlv + sizeof(*discov);
>  
> -	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL,
> 0, 0, changed);
> +	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL,
> 0, 0,
> +			      changed, link_conf->link_id);
>  
>  	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
>  
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> index 0dc4198fcf8b..0d6488522ba7 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> @@ -874,7 +874,8 @@ void mt7996_mac_enable_nf(struct mt7996_dev *dev,
> u8 band);
>  void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
>  			   struct sk_buff *skb, struct mt76_wcid
> *wcid,
>  			   struct ieee80211_key_conf *key, int pid,
> -			   enum mt76_txq_id qid, u32 changed);
> +			   enum mt76_txq_id qid, u32 changed,
> +			   unsigned int link_id);
>  void mt7996_mac_update_beacons(struct mt7996_phy *phy);
>  void mt7996_mac_set_coverage_class(struct mt7996_phy *phy);
>  void mt7996_mac_work(struct work_struct *work);
> 
> ---
> base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
> change-id: 20260530-mt76_tx_status_skb_add-overwrite-fix-85818a9bb31f
> 
> Best regards,
> 
> 
We might expand flags further so this still doesn't solve the issue of
flags being cleared - it only works for MLO flag. And the developers
still won't easily notice that the flags are being cleared.
 
Our current approach is to move memset after mt7996_mac_write_txwi().
It's more flexible and we don't have to constantly change the function
parameters just for the flags.

Ryder

Re: [PATCH v2] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()

[lorenzo]

[-- Attachment #1: Type: text/plain, Size: 7190 bytes --]

> On Sun, 2026-05-31 at 10:55 +0200, Lorenzo Bianconi wrote:
> > mt76_tx_status_skb_add() zeroes the mt76_tx_cb struct stored at
> > info->status.status_driver_data via memset(). Since info->control and
> > info->status are members of the same union in ieee80211_tx_info,
> > this overwrites info->control.flags.
> > In mt7996_tx_prepare_skb(), mt76_tx_status_skb_add() is called before
> > mt7996_mac_write_txwi(), which re-reads info->control.flags to
> > extract
> > IEEE80211_TX_CTRL_MLO_LINK. Because the field has been zeroed, the
> > link_id always resolves to 0 for frames using global_wcid, leading to
> > incorrect TXWI configuration.
> > Fix this by passing link_id as an explicit parameter to
> > mt7996_mac_write_txwi(). In mt7996_tx_prepare_skb(), the link_id is
> > already extracted from info->control.flags before the destructive
> > mt76_tx_status_skb_add() call. For the beacon and inband discovery
> > callers in mcu.c, use link_conf->link_id directly.
> > 
> > Fixes: f0b0b239b8f36 ("wifi: mt76: mt7996: rework
> > mt7996_mac_write_txwi() for MLO support")
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > Changes in v2:
> > - Do not use link_id in mt7996_mac_write_txwi if it is
> > IEEE80211_LINK_UNSPECIFIED
> > - In mt7996_mac_write_txwi() rely on link_id calculated in
> >   mt7996_tx_prepare_skb().
> > - Link to v1:
> > https://lore.kernel.org/r/20260530-mt76_tx_status_skb_add-overwrite-fix-v1-1-e2c3151c391a@kernel.org
> >  
> > ---
> >  drivers/net/wireless/mediatek/mt76/mt7996/mac.c    | 14 ++++--------
> > --
> >  drivers/net/wireless/mediatek/mt76/mt7996/mcu.c    |  5 +++--
> >  drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h |  3 ++-
> >  3 files changed, 9 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > index c98446057282..95b3078d9667 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > @@ -856,7 +856,8 @@ mt7996_mac_write_txwi_80211(struct mt7996_dev
> > *dev, __le32 *txwi,
> >  void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
> >  			   struct sk_buff *skb, struct mt76_wcid
> > *wcid,
> >  			   struct ieee80211_key_conf *key, int pid,
> > -			   enum mt76_txq_id qid, u32 changed)
> > +			   enum mt76_txq_id qid, u32 changed,
> > +			   unsigned int link_id)
> >  {
> >  	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb-
> > >data;
> >  	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> > @@ -866,7 +867,6 @@ void mt7996_mac_write_txwi(struct mt7996_dev
> > *dev, __le32 *txwi,
> >  	bool is_8023 = info->flags &
> > IEEE80211_TX_CTL_HW_80211_ENCAP;
> >  	struct mt76_vif_link *mlink = NULL;
> >  	struct mt7996_vif *mvif;
> > -	unsigned int link_id;
> >  	u16 tx_count = 15;
> >  	u32 val;
> >  	bool inband_disc = !!(changed &
> > (BSS_CHANGED_UNSOL_BCAST_PROBE_RESP |
> > @@ -874,17 +874,11 @@ void mt7996_mac_write_txwi(struct mt7996_dev
> > *dev, __le32 *txwi,
> >  	bool beacon = !!(changed & (BSS_CHANGED_BEACON |
> >  				    BSS_CHANGED_BEACON_ENABLED)) &&
> > (!inband_disc);
> >  
> > -	if (wcid != &dev->mt76.global_wcid)
> > -		link_id = wcid->link_id;
> > -	else
> > -		link_id = u32_get_bits(info->control.flags,
> > -				       IEEE80211_TX_CTRL_MLO_LINK);
> > -
> >  	mvif = vif ? (struct mt7996_vif *)vif->drv_priv : NULL;
> >  	if (mvif) {
> >  		if (wcid->offchannel)
> >  			mlink = rcu_dereference(mvif-
> > >mt76.offchannel_link);
> > -		if (!mlink)
> > +		if (!mlink && link_id != IEEE80211_LINK_UNSPECIFIED)
> >  			mlink = rcu_dereference(mvif-
> > >mt76.link[link_id]);
> >  	}
> >  
> > @@ -1096,7 +1090,7 @@ int mt7996_tx_prepare_skb(struct mt76_dev
> > *mdev, void *txwi_ptr,
> >  	/* Transmit non qos data by 802.11 header and need to fill
> > txd by host*/
> >  	if (!is_8023 || pid >= MT_PACKET_ID_FIRST)
> >  		mt7996_mac_write_txwi(dev, txwi_ptr, tx_info->skb,
> > wcid, key,
> > -				      pid, qid, 0);
> > +				      pid, qid, 0, link_id);
> >  
> >  	/* MT7996 and MT7992 require driver to provide the MAC TXP
> > for AddBA
> >  	 * req
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > index 8be40d60ad29..a14c63438923 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > @@ -3103,7 +3103,7 @@ mt7996_mcu_beacon_cont(struct mt7996_dev *dev,
> >  
> >  	buf = (u8 *)bcn + sizeof(*bcn);
> >  	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL,
> > 0, 0,
> > -			      BSS_CHANGED_BEACON);
> > +			      BSS_CHANGED_BEACON, link_conf-
> > >link_id);
> >  
> >  	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
> >  }
> > @@ -3249,7 +3249,8 @@ int mt7996_mcu_beacon_inband_discov(struct
> > mt7996_dev *dev,
> >  
> >  	buf = (u8 *)tlv + sizeof(*discov);
> >  
> > -	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL,
> > 0, 0, changed);
> > +	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL,
> > 0, 0,
> > +			      changed, link_conf->link_id);
> >  
> >  	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
> >  
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > index 0dc4198fcf8b..0d6488522ba7 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > @@ -874,7 +874,8 @@ void mt7996_mac_enable_nf(struct mt7996_dev *dev,
> > u8 band);
> >  void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
> >  			   struct sk_buff *skb, struct mt76_wcid
> > *wcid,
> >  			   struct ieee80211_key_conf *key, int pid,
> > -			   enum mt76_txq_id qid, u32 changed);
> > +			   enum mt76_txq_id qid, u32 changed,
> > +			   unsigned int link_id);
> >  void mt7996_mac_update_beacons(struct mt7996_phy *phy);
> >  void mt7996_mac_set_coverage_class(struct mt7996_phy *phy);
> >  void mt7996_mac_work(struct work_struct *work);
> > 
> > ---
> > base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
> > change-id: 20260530-mt76_tx_status_skb_add-overwrite-fix-85818a9bb31f
> > 
> > Best regards,
> > 
> > 
> We might expand flags further so this still doesn't solve the issue of
> flags being cleared - it only works for MLO flag. And the developers
> still won't easily notice that the flags are being cleared.

My opinion is we should consider just upstream code and then change it as soon
as you post this new feature upstream, but I will let Felix comments on it.
Moreover, the proposed approach aligns link_id used in mt7996_tx_prepare_skb()
to the one used in mt7996_mac_write_txwi() and fix a possible OOB bug in
mt7996_mac_write_txwi().

Regards,
Lorenzo

>  
> Our current approach is to move memset after mt7996_mac_write_txwi().
> It's more flexible and we don't have to constantly change the function
> parameters just for the flags.
> 
> Ryder
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()

[Ryder Lee]

On Sun, 2026-05-31 at 14:11 +0200, lorenzo@kernel.org wrote:
> > On Sun, 2026-05-31 at 10:55 +0200, Lorenzo Bianconi wrote:
> > > mt76_tx_status_skb_add() zeroes the mt76_tx_cb struct stored at
> > > info->status.status_driver_data via memset(). Since info->control
> > > and
> > > info->status are members of the same union in ieee80211_tx_info,
> > > this overwrites info->control.flags.
> > > In mt7996_tx_prepare_skb(), mt76_tx_status_skb_add() is called
> > > before
> > > mt7996_mac_write_txwi(), which re-reads info->control.flags to
> > > extract
> > > IEEE80211_TX_CTRL_MLO_LINK. Because the field has been zeroed,
> > > the
> > > link_id always resolves to 0 for frames using global_wcid,
> > > leading to
> > > incorrect TXWI configuration.
> > > Fix this by passing link_id as an explicit parameter to
> > > mt7996_mac_write_txwi(). In mt7996_tx_prepare_skb(), the link_id
> > > is
> > > already extracted from info->control.flags before the destructive
> > > mt76_tx_status_skb_add() call. For the beacon and inband
> > > discovery
> > > callers in mcu.c, use link_conf->link_id directly.
> > > 
> > > Fixes: f0b0b239b8f36 ("wifi: mt76: mt7996: rework
> > > mt7996_mac_write_txwi() for MLO support")
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > > Changes in v2:
> > > - Do not use link_id in mt7996_mac_write_txwi if it is
> > > IEEE80211_LINK_UNSPECIFIED
> > > - In mt7996_mac_write_txwi() rely on link_id calculated in
> > >   mt7996_tx_prepare_skb().
> > > - Link to v1:
> > > https://lore.kernel.org/r/20260530-mt76_tx_status_skb_add-overwrite-fix-v1-1-e2c3151c391a@kernel.org
> > >  
> > > ---
> > >  drivers/net/wireless/mediatek/mt76/mt7996/mac.c    | 14 ++++----
> > > ----
> > > --
> > >  drivers/net/wireless/mediatek/mt76/mt7996/mcu.c    |  5 +++--
> > >  drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h |  3 ++-
> > >  3 files changed, 9 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > index c98446057282..95b3078d9667 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > @@ -856,7 +856,8 @@ mt7996_mac_write_txwi_80211(struct mt7996_dev
> > > *dev, __le32 *txwi,
> > >  void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
> > >  			   struct sk_buff *skb, struct mt76_wcid
> > > *wcid,
> > >  			   struct ieee80211_key_conf *key, int
> > > pid,
> > > -			   enum mt76_txq_id qid, u32 changed)
> > > +			   enum mt76_txq_id qid, u32 changed,
> > > +			   unsigned int link_id)
> > >  {
> > >  	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb-
> > > > data;
> > >  	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> > > @@ -866,7 +867,6 @@ void mt7996_mac_write_txwi(struct mt7996_dev
> > > *dev, __le32 *txwi,
> > >  	bool is_8023 = info->flags &
> > > IEEE80211_TX_CTL_HW_80211_ENCAP;
> > >  	struct mt76_vif_link *mlink = NULL;
> > >  	struct mt7996_vif *mvif;
> > > -	unsigned int link_id;
> > >  	u16 tx_count = 15;
> > >  	u32 val;
> > >  	bool inband_disc = !!(changed &
> > > (BSS_CHANGED_UNSOL_BCAST_PROBE_RESP |
> > > @@ -874,17 +874,11 @@ void mt7996_mac_write_txwi(struct
> > > mt7996_dev
> > > *dev, __le32 *txwi,
> > >  	bool beacon = !!(changed & (BSS_CHANGED_BEACON |
> > >  				    BSS_CHANGED_BEACON_ENABLED))
> > > &&
> > > (!inband_disc);
> > >  
> > > -	if (wcid != &dev->mt76.global_wcid)
> > > -		link_id = wcid->link_id;
> > > -	else
> > > -		link_id = u32_get_bits(info->control.flags,
> > > -				      
> > > IEEE80211_TX_CTRL_MLO_LINK);
> > > -
> > >  	mvif = vif ? (struct mt7996_vif *)vif->drv_priv : NULL;
> > >  	if (mvif) {
> > >  		if (wcid->offchannel)
> > >  			mlink = rcu_dereference(mvif-
> > > > mt76.offchannel_link);
> > > -		if (!mlink)
> > > +		if (!mlink && link_id !=
> > > IEEE80211_LINK_UNSPECIFIED)
> > >  			mlink = rcu_dereference(mvif-
> > > > mt76.link[link_id]);
> > >  	}
> > >  
> > > @@ -1096,7 +1090,7 @@ int mt7996_tx_prepare_skb(struct mt76_dev
> > > *mdev, void *txwi_ptr,
> > >  	/* Transmit non qos data by 802.11 header and need to
> > > fill
> > > txd by host*/
> > >  	if (!is_8023 || pid >= MT_PACKET_ID_FIRST)
> > >  		mt7996_mac_write_txwi(dev, txwi_ptr, tx_info-
> > > >skb,
> > > wcid, key,
> > > -				      pid, qid, 0);
> > > +				      pid, qid, 0, link_id);
> > >  
> > >  	/* MT7996 and MT7992 require driver to provide the MAC
> > > TXP
> > > for AddBA
> > >  	 * req
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > index 8be40d60ad29..a14c63438923 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > @@ -3103,7 +3103,7 @@ mt7996_mcu_beacon_cont(struct mt7996_dev
> > > *dev,
> > >  
> > >  	buf = (u8 *)bcn + sizeof(*bcn);
> > >  	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid,
> > > NULL,
> > > 0, 0,
> > > -			      BSS_CHANGED_BEACON);
> > > +			      BSS_CHANGED_BEACON, link_conf-
> > > > link_id);
> > >  
> > >  	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
> > >  }
> > > @@ -3249,7 +3249,8 @@ int mt7996_mcu_beacon_inband_discov(struct
> > > mt7996_dev *dev,
> > >  
> > >  	buf = (u8 *)tlv + sizeof(*discov);
> > >  
> > > -	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid,
> > > NULL,
> > > 0, 0, changed);
> > > +	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid,
> > > NULL,
> > > 0, 0,
> > > +			      changed, link_conf->link_id);
> > >  
> > >  	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
> > >  
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > index 0dc4198fcf8b..0d6488522ba7 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > @@ -874,7 +874,8 @@ void mt7996_mac_enable_nf(struct mt7996_dev
> > > *dev,
> > > u8 band);
> > >  void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
> > >  			   struct sk_buff *skb, struct mt76_wcid
> > > *wcid,
> > >  			   struct ieee80211_key_conf *key, int
> > > pid,
> > > -			   enum mt76_txq_id qid, u32 changed);
> > > +			   enum mt76_txq_id qid, u32 changed,
> > > +			   unsigned int link_id);
> > >  void mt7996_mac_update_beacons(struct mt7996_phy *phy);
> > >  void mt7996_mac_set_coverage_class(struct mt7996_phy *phy);
> > >  void mt7996_mac_work(struct work_struct *work);
> > > 
> > > ---
> > > base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
> > > change-id: 20260530-mt76_tx_status_skb_add-overwrite-fix-
> > > 85818a9bb31f
> > > 
> > > Best regards,
> > > 
> > > 
> > We might expand flags further so this still doesn't solve the issue
> > of
> > flags being cleared - it only works for MLO flag. And the
> > developers
> > still won't easily notice that the flags are being cleared.
> 
> My opinion is we should consider just upstream code and then change
> it as soon
> as you post this new feature upstream, but I will let Felix comments
> on it.
> Moreover, the proposed approach aligns link_id used in
> mt7996_tx_prepare_skb()
> to the one used in mt7996_mac_write_txwi() and fix a possible OOB bug
> in
> mt7996_mac_write_txwi().
> 
> Regards,
> Lorenzo
> 
> > 

Just to tie in with this patch subject - I'm just thinking of a way to
solve this once and for all. If the problem is reading zeroed info-
>control.flags, wouldn't it be better to just pass a u32 flags,
something like this:

u32 flags = info->control.flags

mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL, 0, 0,
			      changed, flags);

We can use all flags then.

Ryder

Re: [PATCH v2] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()

[lorenzo]

[-- Attachment #1: Type: text/plain, Size: 8556 bytes --]

On May 31, Ryder Lee wrote:
> On Sun, 2026-05-31 at 14:11 +0200, lorenzo@kernel.org wrote:
> > > On Sun, 2026-05-31 at 10:55 +0200, Lorenzo Bianconi wrote:
> > > > mt76_tx_status_skb_add() zeroes the mt76_tx_cb struct stored at
> > > > info->status.status_driver_data via memset(). Since info->control
> > > > and
> > > > info->status are members of the same union in ieee80211_tx_info,
> > > > this overwrites info->control.flags.
> > > > In mt7996_tx_prepare_skb(), mt76_tx_status_skb_add() is called
> > > > before
> > > > mt7996_mac_write_txwi(), which re-reads info->control.flags to
> > > > extract
> > > > IEEE80211_TX_CTRL_MLO_LINK. Because the field has been zeroed,
> > > > the
> > > > link_id always resolves to 0 for frames using global_wcid,
> > > > leading to
> > > > incorrect TXWI configuration.
> > > > Fix this by passing link_id as an explicit parameter to
> > > > mt7996_mac_write_txwi(). In mt7996_tx_prepare_skb(), the link_id
> > > > is
> > > > already extracted from info->control.flags before the destructive
> > > > mt76_tx_status_skb_add() call. For the beacon and inband
> > > > discovery
> > > > callers in mcu.c, use link_conf->link_id directly.
> > > > 
> > > > Fixes: f0b0b239b8f36 ("wifi: mt76: mt7996: rework
> > > > mt7996_mac_write_txwi() for MLO support")
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > ---
> > > > Changes in v2:
> > > > - Do not use link_id in mt7996_mac_write_txwi if it is
> > > > IEEE80211_LINK_UNSPECIFIED
> > > > - In mt7996_mac_write_txwi() rely on link_id calculated in
> > > >   mt7996_tx_prepare_skb().
> > > > - Link to v1:
> > > > https://lore.kernel.org/r/20260530-mt76_tx_status_skb_add-overwrite-fix-v1-1-e2c3151c391a@kernel.org
> > > >  
> > > > ---
> > > >  drivers/net/wireless/mediatek/mt76/mt7996/mac.c    | 14 ++++----
> > > > ----
> > > > --
> > > >  drivers/net/wireless/mediatek/mt76/mt7996/mcu.c    |  5 +++--
> > > >  drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h |  3 ++-
> > > >  3 files changed, 9 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > > b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > > index c98446057282..95b3078d9667 100644
> > > > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > > @@ -856,7 +856,8 @@ mt7996_mac_write_txwi_80211(struct mt7996_dev
> > > > *dev, __le32 *txwi,
> > > >  void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
> > > >  			   struct sk_buff *skb, struct mt76_wcid
> > > > *wcid,
> > > >  			   struct ieee80211_key_conf *key, int
> > > > pid,
> > > > -			   enum mt76_txq_id qid, u32 changed)
> > > > +			   enum mt76_txq_id qid, u32 changed,
> > > > +			   unsigned int link_id)
> > > >  {
> > > >  	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb-
> > > > > data;
> > > >  	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> > > > @@ -866,7 +867,6 @@ void mt7996_mac_write_txwi(struct mt7996_dev
> > > > *dev, __le32 *txwi,
> > > >  	bool is_8023 = info->flags &
> > > > IEEE80211_TX_CTL_HW_80211_ENCAP;
> > > >  	struct mt76_vif_link *mlink = NULL;
> > > >  	struct mt7996_vif *mvif;
> > > > -	unsigned int link_id;
> > > >  	u16 tx_count = 15;
> > > >  	u32 val;
> > > >  	bool inband_disc = !!(changed &
> > > > (BSS_CHANGED_UNSOL_BCAST_PROBE_RESP |
> > > > @@ -874,17 +874,11 @@ void mt7996_mac_write_txwi(struct
> > > > mt7996_dev
> > > > *dev, __le32 *txwi,
> > > >  	bool beacon = !!(changed & (BSS_CHANGED_BEACON |
> > > >  				    BSS_CHANGED_BEACON_ENABLED))
> > > > &&
> > > > (!inband_disc);
> > > >  
> > > > -	if (wcid != &dev->mt76.global_wcid)
> > > > -		link_id = wcid->link_id;
> > > > -	else
> > > > -		link_id = u32_get_bits(info->control.flags,
> > > > -				      
> > > > IEEE80211_TX_CTRL_MLO_LINK);
> > > > -
> > > >  	mvif = vif ? (struct mt7996_vif *)vif->drv_priv : NULL;
> > > >  	if (mvif) {
> > > >  		if (wcid->offchannel)
> > > >  			mlink = rcu_dereference(mvif-
> > > > > mt76.offchannel_link);
> > > > -		if (!mlink)
> > > > +		if (!mlink && link_id !=
> > > > IEEE80211_LINK_UNSPECIFIED)
> > > >  			mlink = rcu_dereference(mvif-
> > > > > mt76.link[link_id]);
> > > >  	}
> > > >  
> > > > @@ -1096,7 +1090,7 @@ int mt7996_tx_prepare_skb(struct mt76_dev
> > > > *mdev, void *txwi_ptr,
> > > >  	/* Transmit non qos data by 802.11 header and need to
> > > > fill
> > > > txd by host*/
> > > >  	if (!is_8023 || pid >= MT_PACKET_ID_FIRST)
> > > >  		mt7996_mac_write_txwi(dev, txwi_ptr, tx_info-
> > > > >skb,
> > > > wcid, key,
> > > > -				      pid, qid, 0);
> > > > +				      pid, qid, 0, link_id);
> > > >  
> > > >  	/* MT7996 and MT7992 require driver to provide the MAC
> > > > TXP
> > > > for AddBA
> > > >  	 * req
> > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > > b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > > index 8be40d60ad29..a14c63438923 100644
> > > > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > > @@ -3103,7 +3103,7 @@ mt7996_mcu_beacon_cont(struct mt7996_dev
> > > > *dev,
> > > >  
> > > >  	buf = (u8 *)bcn + sizeof(*bcn);
> > > >  	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid,
> > > > NULL,
> > > > 0, 0,
> > > > -			      BSS_CHANGED_BEACON);
> > > > +			      BSS_CHANGED_BEACON, link_conf-
> > > > > link_id);
> > > >  
> > > >  	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
> > > >  }
> > > > @@ -3249,7 +3249,8 @@ int mt7996_mcu_beacon_inband_discov(struct
> > > > mt7996_dev *dev,
> > > >  
> > > >  	buf = (u8 *)tlv + sizeof(*discov);
> > > >  
> > > > -	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid,
> > > > NULL,
> > > > 0, 0, changed);
> > > > +	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid,
> > > > NULL,
> > > > 0, 0,
> > > > +			      changed, link_conf->link_id);
> > > >  
> > > >  	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
> > > >  
> > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > > b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > > index 0dc4198fcf8b..0d6488522ba7 100644
> > > > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > > @@ -874,7 +874,8 @@ void mt7996_mac_enable_nf(struct mt7996_dev
> > > > *dev,
> > > > u8 band);
> > > >  void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32 *txwi,
> > > >  			   struct sk_buff *skb, struct mt76_wcid
> > > > *wcid,
> > > >  			   struct ieee80211_key_conf *key, int
> > > > pid,
> > > > -			   enum mt76_txq_id qid, u32 changed);
> > > > +			   enum mt76_txq_id qid, u32 changed,
> > > > +			   unsigned int link_id);
> > > >  void mt7996_mac_update_beacons(struct mt7996_phy *phy);
> > > >  void mt7996_mac_set_coverage_class(struct mt7996_phy *phy);
> > > >  void mt7996_mac_work(struct work_struct *work);
> > > > 
> > > > ---
> > > > base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
> > > > change-id: 20260530-mt76_tx_status_skb_add-overwrite-fix-
> > > > 85818a9bb31f
> > > > 
> > > > Best regards,
> > > > 
> > > > 
> > > We might expand flags further so this still doesn't solve the issue
> > > of
> > > flags being cleared - it only works for MLO flag. And the
> > > developers
> > > still won't easily notice that the flags are being cleared.
> > 
> > My opinion is we should consider just upstream code and then change
> > it as soon
> > as you post this new feature upstream, but I will let Felix comments
> > on it.
> > Moreover, the proposed approach aligns link_id used in
> > mt7996_tx_prepare_skb()
> > to the one used in mt7996_mac_write_txwi() and fix a possible OOB bug
> > in
> > mt7996_mac_write_txwi().
> > 
> > Regards,
> > Lorenzo
> > 
> > > 
> 
> Just to tie in with this patch subject - I'm just thinking of a way to
> solve this once and for all. If the problem is reading zeroed info-
> >control.flags, wouldn't it be better to just pass a u32 flags,
> something like this:
> 
> u32 flags = info->control.flags
> 
> mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL, 0, 0,
> 			      changed, flags);
> 
> We can use all flags then.

what about link_id? Should it be the same between mt7996_tx_prepare_skb()
and mt7996_mac_write_txwi()?

Regards,
Lorenzo

> 
> Ryder

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()

[Ryder Lee]

On Sun, 2026-05-31 at 15:12 +0200, lorenzo@kernel.org wrote:
> On May 31, Ryder Lee wrote:
> > On Sun, 2026-05-31 at 14:11 +0200, lorenzo@kernel.org wrote:
> > > > On Sun, 2026-05-31 at 10:55 +0200, Lorenzo Bianconi wrote:
> > > > > mt76_tx_status_skb_add() zeroes the mt76_tx_cb struct stored
> > > > > at
> > > > > info->status.status_driver_data via memset(). Since info-
> > > > > >control
> > > > > and
> > > > > info->status are members of the same union in
> > > > > ieee80211_tx_info,
> > > > > this overwrites info->control.flags.
> > > > > In mt7996_tx_prepare_skb(), mt76_tx_status_skb_add() is
> > > > > called
> > > > > before
> > > > > mt7996_mac_write_txwi(), which re-reads info->control.flags
> > > > > to
> > > > > extract
> > > > > IEEE80211_TX_CTRL_MLO_LINK. Because the field has been
> > > > > zeroed,
> > > > > the
> > > > > link_id always resolves to 0 for frames using global_wcid,
> > > > > leading to
> > > > > incorrect TXWI configuration.
> > > > > Fix this by passing link_id as an explicit parameter to
> > > > > mt7996_mac_write_txwi(). In mt7996_tx_prepare_skb(), the
> > > > > link_id
> > > > > is
> > > > > already extracted from info->control.flags before the
> > > > > destructive
> > > > > mt76_tx_status_skb_add() call. For the beacon and inband
> > > > > discovery
> > > > > callers in mcu.c, use link_conf->link_id directly.
> > > > > 
> > > > > Fixes: f0b0b239b8f36 ("wifi: mt76: mt7996: rework
> > > > > mt7996_mac_write_txwi() for MLO support")
> > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > > ---
> > > > > Changes in v2:
> > > > > - Do not use link_id in mt7996_mac_write_txwi if it is
> > > > > IEEE80211_LINK_UNSPECIFIED
> > > > > - In mt7996_mac_write_txwi() rely on link_id calculated in
> > > > >   mt7996_tx_prepare_skb().
> > > > > - Link to v1:
> > > > > https://lore.kernel.org/r/20260530-mt76_tx_status_skb_add-overwrite-fix-v1-1-e2c3151c391a@kernel.org
> > > > >  
> > > > > ---
> > > > >  drivers/net/wireless/mediatek/mt76/mt7996/mac.c    | 14
> > > > > ++++----
> > > > > ----
> > > > > --
> > > > >  drivers/net/wireless/mediatek/mt76/mt7996/mcu.c    |  5 +++-
> > > > > -
> > > > >  drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h |  3 ++-
> > > > >  3 files changed, 9 insertions(+), 13 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > > > b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > > > index c98446057282..95b3078d9667 100644
> > > > > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > > > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > > > @@ -856,7 +856,8 @@ mt7996_mac_write_txwi_80211(struct
> > > > > mt7996_dev
> > > > > *dev, __le32 *txwi,
> > > > >  void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32
> > > > > *txwi,
> > > > >  			   struct sk_buff *skb, struct
> > > > > mt76_wcid
> > > > > *wcid,
> > > > >  			   struct ieee80211_key_conf *key,
> > > > > int
> > > > > pid,
> > > > > -			   enum mt76_txq_id qid, u32
> > > > > changed)
> > > > > +			   enum mt76_txq_id qid, u32
> > > > > changed,
> > > > > +			   unsigned int link_id)
> > > > >  {
> > > > >  	struct ieee80211_hdr *hdr = (struct ieee80211_hdr
> > > > > *)skb-
> > > > > > data;
> > > > >  	struct ieee80211_tx_info *info =
> > > > > IEEE80211_SKB_CB(skb);
> > > > > @@ -866,7 +867,6 @@ void mt7996_mac_write_txwi(struct
> > > > > mt7996_dev
> > > > > *dev, __le32 *txwi,
> > > > >  	bool is_8023 = info->flags &
> > > > > IEEE80211_TX_CTL_HW_80211_ENCAP;
> > > > >  	struct mt76_vif_link *mlink = NULL;
> > > > >  	struct mt7996_vif *mvif;
> > > > > -	unsigned int link_id;
> > > > >  	u16 tx_count = 15;
> > > > >  	u32 val;
> > > > >  	bool inband_disc = !!(changed &
> > > > > (BSS_CHANGED_UNSOL_BCAST_PROBE_RESP |
> > > > > @@ -874,17 +874,11 @@ void mt7996_mac_write_txwi(struct
> > > > > mt7996_dev
> > > > > *dev, __le32 *txwi,
> > > > >  	bool beacon = !!(changed & (BSS_CHANGED_BEACON |
> > > > >  				   
> > > > > BSS_CHANGED_BEACON_ENABLED))
> > > > > &&
> > > > > (!inband_disc);
> > > > >  
> > > > > -	if (wcid != &dev->mt76.global_wcid)
> > > > > -		link_id = wcid->link_id;
> > > > > -	else
> > > > > -		link_id = u32_get_bits(info->control.flags,
> > > > > -				      
> > > > > IEEE80211_TX_CTRL_MLO_LINK);
> > > > > -
> > > > >  	mvif = vif ? (struct mt7996_vif *)vif->drv_priv :
> > > > > NULL;
> > > > >  	if (mvif) {
> > > > >  		if (wcid->offchannel)
> > > > >  			mlink = rcu_dereference(mvif-
> > > > > > mt76.offchannel_link);
> > > > > -		if (!mlink)
> > > > > +		if (!mlink && link_id !=
> > > > > IEEE80211_LINK_UNSPECIFIED)
> > > > >  			mlink = rcu_dereference(mvif-
> > > > > > mt76.link[link_id]);
> > > > >  	}
> > > > >  
> > > > > @@ -1096,7 +1090,7 @@ int mt7996_tx_prepare_skb(struct
> > > > > mt76_dev
> > > > > *mdev, void *txwi_ptr,
> > > > >  	/* Transmit non qos data by 802.11 header and need
> > > > > to
> > > > > fill
> > > > > txd by host*/
> > > > >  	if (!is_8023 || pid >= MT_PACKET_ID_FIRST)
> > > > >  		mt7996_mac_write_txwi(dev, txwi_ptr,
> > > > > tx_info-
> > > > > > skb,
> > > > > wcid, key,
> > > > > -				      pid, qid, 0);
> > > > > +				      pid, qid, 0, link_id);
> > > > >  
> > > > >  	/* MT7996 and MT7992 require driver to provide the
> > > > > MAC
> > > > > TXP
> > > > > for AddBA
> > > > >  	 * req
> > > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > > > b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > > > index 8be40d60ad29..a14c63438923 100644
> > > > > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > > > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > > > @@ -3103,7 +3103,7 @@ mt7996_mcu_beacon_cont(struct
> > > > > mt7996_dev
> > > > > *dev,
> > > > >  
> > > > >  	buf = (u8 *)bcn + sizeof(*bcn);
> > > > >  	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid,
> > > > > NULL,
> > > > > 0, 0,
> > > > > -			      BSS_CHANGED_BEACON);
> > > > > +			      BSS_CHANGED_BEACON, link_conf-
> > > > > > link_id);
> > > > >  
> > > > >  	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
> > > > >  }
> > > > > @@ -3249,7 +3249,8 @@ int
> > > > > mt7996_mcu_beacon_inband_discov(struct
> > > > > mt7996_dev *dev,
> > > > >  
> > > > >  	buf = (u8 *)tlv + sizeof(*discov);
> > > > >  
> > > > > -	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid,
> > > > > NULL,
> > > > > 0, 0, changed);
> > > > > +	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid,
> > > > > NULL,
> > > > > 0, 0,
> > > > > +			      changed, link_conf->link_id);
> > > > >  
> > > > >  	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
> > > > >  
> > > > > diff --git
> > > > > a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > > > b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > > > index 0dc4198fcf8b..0d6488522ba7 100644
> > > > > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > > > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > > > @@ -874,7 +874,8 @@ void mt7996_mac_enable_nf(struct
> > > > > mt7996_dev
> > > > > *dev,
> > > > > u8 band);
> > > > >  void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32
> > > > > *txwi,
> > > > >  			   struct sk_buff *skb, struct
> > > > > mt76_wcid
> > > > > *wcid,
> > > > >  			   struct ieee80211_key_conf *key,
> > > > > int
> > > > > pid,
> > > > > -			   enum mt76_txq_id qid, u32
> > > > > changed);
> > > > > +			   enum mt76_txq_id qid, u32
> > > > > changed,
> > > > > +			   unsigned int link_id);
> > > > >  void mt7996_mac_update_beacons(struct mt7996_phy *phy);
> > > > >  void mt7996_mac_set_coverage_class(struct mt7996_phy *phy);
> > > > >  void mt7996_mac_work(struct work_struct *work);
> > > > > 
> > > > > ---
> > > > > base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
> > > > > change-id: 20260530-mt76_tx_status_skb_add-overwrite-fix-
> > > > > 85818a9bb31f
> > > > > 
> > > > > Best regards,
> > > > > 
> > > > > 
> > > > We might expand flags further so this still doesn't solve the
> > > > issue
> > > > of
> > > > flags being cleared - it only works for MLO flag. And the
> > > > developers
> > > > still won't easily notice that the flags are being cleared.
> > > 
> > > My opinion is we should consider just upstream code and then
> > > change
> > > it as soon
> > > as you post this new feature upstream, but I will let Felix
> > > comments
> > > on it.
> > > Moreover, the proposed approach aligns link_id used in
> > > mt7996_tx_prepare_skb()
> > > to the one used in mt7996_mac_write_txwi() and fix a possible OOB
> > > bug
> > > in
> > > mt7996_mac_write_txwi().
> > > 
> > > Regards,
> > > Lorenzo
> > > 
> > > > 
> > 
> > Just to tie in with this patch subject - I'm just thinking of a way
> > to
> > solve this once and for all. If the problem is reading zeroed info-
> > > control.flags, wouldn't it be better to just pass a u32 flags,
> > something like this:
> > 
> > u32 flags = info->control.flags
> > 
> > mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL, 0, 0,
> > 			      changed, flags);
> > 
> > We can use all flags then.
> 
> what about link_id? Should it be the same between
> mt7996_tx_prepare_skb()
> and mt7996_mac_write_txwi()?
> 
> 
 
I mean the link_id is only corresponds to one specific flags bit of
mac80211_tx_control_flags. But there are other bits that aren't
handled. Wouldn't u32 flags make it more cleaner?

Ryder

Re: [PATCH v2] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()

[lorenzo]

[-- Attachment #1: Type: text/plain, Size: 10719 bytes --]

On May 31, Ryder Lee wrote:
> On Sun, 2026-05-31 at 15:12 +0200, lorenzo@kernel.org wrote:
> > On May 31, Ryder Lee wrote:
> > > On Sun, 2026-05-31 at 14:11 +0200, lorenzo@kernel.org wrote:
> > > > > On Sun, 2026-05-31 at 10:55 +0200, Lorenzo Bianconi wrote:
> > > > > > mt76_tx_status_skb_add() zeroes the mt76_tx_cb struct stored
> > > > > > at
> > > > > > info->status.status_driver_data via memset(). Since info-
> > > > > > >control
> > > > > > and
> > > > > > info->status are members of the same union in
> > > > > > ieee80211_tx_info,
> > > > > > this overwrites info->control.flags.
> > > > > > In mt7996_tx_prepare_skb(), mt76_tx_status_skb_add() is
> > > > > > called
> > > > > > before
> > > > > > mt7996_mac_write_txwi(), which re-reads info->control.flags
> > > > > > to
> > > > > > extract
> > > > > > IEEE80211_TX_CTRL_MLO_LINK. Because the field has been
> > > > > > zeroed,
> > > > > > the
> > > > > > link_id always resolves to 0 for frames using global_wcid,
> > > > > > leading to
> > > > > > incorrect TXWI configuration.
> > > > > > Fix this by passing link_id as an explicit parameter to
> > > > > > mt7996_mac_write_txwi(). In mt7996_tx_prepare_skb(), the
> > > > > > link_id
> > > > > > is
> > > > > > already extracted from info->control.flags before the
> > > > > > destructive
> > > > > > mt76_tx_status_skb_add() call. For the beacon and inband
> > > > > > discovery
> > > > > > callers in mcu.c, use link_conf->link_id directly.
> > > > > > 
> > > > > > Fixes: f0b0b239b8f36 ("wifi: mt76: mt7996: rework
> > > > > > mt7996_mac_write_txwi() for MLO support")
> > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > > > ---
> > > > > > Changes in v2:
> > > > > > - Do not use link_id in mt7996_mac_write_txwi if it is
> > > > > > IEEE80211_LINK_UNSPECIFIED
> > > > > > - In mt7996_mac_write_txwi() rely on link_id calculated in
> > > > > >   mt7996_tx_prepare_skb().
> > > > > > - Link to v1:
> > > > > > https://lore.kernel.org/r/20260530-mt76_tx_status_skb_add-overwrite-fix-v1-1-e2c3151c391a@kernel.org
> > > > > >  
> > > > > > ---
> > > > > >  drivers/net/wireless/mediatek/mt76/mt7996/mac.c    | 14
> > > > > > ++++----
> > > > > > ----
> > > > > > --
> > > > > >  drivers/net/wireless/mediatek/mt76/mt7996/mcu.c    |  5 +++-
> > > > > > -
> > > > > >  drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h |  3 ++-
> > > > > >  3 files changed, 9 insertions(+), 13 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > > > > b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > > > > index c98446057282..95b3078d9667 100644
> > > > > > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > > > > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > > > > @@ -856,7 +856,8 @@ mt7996_mac_write_txwi_80211(struct
> > > > > > mt7996_dev
> > > > > > *dev, __le32 *txwi,
> > > > > >  void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32
> > > > > > *txwi,
> > > > > >  			   struct sk_buff *skb, struct
> > > > > > mt76_wcid
> > > > > > *wcid,
> > > > > >  			   struct ieee80211_key_conf *key,
> > > > > > int
> > > > > > pid,
> > > > > > -			   enum mt76_txq_id qid, u32
> > > > > > changed)
> > > > > > +			   enum mt76_txq_id qid, u32
> > > > > > changed,
> > > > > > +			   unsigned int link_id)
> > > > > >  {
> > > > > >  	struct ieee80211_hdr *hdr = (struct ieee80211_hdr
> > > > > > *)skb-
> > > > > > > data;
> > > > > >  	struct ieee80211_tx_info *info =
> > > > > > IEEE80211_SKB_CB(skb);
> > > > > > @@ -866,7 +867,6 @@ void mt7996_mac_write_txwi(struct
> > > > > > mt7996_dev
> > > > > > *dev, __le32 *txwi,
> > > > > >  	bool is_8023 = info->flags &
> > > > > > IEEE80211_TX_CTL_HW_80211_ENCAP;
> > > > > >  	struct mt76_vif_link *mlink = NULL;
> > > > > >  	struct mt7996_vif *mvif;
> > > > > > -	unsigned int link_id;
> > > > > >  	u16 tx_count = 15;
> > > > > >  	u32 val;
> > > > > >  	bool inband_disc = !!(changed &
> > > > > > (BSS_CHANGED_UNSOL_BCAST_PROBE_RESP |
> > > > > > @@ -874,17 +874,11 @@ void mt7996_mac_write_txwi(struct
> > > > > > mt7996_dev
> > > > > > *dev, __le32 *txwi,
> > > > > >  	bool beacon = !!(changed & (BSS_CHANGED_BEACON |
> > > > > >  				   
> > > > > > BSS_CHANGED_BEACON_ENABLED))
> > > > > > &&
> > > > > > (!inband_disc);
> > > > > >  
> > > > > > -	if (wcid != &dev->mt76.global_wcid)
> > > > > > -		link_id = wcid->link_id;
> > > > > > -	else
> > > > > > -		link_id = u32_get_bits(info->control.flags,
> > > > > > -				      
> > > > > > IEEE80211_TX_CTRL_MLO_LINK);
> > > > > > -
> > > > > >  	mvif = vif ? (struct mt7996_vif *)vif->drv_priv :
> > > > > > NULL;
> > > > > >  	if (mvif) {
> > > > > >  		if (wcid->offchannel)
> > > > > >  			mlink = rcu_dereference(mvif-
> > > > > > > mt76.offchannel_link);
> > > > > > -		if (!mlink)
> > > > > > +		if (!mlink && link_id !=
> > > > > > IEEE80211_LINK_UNSPECIFIED)
> > > > > >  			mlink = rcu_dereference(mvif-
> > > > > > > mt76.link[link_id]);
> > > > > >  	}
> > > > > >  
> > > > > > @@ -1096,7 +1090,7 @@ int mt7996_tx_prepare_skb(struct
> > > > > > mt76_dev
> > > > > > *mdev, void *txwi_ptr,
> > > > > >  	/* Transmit non qos data by 802.11 header and need
> > > > > > to
> > > > > > fill
> > > > > > txd by host*/
> > > > > >  	if (!is_8023 || pid >= MT_PACKET_ID_FIRST)
> > > > > >  		mt7996_mac_write_txwi(dev, txwi_ptr,
> > > > > > tx_info-
> > > > > > > skb,
> > > > > > wcid, key,
> > > > > > -				      pid, qid, 0);
> > > > > > +				      pid, qid, 0, link_id);
> > > > > >  
> > > > > >  	/* MT7996 and MT7992 require driver to provide the
> > > > > > MAC
> > > > > > TXP
> > > > > > for AddBA
> > > > > >  	 * req
> > > > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > > > > b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > > > > index 8be40d60ad29..a14c63438923 100644
> > > > > > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > > > > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> > > > > > @@ -3103,7 +3103,7 @@ mt7996_mcu_beacon_cont(struct
> > > > > > mt7996_dev
> > > > > > *dev,
> > > > > >  
> > > > > >  	buf = (u8 *)bcn + sizeof(*bcn);
> > > > > >  	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid,
> > > > > > NULL,
> > > > > > 0, 0,
> > > > > > -			      BSS_CHANGED_BEACON);
> > > > > > +			      BSS_CHANGED_BEACON, link_conf-
> > > > > > > link_id);
> > > > > >  
> > > > > >  	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
> > > > > >  }
> > > > > > @@ -3249,7 +3249,8 @@ int
> > > > > > mt7996_mcu_beacon_inband_discov(struct
> > > > > > mt7996_dev *dev,
> > > > > >  
> > > > > >  	buf = (u8 *)tlv + sizeof(*discov);
> > > > > >  
> > > > > > -	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid,
> > > > > > NULL,
> > > > > > 0, 0, changed);
> > > > > > +	mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid,
> > > > > > NULL,
> > > > > > 0, 0,
> > > > > > +			      changed, link_conf->link_id);
> > > > > >  
> > > > > >  	memcpy(buf + MT_TXD_SIZE, skb->data, skb->len);
> > > > > >  
> > > > > > diff --git
> > > > > > a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > > > > b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > > > > index 0dc4198fcf8b..0d6488522ba7 100644
> > > > > > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > > > > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> > > > > > @@ -874,7 +874,8 @@ void mt7996_mac_enable_nf(struct
> > > > > > mt7996_dev
> > > > > > *dev,
> > > > > > u8 band);
> > > > > >  void mt7996_mac_write_txwi(struct mt7996_dev *dev, __le32
> > > > > > *txwi,
> > > > > >  			   struct sk_buff *skb, struct
> > > > > > mt76_wcid
> > > > > > *wcid,
> > > > > >  			   struct ieee80211_key_conf *key,
> > > > > > int
> > > > > > pid,
> > > > > > -			   enum mt76_txq_id qid, u32
> > > > > > changed);
> > > > > > +			   enum mt76_txq_id qid, u32
> > > > > > changed,
> > > > > > +			   unsigned int link_id);
> > > > > >  void mt7996_mac_update_beacons(struct mt7996_phy *phy);
> > > > > >  void mt7996_mac_set_coverage_class(struct mt7996_phy *phy);
> > > > > >  void mt7996_mac_work(struct work_struct *work);
> > > > > > 
> > > > > > ---
> > > > > > base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
> > > > > > change-id: 20260530-mt76_tx_status_skb_add-overwrite-fix-
> > > > > > 85818a9bb31f
> > > > > > 
> > > > > > Best regards,
> > > > > > 
> > > > > > 
> > > > > We might expand flags further so this still doesn't solve the
> > > > > issue
> > > > > of
> > > > > flags being cleared - it only works for MLO flag. And the
> > > > > developers
> > > > > still won't easily notice that the flags are being cleared.
> > > > 
> > > > My opinion is we should consider just upstream code and then
> > > > change
> > > > it as soon
> > > > as you post this new feature upstream, but I will let Felix
> > > > comments
> > > > on it.
> > > > Moreover, the proposed approach aligns link_id used in
> > > > mt7996_tx_prepare_skb()
> > > > to the one used in mt7996_mac_write_txwi() and fix a possible OOB
> > > > bug
> > > > in
> > > > mt7996_mac_write_txwi().
> > > > 
> > > > Regards,
> > > > Lorenzo
> > > > 
> > > > > 
> > > 
> > > Just to tie in with this patch subject - I'm just thinking of a way
> > > to
> > > solve this once and for all. If the problem is reading zeroed info-
> > > > control.flags, wouldn't it be better to just pass a u32 flags,
> > > something like this:
> > > 
> > > u32 flags = info->control.flags
> > > 
> > > mt7996_mac_write_txwi(dev, (__le32 *)buf, skb, wcid, NULL, 0, 0,
> > > 			      changed, flags);
> > > 
> > > We can use all flags then.
> > 
> > what about link_id? Should it be the same between
> > mt7996_tx_prepare_skb()
> > and mt7996_mac_write_txwi()?
> > 
> > 
>  
> I mean the link_id is only corresponds to one specific flags bit of
> mac80211_tx_control_flags. But there are other bits that aren't
> handled. Wouldn't u32 flags make it more cleaner?

Yes, I got your point, but my concern is if we need to sync link_id between
mt7996_tx_prepare_skb() and mt7996_mac_write_txwi(). If so, I guess it is
much better to pass link_id explicitly to mt7996_mac_write_txwi() since it
does not just depended on mac80211_tx_control_flags and I think we should
not duplicate the logic in mt7996_mac_write_txwi(). Got my point?
If in the future (not required now) we need to pass mac80211_tx_control_flags
to mt7996_mac_write_txwi(), we will do it easily.

Regards,
Lorenzo

> 
> Ryder
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]