* [PATCH] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()
@ 2026-05-30 15:25 Lorenzo Bianconi
2026-05-31 2:03 ` Ryder Lee
0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Bianconi @ 2026-05-30 15:25 UTC (permalink / raw)
To: Felix Fietkau, Ryder Lee, Shayne Chen, Sean Wang,
Matthias Brugger, AngeloGioacchino Del Regno, Bo Jiao, Peter Chiu,
Lorenzo Bianconi
Cc: Roy Luo, linux-wireless, linux-arm-kernel, linux-mediatek
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>
---
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);
---
base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
change-id: 20260530-mt76_tx_status_skb_add-overwrite-fix-85818a9bb31f
Best regards,
--
Lorenzo Bianconi <lorenzo@kernel•org>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()
2026-05-30 15:25 [PATCH] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add() Lorenzo Bianconi
@ 2026-05-31 2:03 ` Ryder Lee
2026-05-31 7:09 ` lorenzo
0 siblings, 1 reply; 5+ messages in thread
From: Ryder Lee @ 2026-05-31 2:03 UTC (permalink / raw)
To: Shayne Chen (陳軒丞), lorenzo@kernel•org,
nbd@nbd•name, AngeloGioacchino Del Regno,
Chui-hao Chiu (邱垂浩), Sean Wang,
Bo Jiao (焦波), matthias.bgg@gmail•com
Cc: linux-wireless@vger•kernel.org,
linux-arm-kernel@lists•infradead.org, Roy-CH Luo,
linux-mediatek@lists•infradead.org
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 <lorenzo@kernel•org>
> ---
> 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.
Ryder
>
> ---
> base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
> change-id: 20260530-mt76_tx_status_skb_add-overwrite-fix-85818a9bb31f
>
> Best regards,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()
2026-05-31 2:03 ` Ryder Lee
@ 2026-05-31 7:09 ` lorenzo
2026-05-31 10:07 ` Ryder Lee
0 siblings, 1 reply; 5+ messages in thread
From: lorenzo @ 2026-05-31 7:09 UTC (permalink / raw)
To: Ryder Lee
Cc: Shayne Chen (陳軒丞), nbd@nbd•name,
AngeloGioacchino Del Regno,
Chui-hao Chiu (邱垂浩), Sean Wang,
Bo Jiao (焦波), matthias.bgg@gmail•com,
linux-wireless@vger•kernel.org,
linux-arm-kernel@lists•infradead.org, Roy-CH Luo,
linux-mediatek@lists•infradead.org
[-- Attachment #1: Type: text/plain, Size: 6203 bytes --]
> 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 <lorenzo@kernel•org>
> > ---
> > 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
>
> Ryder
> >
> > ---
> > base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
> > change-id: 20260530-mt76_tx_status_skb_add-overwrite-fix-85818a9bb31f
> >
> > Best regards,
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()
2026-05-31 7:09 ` lorenzo
@ 2026-05-31 10:07 ` Ryder Lee
2026-05-31 12:02 ` lorenzo
0 siblings, 1 reply; 5+ messages in thread
From: Ryder Lee @ 2026-05-31 10:07 UTC (permalink / raw)
To: lorenzo@kernel•org
Cc: linux-mediatek@lists•infradead.org,
Shayne Chen (陳軒丞), nbd@nbd•name, Roy-CH Luo,
AngeloGioacchino Del Regno,
Chui-hao Chiu (邱垂浩), Sean Wang,
Bo Jiao (焦波), matthias.bgg@gmail•com,
linux-arm-kernel@lists•infradead.org,
linux-wireless@vger•kernel.org
On Sun, 2026-05-31 at 09:09 +0200, lorenzo@kernel•org 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 <lorenzo@kernel•org>
> > > ---
> > > 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add()
2026-05-31 10:07 ` Ryder Lee
@ 2026-05-31 12:02 ` lorenzo
0 siblings, 0 replies; 5+ messages in thread
From: lorenzo @ 2026-05-31 12:02 UTC (permalink / raw)
To: Ryder Lee
Cc: linux-mediatek@lists•infradead.org,
Shayne Chen (陳軒丞), nbd@nbd•name, Roy-CH Luo,
AngeloGioacchino Del Regno,
Chui-hao Chiu (邱垂浩), Sean Wang,
Bo Jiao (焦波), matthias.bgg@gmail•com,
linux-arm-kernel@lists•infradead.org,
linux-wireless@vger•kernel.org
[-- Attachment #1: Type: text/plain, Size: 7475 bytes --]
On May 31, Ryder Lee wrote:
> On Sun, 2026-05-31 at 09:09 +0200, lorenzo@kernel•org 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 <lorenzo@kernel•org>
> > > > ---
> > > > 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.
But this is necessary for non-upstream code IIUC. Are you going to post it
upstream along with the fix?
I guess it is better to align the link_id used in mt7996_mac_write_txwi()
to the one used in mt7996_mac_write_txwi(). Please take a look to v3.
Regards,
Lorenzo
>
> Ryder
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-31 12:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-30 15:25 [PATCH] wifi: mt76: mt7996: fix reading zeroed info->control.flags after mt76_tx_status_skb_add() Lorenzo Bianconi
2026-05-31 2:03 ` Ryder Lee
2026-05-31 7:09 ` lorenzo
2026-05-31 10:07 ` Ryder Lee
2026-05-31 12:02 ` lorenzo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox