From: Lorenzo Bianconi <lorenzo@kernel.org>
To: Ryder Lee <ryder.lee@mediatek.com>
Cc: Felix Fietkau <nbd@nbd.name>,
linux-mediatek@lists.infradead.org,
linux-wireless@vger.kernel.org,
Shayne Chen <shayne.chen@mediatek.com>,
Roy Luo <roy-ch.luo@mediatek.com>
Subject: Re: [PATCH] wifi: mt76: mt7996: avoid memset overwriting tx_info->control.flags
Date: Fri, 15 May 2026 18:29:08 +0200 [thread overview]
Message-ID: <agdJ1EtZjjWh2R1h@lore-desk> (raw)
In-Reply-To: <5ecac6a9b7d29526e8438dea105b58f5487c93aa.1778521232.git.ryder.lee@mediatek.com>
[-- Attachment #1: Type: text/plain, Size: 8864 bytes --]
> mt76_tx_status_skb_add uses memset on status.status_driver_data, which
> overwrite info->control.flags in ieee80211_tx_info. Copy tx_info before
> calling mt76_tx_status_skb_add to ensure control fields are preserved
> and remain valid for later processing.
>
> Reported-By: Roy Luo <roy-ch.luo@mediatek.com>
> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
Hi Ryder,
I have not completely got the issue here:
struct ieee80211_tx_info {
u32 flags; /* 0 4 */
u32 band:3; /* 4: 0 4 */
u32 status_data_idr:1; /* 4: 3 4 */
u32 status_data:13; /* 4: 4 4 */
u32 hw_queue:4; /* 4:17 4 */
u32 tx_time_est:10; /* 4:21 4 */
/* XXX 1 bit hole, try to pack */
union {
struct {
union {
struct {
struct ieee80211_tx_rate rates[4]; /* 8 12 */
s8 rts_cts_rate_idx; /* 20 1 */
u8 use_rts:1; /* 21: 0 1 */
u8 use_cts_prot:1; /* 21: 1 1 */
u8 short_preamble:1; /* 21: 2 1 */
u8 skip_table:1; /* 21: 3 1 */
u8 antennas:2; /* 21: 4 1 */
}; /* 8 14 */
long unsigned int jiffies; /* 8 8 */
}; /* 8 16 */
struct ieee80211_vif * vif; /* 24 8 */
struct ieee80211_key_conf * hw_key; /* 32 8 */
u32 flags; /* 40 4 */
codel_time_t enqueue_time; /* 44 4 */
} control; /* 8 40 */
struct {
u64 cookie; /* 8 8 */
} ack; /* 8 8 */
struct {
struct ieee80211_tx_rate rates[4]; /* 8 12 */
s32 ack_signal; /* 20 4 */
u8 ampdu_ack_len; /* 24 1 */
u8 ampdu_len; /* 25 1 */
u8 antenna; /* 26 1 */
u8 pad; /* 27 1 */
u16 tx_time; /* 28 2 */
u8 flags; /* 30 1 */
u8 pad2; /* 31 1 */
void * status_driver_data[2]; /* 32 16 */
} status; /* 8 40 */
struct {
struct ieee80211_tx_rate driver_rates[4]; /* 8 12 */
u8 pad[4]; /* 20 4 */
void * rate_driver_data[3]; /* 24 24 */
}; /* 8 40 */
void * driver_data[5]; /* 8 40 */
}; /* 8 40 */
/* size: 48, cachelines: 1, members: 7 */
/* sum members: 44 */
/* sum bitfield members: 31 bits, bit holes: 1, sum bit holes: 1 bits */
/* last cacheline: 48 bytes */
};
According to pahole, the size of the control inner union is actually 16 bytes
since the compiler adds 2 bytes of padding. Since mt76_tx_status_skb_add()
meset to 0 just mt76_tx_cb size (that is 16 bytes) I can't see how
control.flags is overwritten. Am I missing something?
struct mt76_tx_cb {
long unsigned int jiffies; /* 0 8 */
u16 wcid; /* 8 2 */
u8 pktid; /* 10 1 */
u8 flags; /* 11 1 */
/* size: 16, cachelines: 1, members: 4 */
/* padding: 4 */
/* last cacheline: 16 bytes */
};
Regards,
Lorenzo
> ---
> drivers/net/wireless/mediatek/mt76/mt7996/mac.c | 17 +++++++++--------
> .../net/wireless/mediatek/mt76/mt7996/mt7996.h | 2 +-
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> index e2a83da3a09c..a59c14c8f2af 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> @@ -855,11 +855,12 @@ 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,
> + struct ieee80211_tx_info *tx_info, int pid,
> enum mt76_txq_id qid, u32 changed)
> {
> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> - struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> + struct ieee80211_key_conf *key = tx_info ? tx_info->control.hw_key : NULL;
> + struct ieee80211_tx_info *info = tx_info ? tx_info : IEEE80211_SKB_CB(skb);
> struct ieee80211_vif *vif = info->control.vif;
> u8 band_idx = (info->hw_queue & MT_TX_HW_QUEUE_PHY) >> 2;
> u8 p_fmt, q_idx, omac_idx = 0, wmm_idx = 0;
> @@ -1006,15 +1007,15 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
> {
> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx_info->skb->data;
> struct mt7996_dev *dev = container_of(mdev, struct mt7996_dev, mt76);
> - struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx_info->skb);
> - struct ieee80211_key_conf *key = info->control.hw_key;
> - struct ieee80211_vif *vif = info->control.vif;
> + struct ieee80211_tx_info info = *IEEE80211_SKB_CB(tx_info->skb);
> + struct ieee80211_key_conf *key = info.control.hw_key;
> + struct ieee80211_vif *vif = info.control.vif;
> struct mt7996_vif *mvif = vif ? (struct mt7996_vif *)vif->drv_priv : NULL;
> struct mt7996_sta *msta = sta ? (struct mt7996_sta *)sta->drv_priv : NULL;
> struct mt76_vif_link *mlink = NULL;
> struct mt76_txwi_cache *t;
> int id, i, pid, nbuf = tx_info->nbuf - 1;
> - bool is_8023 = info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP;
> + bool is_8023 = info.flags & IEEE80211_TX_CTL_HW_80211_ENCAP;
> __le32 *ptr = (__le32 *)txwi_ptr;
> u8 *txwi = (u8 *)txwi_ptr;
> u8 link_id;
> @@ -1031,7 +1032,7 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
>
> link_id = (tid % 2) ? msta->seclink_id : msta->deflink_id;
> } else {
> - link_id = u32_get_bits(info->control.flags,
> + link_id = u32_get_bits(info.control.flags,
> IEEE80211_TX_CTRL_MLO_LINK);
> }
>
> @@ -1095,7 +1096,7 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
> memset(txwi_ptr, 0, MT_TXD_SIZE);
> /* 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,
> + mt7996_mac_write_txwi(dev, txwi_ptr, tx_info->skb, wcid, &info,
> pid, qid, 0);
>
> /* MT7996 and MT7992 require driver to provide the MAC TXP for AddBA
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> index bdcf72457954..319fcc4fb469 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> @@ -854,7 +854,7 @@ void mt7996_mac_cca_stats_reset(struct mt7996_phy *phy);
> 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,
> + struct ieee80211_tx_info *tx_info, int pid,
> enum mt76_txq_id qid, u32 changed);
> void mt7996_mac_update_beacons(struct mt7996_phy *phy);
> void mt7996_mac_set_coverage_class(struct mt7996_phy *phy);
> --
> 2.45.2
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2026-05-15 16:29 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-11 18:32 [PATCH] wifi: mt76: mt7996: avoid memset overwriting tx_info->control.flags Ryder Lee
2026-05-15 16:29 ` Lorenzo Bianconi [this message]
2026-05-15 18:04 ` Cheng Hao Luo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=agdJ1EtZjjWh2R1h@lore-desk \
--to=lorenzo@kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-wireless@vger.kernel.org \
--cc=nbd@nbd.name \
--cc=roy-ch.luo@mediatek.com \
--cc=ryder.lee@mediatek.com \
--cc=shayne.chen@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox