On Fri, Jun 23, 2023 at 05:24:00PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@xxxxxxxx>
>
> 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@xxxxxxxx>
> ---
> 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@xxxxxxxxxxxx>
--
Kees Cook
