[PATCH net v2] e1000: check return value of e1000_read_eeprom

Agalakov Daniil posted 1 patch 1 week, 1 day ago
There is a newer version of this series
drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 5 +++++
1 file changed, 5 insertions(+)
[PATCH net v2] e1000: check return value of e1000_read_eeprom
Posted by Agalakov Daniil 1 week, 1 day ago
[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
Posted by Loktionov, Aleksandr 1 week, 1 day ago

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