[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")
Co-developed-by: Iskhakov Daniil <dish@amicon.ru>
Signed-off-by: Iskhakov Daniil <dish@amicon.ru>
Signed-off-by: Agalakov Daniil <ade@amicon.ru>
---
 drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index 4dcbeabb3ad2..c15ad95c63c1 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -499,6 +499,9 @@ static int e1000_set_eeprom(struct net_device *netdev,
 		if (ret_val)
 			goto out;
 
+		/* Device's eeprom is always little-endian, word addressable */
+		le16_to_cpus(&eeprom_buff[0]);
+
 		ptr++;
 	}
 	if ((eeprom->offset + eeprom->len) & 1) {
@@ -509,11 +512,10 @@ static int e1000_set_eeprom(struct net_device *netdev,
 					    &eeprom_buff[last_word - first_word]);
 		if (ret_val)
 			goto out;
-	}
 
-	/* Device's eeprom is always little-endian, word addressable */
-	for (i = 0; i < last_word - first_word + 1; i++)
-		le16_to_cpus(&eeprom_buff[i]);
+		/* Device's eeprom is always little-endian, word addressable */
+		le16_to_cpus(&eeprom_buff[last_word - first_word]);
+	}
 
 	memcpy(ptr, bytes, eeprom->len);
 
-- 
2.51.0

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.

Thanks,
Tony

> Co-developed-by: Iskhakov Daniil <dish@amicon.ru>
> Signed-off-by: Iskhakov Daniil <dish@amicon.ru>
> Signed-off-by: Agalakov Daniil <ade@amicon.ru>
> ---
>   drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> index 4dcbeabb3ad2..c15ad95c63c1 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> @@ -499,6 +499,9 @@ static int e1000_set_eeprom(struct net_device *netdev,
>   		if (ret_val)
>   			goto out;
>   
> +		/* Device's eeprom is always little-endian, word addressable */
> +		le16_to_cpus(&eeprom_buff[0]);
> +
>   		ptr++;
>   	}
>   	if ((eeprom->offset + eeprom->len) & 1) {
> @@ -509,11 +512,10 @@ static int e1000_set_eeprom(struct net_device *netdev,
>   					    &eeprom_buff[last_word - first_word]);
>   		if (ret_val)
>   			goto out;
> -	}
>   
> -	/* Device's eeprom is always little-endian, word addressable */
> -	for (i = 0; i < last_word - first_word + 1; i++)
> -		le16_to_cpus(&eeprom_buff[i]);
> +		/* Device's eeprom is always little-endian, word addressable */
> +		le16_to_cpus(&eeprom_buff[last_word - first_word]);
> +	}
>   
>   	memcpy(ptr, bytes, eeprom->len);
>

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.

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.

On Wed, 25. Mar 16:01, Jacob Keller wrote:
> 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.

My first thought was that reading and writing uninitialized memory is just
undefined behavior in C.  It's hard to say what some compiler might invent
here.

The current situation is not trivial though because there is a chunk of
uninit memory allocated with kmalloc() to which we have a valid pointer,
and we are swapping the order of uninitialized bytes inside this chunk,
i.e. reading/writing uninitialized memory.  Basically it should just go
down to swapping garbage values without any suspicious compiler
optimizations - I don't think the compiler is aware that kmalloc() returns
uninit memory.


KMSAN actually supports one big-endian arch - that's s390 (the only arch
except x86 that KMSAN supports).  I've tried quickly to trigger the splat
out of curiosity, but booting the KMSAN-enabled kernel with s390 QEMU TCG
looks like taking forever so unfortunately no specific results here.

KASAN/UBSAN don't catch error patterns of this type.

> 
> 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.

Thanks for the feedback.  I see Daniil has already sent some newer
versions targeting -next.  The problem in question is rather obscure about
severity of its consequences but spending some time on investigating this
now I'm more inclined to think there should be no opportunity for UB here
and it's okay to process the patch like Tony suggested..

On 3/29/2026 9:02 AM, Fedor Pchelkin wrote:
> On Wed, 25. Mar 16:01, Jacob Keller wrote:
>> On 3/25/2026 8:19 AM, Fedor Pchelkin wrote:
>>> Hi,
>>>
>>> On Tue, 24. Mar 16:26, Tony Nguyen wrote:
> Thanks for the feedback.  I see Daniil has already sent some newer
> versions targeting -next.  The problem in question is rather obscure about
> severity of its consequences but spending some time on investigating this
> now I'm more inclined to think there should be no opportunity for UB here
> and it's okay to process the patch like Tony suggested..

Makes sense. Thanks for the careful review either way!

Regards,
Jake

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Agalakov Daniil
> Sent: Wednesday, March 18, 2026 1:05 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Cc: Agalakov Daniil <ade@amicon.ru>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; lvc-
> project@linuxtesting.org; Daniil Iskhakov <dish@amicon.ru>; Roman
> Razov <rrv@amicon.ru>
> Subject: [Intel-wired-lan] [PATCH net 2/3] e1000: fix endianness
> conversion of uninitialized words
> 
> [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")
> Co-developed-by: Iskhakov Daniil <dish@amicon.ru>
> Signed-off-by: Iskhakov Daniil <dish@amicon.ru>
> Signed-off-by: Agalakov Daniil <ade@amicon.ru>
> ---
>  drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> index 4dcbeabb3ad2..c15ad95c63c1 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> @@ -499,6 +499,9 @@ static int e1000_set_eeprom(struct net_device
> *netdev,
>  		if (ret_val)
>  			goto out;
> 
> +		/* Device's eeprom is always little-endian, word
> addressable */
> +		le16_to_cpus(&eeprom_buff[0]);
> +
>  		ptr++;
>  	}
>  	if ((eeprom->offset + eeprom->len) & 1) { @@ -509,11 +512,10 @@
> static int e1000_set_eeprom(struct net_device *netdev,
>  					    &eeprom_buff[last_word -
> first_word]);
>  		if (ret_val)
>  			goto out;
> -	}
> 
> -	/* Device's eeprom is always little-endian, word addressable */
> -	for (i = 0; i < last_word - first_word + 1; i++)
> -		le16_to_cpus(&eeprom_buff[i]);
> +		/* Device's eeprom is always little-endian, word
> addressable */
> +		le16_to_cpus(&eeprom_buff[last_word - first_word]);
> +	}
> 
>  	memcpy(ptr, bytes, eeprom->len);
> 
> --
> 2.51.0

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>