Search Linux Wireless

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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022-01-07 10:18, Johannes Berg wrote:
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.
I would also prefer explicitly adding the padding and leaving the rest of the code as-is.

- Felix



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux