| Message ID | 20230623152443.2296825-2-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [1/2] carl9170: re-fix fortified-memset warning | expand |
On Fri, Jun 23, 2023 at 05:24:00PM +0200, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > While looking at a bug, I got rather confused by the layout of the > 'status' field in ieee80211_tx_info. Apparently, the intention is that > status_driver_data[] is used for driver specific data, and fills up the > size of the union to 40 bytes, just like the other ones. > > This is indeed what actually happens, but only because of the > combination of two mistakes: > > - "void *status_driver_data[18 / sizeof(void *)];" is intended > to be 18 bytes long but is actually two bytes shorter because of > rounding-down in the division, to a multiple of the pointer > size (4 bytes or 8 bytes). > > - The other fields combined are intended to be 22 bytes long, but > are actually 24 bytes because of padding in front of the > unaligned tx_time member, and in front of the pointer array. > > The two mistakes cancel out. so the size ends up fine, but it seems > more helpful to make this explicit, by having a multiple of 8 bytes > in the size calculation and explicitly describing the padding. > > Fixes: ea5907db2a9cc ("mac80211: fix struct ieee80211_tx_info size") > Fixes: 02219b3abca59 ("mac80211: add WMM admission control support") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > include/net/mac80211.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/net/mac80211.h b/include/net/mac80211.h > index 3a8a2d2c58c38..ca4dc8a14f1bb 100644 > --- a/include/net/mac80211.h > +++ b/include/net/mac80211.h > @@ -1192,9 +1192,11 @@ struct ieee80211_tx_info { > u8 ampdu_ack_len; > u8 ampdu_len; > u8 antenna; > + u8 pad; > u16 tx_time; > u8 flags; > - void *status_driver_data[18 / sizeof(void *)]; > + u8 pad2; > + void *status_driver_data[16 / sizeof(void *)]; > } status; pahole agrees with your assessment. :) struct ieee80211_tx_info { ... union { ... 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 */ /* XXX 1 byte hole, try to pack */ u16 tx_time; /* 28 2 */ u8 flags; /* 30 1 */ /* XXX 1 byte hole, try to pack */ 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 */ ... }; Reviewed-by: Kees Cook <keescook@chromium.org>
diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 3a8a2d2c58c38..ca4dc8a14f1bb 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -1192,9 +1192,11 @@ struct ieee80211_tx_info { u8 ampdu_ack_len; u8 ampdu_len; u8 antenna; + u8 pad; u16 tx_time; u8 flags; - void *status_driver_data[18 / sizeof(void *)]; + u8 pad2; + void *status_driver_data[16 / sizeof(void *)]; } status; struct { struct ieee80211_tx_rate driver_rates[