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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.