[Intel-wired-lan] [PATCH net 0/3] e1000/e1000e: fix uninitialized memory access in EEPROM write

[Intel-wired-lan] [PATCH net 0/3] e1000/e1000e: fix uninitialized memory access in EEPROM write

[Agalakov Daniil]

This series addresses a common issues related to uninitialized memory
access during EEPROM write operations in the e1000 and e1000e drivers.

In the e1000 driver, the return value of e1000_read_eeprom() was not
checked, and both e1000 and e1000e drivers performed endianness
conversion on the entire buffer, including uninitialized interior words.

The changes are:
 - add missing error checks for e1000_read_eeprom() in e1000;
 - limit endianness conversion to the boundary words being read,
   avoiding a loop over entire (and partially uninitialized) buffer in
   both e1000 and e1000e.

Patch 1: e1000: check return value of e1000_read_eeprom
Patch 2: e1000: fix endianness conversion of uninitialized words
Patch 3: e1000e: fix endianness conversion of uninitialized words

Co-developed-by: Iskhakov Daniil <dish@amicon.ru>
Signed-off-by: Iskhakov Daniil <dish@amicon.ru>
Signed-off-by: Agalakov Daniil <ade@amicon.ru>
---
 .../net/ethernet/intel/e1000/e1000_ethtool.c  | 18 +++++++++++++-----
 drivers/net/ethernet/intel/e1000e/ethtool.c   | 19 ++++++++++++-------
 2 files changed, 25 insertions(+), 12 deletions(-)

-- 
2.51.0

[Intel-wired-lan] [PATCH net 1/3] e1000: check return value of e1000_read_eeprom

[Agalakov Daniil]

[Why]
e1000_set_eeprom() performs a read-modify-write operation when the write
range is not word-aligned. This requires reading the first and last words
of the range from the EEPROM to preserve the unmodified bytes.

However, the code does not check the return value of e1000_read_eeprom().
If the read fails, the operation continues using uninitialized data from
eeprom_buff. This results in corrupted data being written back to the
EEPROM for the boundary words.

Add the missing error checks and abort the operation if reading fails.

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 | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index ab232b3fbbd0..4dcbeabb3ad2 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -496,14 +496,19 @@ static int e1000_set_eeprom(struct net_device *netdev,
 		 */
 		ret_val = e1000_read_eeprom(hw, first_word, 1,
 					    &eeprom_buff[0]);
+		if (ret_val)
+			goto out;
+
 		ptr++;
 	}
-	if (((eeprom->offset + eeprom->len) & 1) && (ret_val == 0)) {
+	if ((eeprom->offset + eeprom->len) & 1) {
 		/* need read/modify/write of last changed EEPROM word
 		 * only the first byte of the word is being modified
 		 */
 		ret_val = e1000_read_eeprom(hw, last_word, 1,
 					    &eeprom_buff[last_word - first_word]);
+		if (ret_val)
+			goto out;
 	}
 
 	/* Device's eeprom is always little-endian, word addressable */
@@ -522,6 +527,7 @@ static int e1000_set_eeprom(struct net_device *netdev,
 	if ((ret_val == 0) && (first_word <= EEPROM_CHECKSUM_REG))
 		e1000_update_eeprom_checksum(hw);
 
+out:
 	kfree(eeprom_buff);
 	return ret_val;
 }
-- 
2.51.0

Re: [Intel-wired-lan] [PATCH net 1/3] e1000: check return value of e1000_read_eeprom

[Loktionov, Aleksandr]

> -----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 1/3] e1000: check return value
> of e1000_read_eeprom
> 
> [Why]
> e1000_set_eeprom() performs a read-modify-write operation when the
> write range is not word-aligned. This requires reading the first and
> last words of the range from the EEPROM to preserve the unmodified
> bytes.
> 
> However, the code does not check the return value of
> e1000_read_eeprom().
> If the read fails, the operation continues using uninitialized data
> from eeprom_buff. This results in corrupted data being written back to
> the EEPROM for the boundary words.
> 
> Add the missing error checks and abort the operation if reading fails.
> 
> 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 | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> index ab232b3fbbd0..4dcbeabb3ad2 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> @@ -496,14 +496,19 @@ static int e1000_set_eeprom(struct net_device
> *netdev,
>  		 */
>  		ret_val = e1000_read_eeprom(hw, first_word, 1,
>  					    &eeprom_buff[0]);
> +		if (ret_val)
> +			goto out;
> +
>  		ptr++;
>  	}
> -	if (((eeprom->offset + eeprom->len) & 1) && (ret_val == 0)) {
> +	if ((eeprom->offset + eeprom->len) & 1) {
>  		/* need read/modify/write of last changed EEPROM word
>  		 * only the first byte of the word is being modified
>  		 */
>  		ret_val = e1000_read_eeprom(hw, last_word, 1,
>  					    &eeprom_buff[last_word -
> first_word]);
> +		if (ret_val)
> +			goto out;
>  	}
> 
>  	/* Device's eeprom is always little-endian, word addressable */
> @@ -522,6 +527,7 @@ static int e1000_set_eeprom(struct net_device
> *netdev,
>  	if ((ret_val == 0) && (first_word <= EEPROM_CHECKSUM_REG))
>  		e1000_update_eeprom_checksum(hw);
> 
> +out:
>  	kfree(eeprom_buff);
>  	return ret_val;
>  }
> --
> 2.51.0

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

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

[Agalakov Daniil]

[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

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

[Loktionov, Aleksandr]

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

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

[Tony Nguyen]

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);
>   

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

[Fedor Pchelkin]

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.

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

[Jacob Keller]

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.

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

[Fedor Pchelkin]

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

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

[Jacob Keller]

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

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

[Agalakov Daniil]

[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/e1000e/ethtool.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index dbed30943ef4..a8b35ae41141 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -583,20 +583,25 @@ static int e1000_set_eeprom(struct net_device *netdev,
 		/* need read/modify/write of first changed EEPROM word */
 		/* only the second byte of the word is being modified */
 		ret_val = e1000_read_nvm(hw, first_word, 1, &eeprom_buff[0]);
+		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) && (!ret_val))
+	if ((eeprom->offset + eeprom->len) & 1) {
 		/* need read/modify/write of last changed EEPROM word */
 		/* only the first byte of the word is being modified */
 		ret_val = e1000_read_nvm(hw, last_word, 1,
 					 &eeprom_buff[last_word - first_word]);
+		if (ret_val)
+			goto out;
 
-	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

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

[Loktionov, Aleksandr]

> -----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 3/3] e1000e: 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/e1000e/ethtool.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c
> b/drivers/net/ethernet/intel/e1000e/ethtool.c
> index dbed30943ef4..a8b35ae41141 100644
> --- a/drivers/net/ethernet/intel/e1000e/ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
> @@ -583,20 +583,25 @@ static int e1000_set_eeprom(struct net_device
> *netdev,
>  		/* need read/modify/write of first changed EEPROM word
> */
>  		/* only the second byte of the word is being modified */
>  		ret_val = e1000_read_nvm(hw, first_word, 1,
> &eeprom_buff[0]);
> +		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) && (!ret_val))
> +	if ((eeprom->offset + eeprom->len) & 1) {
>  		/* need read/modify/write of last changed EEPROM word */
>  		/* only the first byte of the word is being modified */
>  		ret_val = e1000_read_nvm(hw, last_word, 1,
>  					 &eeprom_buff[last_word -
> first_word]);
> +		if (ret_val)
> +			goto out;
> 
> -	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>

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

[Tony Nguyen]

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")

AI Review reports:

The commit message cites the initial git repository commit 1da177e4c3f4
("Linux-2.6.12-rc2") from 2005 as the source of the bug. However, the
e1000e driver wasn't introduced until 2007 in commit bc7f75fa9788
("[E1000E]: New pci-express e1000 driver (currently for ICH9 devices
only)"). While the e1000 driver did have this bug pattern in the initial
commit, this patch fixes the e1000e driver, which is a separate driver.

Should the Fixes: tag reference bc7f75fa9788 instead, since that's when
the buggy pattern was first introduced in e1000e?

Also, the same comment from the e1000 patch applies here. I think this 
patch should be split like the e1000 ones with the return value going to 
*-net and the endian to *-next.

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/e1000e/ethtool.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
> index dbed30943ef4..a8b35ae41141 100644
> --- a/drivers/net/ethernet/intel/e1000e/ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
> @@ -583,20 +583,25 @@ static int e1000_set_eeprom(struct net_device *netdev,
>   		/* need read/modify/write of first changed EEPROM word */
>   		/* only the second byte of the word is being modified */
>   		ret_val = e1000_read_nvm(hw, first_word, 1, &eeprom_buff[0]);
> +		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) && (!ret_val))
> +	if ((eeprom->offset + eeprom->len) & 1) {
>   		/* need read/modify/write of last changed EEPROM word */
>   		/* only the first byte of the word is being modified */
>   		ret_val = e1000_read_nvm(hw, last_word, 1,
>   					 &eeprom_buff[last_word - first_word]);
> +		if (ret_val)
> +			goto out;
>   
> -	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);
>   

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

[Агалаков Даниил]

[-- Attachment #1: Type: text/plain, Size: 4748 bytes --]

Hi Tony,

Thanks for the review!

Regarding e1000e: the return value of e1000_read_eeprom() is already properly checked.
Therefore, for e1000e, I am only submitting the endianness conversion cleanup as part
of a separate 'net-next' series. The functional fix for e1000 (missing return value check)
will be sent as a standalone patch for the 'net' tree.

I'm also removing the Fixes: tags from the endianness cleanup patches since, as you noted,
the uninitialized data is eventually overwritten by memcpy() and doesn't cause a functional
failure.

Best regards,
Daniil
________________________________
From: Tony Nguyen <anthony.l.nguyen@intel.com>
Sent: Wednesday, March 25, 2026 2:27:12 AM
To: Агалаков Даниил
Cc: Przemek Kitszel; Andrew Lunn; David S. Miller; Eric Dumazet; Jakub Kicinski; Paolo Abeni; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; lvc-project@linuxtesting.org; Исхаков Даниил; Разов Роман
Subject: Re: [PATCH net 3/3] e1000e: fix endianness conversion of uninitialized words



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")

AI Review reports:

The commit message cites the initial git repository commit 1da177e4c3f4
("Linux-2.6.12-rc2") from 2005 as the source of the bug. However, the
e1000e driver wasn't introduced until 2007 in commit bc7f75fa9788
("[E1000E]: New pci-express e1000 driver (currently for ICH9 devices
only)"). While the e1000 driver did have this bug pattern in the initial
commit, this patch fixes the e1000e driver, which is a separate driver.

Should the Fixes: tag reference bc7f75fa9788 instead, since that's when
the buggy pattern was first introduced in e1000e?

Also, the same comment from the e1000 patch applies here. I think this
patch should be split like the e1000 ones with the return value going to
*-net and the endian to *-next.

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/e1000e/ethtool.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
> index dbed30943ef4..a8b35ae41141 100644
> --- a/drivers/net/ethernet/intel/e1000e/ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
> @@ -583,20 +583,25 @@ static int e1000_set_eeprom(struct net_device *netdev,
>                /* need read/modify/write of first changed EEPROM word */
>                /* only the second byte of the word is being modified */
>                ret_val = e1000_read_nvm(hw, first_word, 1, &eeprom_buff[0]);
> +             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) && (!ret_val))
> +     if ((eeprom->offset + eeprom->len) & 1) {
>                /* need read/modify/write of last changed EEPROM word */
>                /* only the first byte of the word is being modified */
>                ret_val = e1000_read_nvm(hw, last_word, 1,
>                                         &eeprom_buff[last_word - first_word]);
> +             if (ret_val)
> +                     goto out;
>
> -     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);
>


[-- Attachment #2: Type: text/html, Size: 8308 bytes --]

[Intel-wired-lan] [PATCH net v2] e1000: check return value of e1000_read_eeprom

[Agalakov Daniil]

[Why]
e1000_set_eeprom() performs a read-modify-write operation when the write
range is not word-aligned. This requires reading the first and last words
of the range from the EEPROM to preserve the unmodified bytes.

However, the code does not check the return value of e1000_read_eeprom().
If the read fails, the operation continues using uninitialized data from
eeprom_buff. This results in corrupted data being written back to the
EEPROM for the boundary words.

Add the missing error checks and abort the operation if reading fails.

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>
---
v2:
 - Split from original series.
 - Updated the error checking logic to be consistent with the
   implementation in the e1000e driver.

 drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index ab232b3fbbd0..a9c56505adcb 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -506,6 +506,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]);
@@ -522,6 +526,7 @@ static int e1000_set_eeprom(struct net_device *netdev,
 	if ((ret_val == 0) && (first_word <= EEPROM_CHECKSUM_REG))
 		e1000_update_eeprom_checksum(hw);
 
+out:
 	kfree(eeprom_buff);
 	return ret_val;
 }
-- 
2.51.0

Re: [Intel-wired-lan] [PATCH net v2] e1000: check return value of e1000_read_eeprom

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Agalakov Daniil
> Sent: Wednesday, March 25, 2026 4:02 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 v2] e1000: check return value of
> e1000_read_eeprom
> 
> [Why]
> e1000_set_eeprom() performs a read-modify-write operation when the
> write range is not word-aligned. This requires reading the first and
> last words of the range from the EEPROM to preserve the unmodified
> bytes.
> 
> However, the code does not check the return value of
> e1000_read_eeprom().
> If the read fails, the operation continues using uninitialized data
> from eeprom_buff. This results in corrupted data being written back to
> the EEPROM for the boundary words.
> 
> Add the missing error checks and abort the operation if reading fails.
> 
> 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>
> ---
> v2:
>  - Split from original series.
>  - Updated the error checking logic to be consistent with the
>    implementation in the e1000e driver.
> 
>  drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> index ab232b3fbbd0..a9c56505adcb 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> @@ -506,6 +506,10 @@ static int e1000_set_eeprom(struct net_device
> *netdev,
>  					    &eeprom_buff[last_word -
> first_word]);
>  	}
> 
> +	if (ret_val)
> +		goto out;
> +
> + 
Extra blank line.
Otherwise looks good for me

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

>  	/* 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]);
> @@ -522,6 +526,7 @@ static int e1000_set_eeprom(struct net_device
> *netdev,
>  	if ((ret_val == 0) && (first_word <= EEPROM_CHECKSUM_REG))
>  		e1000_update_eeprom_checksum(hw);
> 
> +out:
>  	kfree(eeprom_buff);
>  	return ret_val;
>  }
> --
> 2.51.0

[Intel-wired-lan] [PATCH net-next v2 0/2] e1000/e1000e: limit endianness conversion to boundary words

[Agalakov Daniil]

This series refactors the EEPROM write logic in e1000 and e1000e drivers
to avoid processing uninitialized memory. Instead of looping over the
entire buffer, we now only perform endianness conversion on the boundary
words that were actually read from the hardware.

Patch 1: e1000: limit endianness conversion to boundary words
Patch 2: e1000e: limit endianness conversion to boundary words
---
v2:
 - Moved these improvements to the 'net-next' tree.
 - Improved commit description for clarity.

 drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 11 +++++++----
 drivers/net/ethernet/intel/e1000e/ethtool.c      | 10 +++++++++-
 2 files changed, 16 insertions(+), 5 deletions(-)

-- 
2.51.0

[Intel-wired-lan] [PATCH net-next v2 1/2] e1000: limit endianness conversion to boundary words

[Agalakov Daniil]

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

Co-developed-by: Iskhakov Daniil <dish@amicon.ru>
Signed-off-by: Iskhakov Daniil <dish@amicon.ru>
Signed-off-by: Agalakov Daniil <ade@amicon.ru>
---
v2:
 - Split from the original bugfix series and targeted at 'net-text'.
 - Removed the Fixes: tag; limiting the conversion scope is an
   improvement to avoid unnecessary processing of uninitialized memory.
 - Improved commit description for clarity.

 drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index ab232b3fbbd0..38b1f91823ef 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -496,6 +496,10 @@ static int e1000_set_eeprom(struct net_device *netdev,
 		 */
 		ret_val = e1000_read_eeprom(hw, first_word, 1,
 					    &eeprom_buff[0]);
+
+		/* Device's eeprom is always little-endian, word addressable */
+		le16_to_cpus(&eeprom_buff[0]);
+
 		ptr++;
 	}
 	if (((eeprom->offset + eeprom->len) & 1) && (ret_val == 0)) {
@@ -504,11 +508,10 @@ static int e1000_set_eeprom(struct net_device *netdev,
 		 */
 		ret_val = e1000_read_eeprom(hw, last_word, 1,
 					    &eeprom_buff[last_word - first_word]);
-	}
 
-	/* 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

Re: [Intel-wired-lan] [PATCH net-next v2 1/2] e1000: limit endianness conversion to boundary words

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Agalakov Daniil
> Sent: Wednesday, March 25, 2026 4:16 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-next v2 1/2] e1000: limit
> endianness conversion to boundary 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.
> 
> Co-developed-by: Iskhakov Daniil <dish@amicon.ru>
> Signed-off-by: Iskhakov Daniil <dish@amicon.ru>
> Signed-off-by: Agalakov Daniil <ade@amicon.ru>
> ---
> v2:
>  - Split from the original bugfix series and targeted at 'net-text'.
>  - Removed the Fixes: tag; limiting the conversion scope is an
>    improvement to avoid unnecessary processing of uninitialized
> memory.
>  - Improved commit description for clarity.
> 
>  drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> index ab232b3fbbd0..38b1f91823ef 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
> @@ -496,6 +496,10 @@ static int e1000_set_eeprom(struct net_device
> *netdev,
>  		 */
>  		ret_val = e1000_read_eeprom(hw, first_word, 1,
>  					    &eeprom_buff[0]);
> +
> +		/* Device's eeprom is always little-endian, word
> addressable */
> +		le16_to_cpus(&eeprom_buff[0]);
> +
>  		ptr++;
>  	}
>  	if (((eeprom->offset + eeprom->len) & 1) && (ret_val == 0)) {
> @@ -504,11 +508,10 @@ static int e1000_set_eeprom(struct net_device
> *netdev,
>  		 */
>  		ret_val = e1000_read_eeprom(hw, last_word, 1,
>  					    &eeprom_buff[last_word -
> first_word]);
> -	}
> 
> -	/* 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

e1000e: limit endianness conversion to boundary words

[Intel-wired-lan] [PATCH net-next v2 2/2] e1000e: limit endianness conversion to boundary words

[Agalakov Daniil]

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

Co-developed-by: Iskhakov Daniil <dish@amicon.ru>
Signed-off-by: Iskhakov Daniil <dish@amicon.ru>
Signed-off-by: Agalakov Daniil <ade@amicon.ru>
---
v2:
 - Split from the original bugfix series and targeted at 'net-text'.
 - Removed the Fixes: tag; limiting the conversion scope is an
   improvement to avoid unnecessary processing of uninitialized memory.
 - Improved commit description for clarity.
 - Note on e1000e: this driver already contains the necessary return
   value checks for EEPROM reads, so only the endianness conversion
   cleanup is included for e1000e.

 drivers/net/ethernet/intel/e1000e/ethtool.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index dbed30943ef4..785d89477c43 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -583,13 +583,21 @@ static int e1000_set_eeprom(struct net_device *netdev,
 		/* need read/modify/write of first changed EEPROM word */
 		/* only the second byte of the word is being modified */
 		ret_val = e1000_read_nvm(hw, first_word, 1, &eeprom_buff[0]);
+
+		/* Device's eeprom is always little-endian, word addressable */
+		le16_to_cpus(&eeprom_buff[0]);
+
 		ptr++;
 	}
-	if (((eeprom->offset + eeprom->len) & 1) && (!ret_val))
+	if (((eeprom->offset + eeprom->len) & 1) && (!ret_val)) {
 		/* need read/modify/write of last changed EEPROM word */
 		/* only the first byte of the word is being modified */
 		ret_val = e1000_read_nvm(hw, last_word, 1,
 					 &eeprom_buff[last_word - first_word]);
+	
+		/* Device's eeprom is always little-endian, word addressable */
+		le16_to_cpus(&eeprom_buff[last_word - first_word]);
+	}
 
 	if (ret_val)
 		goto out;
-- 
2.51.0

Re: [Intel-wired-lan] [PATCH net-next v2 2/2] e1000e: limit endianness conversion to boundary words

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Agalakov Daniil
> Sent: Wednesday, March 25, 2026 4:16 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-next v2 2/2] e1000e: limit
> endianness conversion to boundary 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.
> 
> Co-developed-by: Iskhakov Daniil <dish@amicon.ru>
> Signed-off-by: Iskhakov Daniil <dish@amicon.ru>
> Signed-off-by: Agalakov Daniil <ade@amicon.ru>
> ---
> v2:
>  - Split from the original bugfix series and targeted at 'net-text'.
>  - Removed the Fixes: tag; limiting the conversion scope is an
>    improvement to avoid unnecessary processing of uninitialized
> memory.
>  - Improved commit description for clarity.
>  - Note on e1000e: this driver already contains the necessary return
>    value checks for EEPROM reads, so only the endianness conversion
>    cleanup is included for e1000e.
> 
>  drivers/net/ethernet/intel/e1000e/ethtool.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c
> b/drivers/net/ethernet/intel/e1000e/ethtool.c
> index dbed30943ef4..785d89477c43 100644
> --- a/drivers/net/ethernet/intel/e1000e/ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
> @@ -583,13 +583,21 @@ static int e1000_set_eeprom(struct net_device
> *netdev,
>  		/* need read/modify/write of first changed EEPROM word
> */
>  		/* only the second byte of the word is being modified */
>  		ret_val = e1000_read_nvm(hw, first_word, 1,
> &eeprom_buff[0]);
> +
> +		/* Device's eeprom is always little-endian, word
> addressable */
> +		le16_to_cpus(&eeprom_buff[0]);
> +
>  		ptr++;
>  	}
> -	if (((eeprom->offset + eeprom->len) & 1) && (!ret_val))
> +	if (((eeprom->offset + eeprom->len) & 1) && (!ret_val)) {
>  		/* need read/modify/write of last changed EEPROM word */
>  		/* only the first byte of the word is being modified */
>  		ret_val = e1000_read_nvm(hw, last_word, 1,
>  					 &eeprom_buff[last_word -
> first_word]);
> +
> +		/* Device's eeprom is always little-endian, word
> addressable */
> +		le16_to_cpus(&eeprom_buff[last_word - first_word]);
> +	}
> 
>  	if (ret_val)
>  		goto out;
> --
> 2.51.0

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

Re: [Intel-wired-lan] [PATCH net-next v2 2/2] e1000e: limit endianness conversion to boundary words

[Tony Nguyen]

On 3/25/2026 8:16 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.
> 
> Co-developed-by: Iskhakov Daniil <dish@amicon.ru>
> Signed-off-by: Iskhakov Daniil <dish@amicon.ru>
> Signed-off-by: Agalakov Daniil <ade@amicon.ru>
> ---
> v2:
>   - Split from the original bugfix series and targeted at 'net-text'.
>   - Removed the Fixes: tag; limiting the conversion scope is an
>     improvement to avoid unnecessary processing of uninitialized memory.
>   - Improved commit description for clarity.
>   - Note on e1000e: this driver already contains the necessary return
>     value checks for EEPROM reads, so only the endianness conversion
>     cleanup is included for e1000e.
> 
>   drivers/net/ethernet/intel/e1000e/ethtool.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
> index dbed30943ef4..785d89477c43 100644
> --- a/drivers/net/ethernet/intel/e1000e/ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
> @@ -583,13 +583,21 @@ static int e1000_set_eeprom(struct net_device *netdev,
>   		/* need read/modify/write of first changed EEPROM word */
>   		/* only the second byte of the word is being modified */
>   		ret_val = e1000_read_nvm(hw, first_word, 1, &eeprom_buff[0]);
> +
> +		/* Device's eeprom is always little-endian, word addressable */
> +		le16_to_cpus(&eeprom_buff[0]);

I think the v1 was better. We should check the ret_val first so we don't 
do this conversion on error.

> +
>   		ptr++;
>   	}
> -	if (((eeprom->offset + eeprom->len) & 1) && (!ret_val))
> +	if (((eeprom->offset + eeprom->len) & 1) && (!ret_val)) {
>   		/* need read/modify/write of last changed EEPROM word */
>   		/* only the first byte of the word is being modified */
>   		ret_val = e1000_read_nvm(hw, last_word, 1,
>   					 &eeprom_buff[last_word - first_word]);

Same here.

Also, same for the e1000 version.

> +	
> +		/* Device's eeprom is always little-endian, word addressable */
> +		le16_to_cpus(&eeprom_buff[last_word - first_word]);
> +	}
>   
>   	if (ret_val)
>   		goto out;

Also, AI review reports:

The patch adds le16_to_cpus() conversion immediately after reading the
boundary words, but doesn't this cause double endianness conversion?

Looking at the original code before this patch, there should be loops
after the "goto out" section that convert the entire buffer:

     /* 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]);

And then another loop before writing back:

     for (i = 0; i < last_word - first_word + 1; i++)
         cpu_to_le16s(&eeprom_buff[i]);

If those loops are still present, the boundary words at indices 0 and
(last_word - first_word) would be converted twice - once here at lines
588 and 597, then again in the loop. On big-endian systems, converting
LE->CPU->LE would byte-swap the boundary words compared to the user's
intended values. Does the patch also remove those loops, or is this
causing data corruption on big-endian architectures?

Thanks,
Tony

[Intel-wired-lan] [PATCH net-next v3 0/2] e1000/e1000e: limit endianness conversion to boundary words

[Agalakov Daniil]

This series refactors the EEPROM write logic in e1000 and e1000e drivers
to avoid processing uninitialized memory. Instead of looping over the
entire buffer, we now only perform endianness conversion on the boundary
words that were actually read from the hardware.

Patch 1: e1000: limit endianness conversion to boundary words
Patch 2: e1000e: limit endianness conversion to boundary words
---
v3:
 - Reverted to v1's "check-then-convert" logic in patch for e1000e: the
   return value of e1000_read_nvm() is now checked before performing
   le16_to_cpus().
 - Removed the redundant full-buffer loops in patch for e1000e that
   caused double endianness conversion in v2.

v2:
 - Moved these improvements to the 'net-next' tree.
 - Improved commit description for clarity.

 .../net/ethernet/intel/e1000/e1000_ethtool.c  | 11 +++++++----
 drivers/net/ethernet/intel/e1000e/ethtool.c   | 19 ++++++++++++-------
 2 files changed, 19 insertions(+), 11 deletions(-)

-- 
2.51.0

[Intel-wired-lan] [PATCH net-next v3 1/2] e1000: limit endianness conversion to boundary words

[Agalakov Daniil]

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

Co-developed-by: Iskhakov Daniil <dish@amicon.ru>
Signed-off-by: Iskhakov Daniil <dish@amicon.ru>
Signed-off-by: Agalakov Daniil <ade@amicon.ru>
---
v2:
 - Split from the original bugfix series and targeted at 'net-next'.
 - Removed the Fixes: tag; limiting the conversion scope is an
   improvement to avoid unnecessary processing of uninitialized memory.
 - Improved commit description for clarity.

 drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index ab232b3fbbd0..38b1f91823ef 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -496,6 +496,10 @@ static int e1000_set_eeprom(struct net_device *netdev,
 		 */
 		ret_val = e1000_read_eeprom(hw, first_word, 1,
 					    &eeprom_buff[0]);
+
+		/* Device's eeprom is always little-endian, word addressable */
+		le16_to_cpus(&eeprom_buff[0]);
+
 		ptr++;
 	}
 	if (((eeprom->offset + eeprom->len) & 1) && (ret_val == 0)) {
@@ -504,11 +508,10 @@ static int e1000_set_eeprom(struct net_device *netdev,
 		 */
 		ret_val = e1000_read_eeprom(hw, last_word, 1,
 					    &eeprom_buff[last_word - first_word]);
-	}
 
-	/* 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

[Intel-wired-lan] [PATCH net-next v3 2/2] e1000e: limit endianness conversion to boundary words

[Agalakov Daniil]

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

Co-developed-by: Iskhakov Daniil <dish@amicon.ru>
Signed-off-by: Iskhakov Daniil <dish@amicon.ru>
Signed-off-by: Agalakov Daniil <ade@amicon.ru>
---
v3:
 - Reverted to v1's "check-then-convert" logic: the return value of
   e1000_read_nvm() is now checked before performing le16_to_cpus().
 - Removed the redundant full-buffer loops that caused double endianness
   conversion in v2.

v2:
 - Split from the original bugfix series and targeted at 'net-next'.
 - Removed the Fixes: tag; limiting the conversion scope is an
   improvement to avoid unnecessary processing of uninitialized memory.
 - Improved commit description for clarity.
 - Note on e1000e: this driver already contains the necessary return
   value checks for EEPROM reads, so only the endianness conversion
   cleanup is included for e1000e.

 drivers/net/ethernet/intel/e1000e/ethtool.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index dbed30943ef4..a8b35ae41141 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -583,20 +583,25 @@ static int e1000_set_eeprom(struct net_device *netdev,
 		/* need read/modify/write of first changed EEPROM word */
 		/* only the second byte of the word is being modified */
 		ret_val = e1000_read_nvm(hw, first_word, 1, &eeprom_buff[0]);
+		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) && (!ret_val))
+	if ((eeprom->offset + eeprom->len) & 1) {
 		/* need read/modify/write of last changed EEPROM word */
 		/* only the first byte of the word is being modified */
 		ret_val = e1000_read_nvm(hw, last_word, 1,
 					 &eeprom_buff[last_word - first_word]);
+		if (ret_val)
+			goto out;
 
-	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

Re: [Intel-wired-lan] [PATCH net-next v3 2/2] e1000e: limit endianness conversion to boundary words

[Dahan, AvigailX]

On 01/04/2026 15:08, 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.
> 
> Co-developed-by: Iskhakov Daniil <dish@amicon.ru>
> Signed-off-by: Iskhakov Daniil <dish@amicon.ru>
> Signed-off-by: Agalakov Daniil <ade@amicon.ru>
> ---
> v3:
>   - Reverted to v1's "check-then-convert" logic: the return value of
>     e1000_read_nvm() is now checked before performing le16_to_cpus().
>   - Removed the redundant full-buffer loops that caused double endianness
>     conversion in v2.
> 
> v2:
>   - Split from the original bugfix series and targeted at 'net-next'.
>   - Removed the Fixes: tag; limiting the conversion scope is an
>     improvement to avoid unnecessary processing of uninitialized memory.
>   - Improved commit description for clarity.
>   - Note on e1000e: this driver already contains the necessary return
>     value checks for EEPROM reads, so only the endianness conversion
>     cleanup is included for e1000e.
> 
>   drivers/net/ethernet/intel/e1000e/ethtool.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 

Tested-by: Avigail Dahan <avigailx.dahan@intel.com>

Re: [Intel-wired-lan] [PATCH net-next v3 0/2] e1000/e1000e: limit endianness conversion to boundary words

[Fedor Pchelkin]

On Wed, 01. Apr 15:08, Agalakov Daniil wrote:
> This series refactors the EEPROM write logic in e1000 and e1000e drivers
> to avoid processing uninitialized memory. Instead of looping over the
> entire buffer, we now only perform endianness conversion on the boundary
> words that were actually read from the hardware.
> 
> Patch 1: e1000: limit endianness conversion to boundary words
> Patch 2: e1000e: limit endianness conversion to boundary words
> ---

Daniil, for future submissions, please post new versions of the patches
in a separate email-thread, not In-Reply-To.

https://docs.kernel.org/process/maintainer-netdev.html#resending-after-review
https://docs.kernel.org/process/maintainer-netdev.html#changes-requested

[Intel-wired-lan] [PATCH net v3] e1000: check return value of e1000_read_eeprom

[Agalakov Daniil]

[Why]
e1000_set_eeprom() performs a read-modify-write operation when the write
range is not word-aligned. This requires reading the first and last words
of the range from the EEPROM to preserve the unmodified bytes.

However, the code does not check the return value of e1000_read_eeprom().
If the read fails, the operation continues using uninitialized data from
eeprom_buff. This results in corrupted data being written back to the
EEPROM for the boundary words.

Add the missing error checks and abort the operation if reading fails.

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>
---
v3:
 - Remove extra blank line.

v2:
 - Split from original series.
 - Updated the error checking logic to be consistent with the
   implementation in the e1000e driver.

 drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 4 ++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index ab232b3fbbd0..3c3c6de95af7 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -506,6 +506,9 @@ 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]);
@@ -522,6 +525,7 @@ static int e1000_set_eeprom(struct net_device *netdev,
 	if ((ret_val == 0) && (first_word <= EEPROM_CHECKSUM_REG))
 		e1000_update_eeprom_checksum(hw);
 
+out:
 	kfree(eeprom_buff);
 	return ret_val;
 }
-- 
2.51.0