[PATCH] mt76: mt7915: fix a couple information leaks
Felix Fietkau
nbd at nbd.name
Fri Jan 7 02:08:28 PST 2022
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
More information about the Linux-mediatek
mailing list