Re: [Intel-wired-lan] [PATCH net 2/3] e1000: fix endianness conversion of uninitialized words

Jacob Keller
2026-03-25T23:01:53Z

On 3/25/2026 8:19 AM, Fedor Pchelkin wrote:
> Hi,
>
> On Tue, 24. Mar 16:26, Tony Nguyen wrote:
>> On 3/18/2026 5:05 AM, Agalakov Daniil wrote:
>>> [Why]
>>> In e1000_set_eeprom(), the eeprom_buff is allocated to hold a range of
>>> words. However, only the boundary words (the first and the last) are
>>> populated from the EEPROM if the write request is not word-aligned.
>>> The words in the middle of the buffer remain uninitialized because they
>>> are intended to be completely overwritten by the new data via memcpy().
>>>
>>> The previous implementation had a loop that performed le16_to_cpus()
>>> on the entire buffer. This resulted in endianness conversion being
>>> performed on uninitialized memory for all interior words.
>>>
>>> Fix this by converting the endianness only for the boundary words
>>> immediately after they are successfully read from the EEPROM.
>>>
>>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>>>
>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>
>> While this is definitely better, I'm not sure there's a bug here since it's
>> being immediately overwritten. Seems like this patch would be better going
>> to *-next as an improvement.
>
> It's worth stating in the commit message that the uninitialized memory is
> touched with le16_to_cpus() in the loop only on BE systems. Little-endian
> ones are not affected - le16_to_cpus() is a no-op there.
>
> Anyway, for the BE case, touching and manipulating uninit memory bytes is
> still in general considered a bug, even if this memory is overwritten a
> few lines after that. I guess if KMSAN supported big-endian architectures,
> it would hit this, and that wouldn't be a false-positive.
>
> I'm not aware of the details on how you treat the bugs in these drivers
> for BE-systems: maybe they aren't prioritized and then would occasionnaly
> go as -next material. But, again, this situation looks like a real bug
> worth fixing on BE-systems.
>

Typically the bar for a fixes and a net change is that it requires some
user visible behavioral impact. That's where the hesitance on our end
comes from: how does this cause a user-visible bug?

If there are truly user-visible impact on BE system for touching and
manipulating the uninitialized memory, then it makes sense to go to net
for me. I guess "KASAN/UBSAN complains you touched uninitialized memory"
would be such a bug.

Alternatively: is there a risk of some side channel method to capture
that uninitialized data and leak kernel memory? I don't recall enough to
know whether that would be an issue here...

I don't personally have an objection to going through net (besides
fixing the commit hash for the Fixes: tag on the patch that was wrong)
since its an obvious fix.