Re: [PATCH] mt76: mt7915: fix a couple information leaks

[Johannes Berg <johannes@sipsolutions.net>]

On Fri, 2022-01-07 at 10:36 +0300, Dan Carpenter wrote:
> Unfortunately this code has stumbled into some deep C standards
> nonsense.  These two structs have a 3 byte struct hole at the end.  If
> you partially initialize a struct then the C standard specifies that
> all the struct holes are zeroed out.  But when you initialize all the
> members of the struct, as this code does, then struct holes may be left
> with uninitialized stack data.  This is from C11 section 6.7.9 and how
> it is implemented in GCC.

Wow, nice find ...

> +	memset(&data, 0, sizeof(data));
> +	data.cmd = cpu_to_le32(MURU_SET_TXC_TX_STATS_EN);
> +	data.enable = enabled;
> 

Maybe add a comment? This is not going to be obvious in the future.

>  	return mt76_mcu_send_msg(&dev->mt76, MCU_EXT_CMD(MURU_CTRL),
> &data,
>  				sizeof(data), false);

Or maybe instead just mark the thing __packed (and/or explicitly add the
padding if needed), it seems weird that we'd send something to the
*firmware* that has a struct layout subject to compiler/arch padding
rules.

johannes