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

From: Ryder Lee <[email-masked]>
Date: Sun, 31 May 2026 10:07:56 +0000

On Sun, 2026-05-31 at 09:09 +0200, [email-masked] wrote:
> > On Sat, 2026-05-30 at 17:25 +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 <[email-masked]>
> > > ---
> > >  drivers/net/wireless/mediatek/mt76/mt7996/mac.c    | 9 +++------
> > >  drivers/net/wireless/mediatek/mt76/mt7996/mcu.c    | 5 +++--
> > >  drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h | 3 ++-
> > >  3 files changed, 8 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > > index c98446057282..2d3f80b3e41a 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 |
> > > @@ -876,9 +876,6 @@ void mt7996_mac_write_txwi(struct mt7996_dev
> > > *dev, __le32 *txwi,
> > >  
> > >  	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) {
> > > @@ -1096,7 +1093,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);
> > 
> > The reason we didn't make the same change is because we use other
> > control flags (IEEE80211_TX_CTRL*) of info->control.flags not just
> > MLO
> > one. So with this change we still need to copy over the other flags
> > and
> > pass them in as well.
> 
> Do you mean you are using info->control.flags in
> mt7996_mac_write_txwi() in
> some downstream code? If so, I guess you can use a similar approach
> and
> just pass the required field. Copy the full ieee80211_tx_info struct
> on
> per-packet basis seems unnecessary.
> 
> Regards,
> Lorenzo
> > 
> > 

We will use 3 mac80211_tx_control_flags (i.e.
IEEE80211_TX_CTRL_RATE_INJECT), so it's better to at least copy info-
>control.flags directly and pass them in. If you pass each
individually, the function will end up with 12 parameters, and if more
flags are needed in the future, you will have to add even more.

Ryder