[PATCH] wifi: mt76: mt7996: avoid memset overwriting tx_info->control.flags

[Lorenzo Bianconi]

>
> On Mon, May 18, 2026 at 5:31 AM Lorenzo Bianconi <lorenzo at kernel.org> wrote:
> >
> > On May 15, Cheng Hao Luo wrote:
> > > > 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 */
> > > > };
> > >
> > > Hi Lorenzo,
> > >
> > > The mt76_tx_cb is placed at status.status_driver_data (offset 32).
> > > It overlaps with hw_key, flags and enqueue_time in the control union.
> > >
> > > static inline struct mt76_tx_cb *mt76_tx_skb_cb(struct sk_buff *skb)
> > > {
> > > BUILD_BUG_ON(sizeof(struct mt76_tx_cb) >
> > >     sizeof(IEEE80211_SKB_CB(skb)->status.status_driver_data));
> > > return ((void *)IEEE80211_SKB_CB(skb)->status.status_driver_data);
> > > }
> >
> > Hi Roy,
> >
> > I still do not understand since mt76_tx_status_skb_add() sets to 0 just sizeof
> > of mt76_tx_cb, that according to pahole is 16 bytes, so it can't overwrite
> > hw_key pointer (whose offset respect to the beginning of the control struct is
> > 24, 32 - 8).
> >
> > Regards,
> > Lorenzo
> >
> > >
> > > Regards,
> > > Roy Luo
>
> Hi Lorenzo,
>
> The mt76_tx_status_skb_add() memset zero the 16 bytes starting from
> status.status_driver_data (please see the above inline function shared
> in my last response) whose offset with respect to the beginning of
> the control/status union is exactly 24 (32 - 8) instead of 0.
>
> Regards,
> Roy Luo

Hi Roy,

I can see the issue now, I was confusing status.status_driver_data
with
driver_data. You are right, we have an issue here. However, copying
all the
ieee80211_tx_info struct seems a bit overkill, what do you think?
Moreover, we have the same issue for various chipsets (e.g. mt7925 and
mt7915).  I guess we should try to find a global solution for the
problem.

Regards,
Lorenzo