Re: [Intel-wired-lan] [PATCH iwl-net v3 1/1] e1000e: fix heap overflow in e1000_set_eeprom

[Przemek Kitszel]

On 8/14/25 09:18, Vitaly Lifshits wrote:

Fix a possible heap overflow in e1000_set_eeprom function by adding
input validation for the requested length of the change in the EEPROM.
In addition, change the variable type from int to size_t for better

thats good

code practices and rearrange declarations to RCT.

I would avoid, especially that this is now majority of the changes in
the first chunk. But OTOH, does not matter that much for rarely updated
driver.

Fixes: bc7f75fa9788 ("[E1000E]: New pci-express e1000 driver (currently for ICH9 
devices only)")
Co-developed-by: Mikael Wessel <post@mikaelkw.online>
Signed-off-by: Mikael Wessel <post@mikaelkw.online>
Signed-off-by: Vitaly Lifshits <vitaly.lifsh...@intel.com>
---
v3: Change max_len and total_len to size_t
v2: Use check_add_overflow for boundary checking
---
  drivers/net/ethernet/intel/e1000e/ethtool.c | 10 +++++++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c 
b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 9364bc2b4eb1..d7f25f007e8e 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -549,12 +549,12 @@ static int e1000_set_eeprom(struct net_device *netdev,
  {
        struct e1000_adapter *adapter = netdev_priv(netdev);
        struct e1000_hw *hw = &adapter->hw;
+       size_t total_len, max_len;
        u16 *eeprom_buff;
-       void *ptr;
-       int max_len;
+       int ret_val = 0;
        int first_word;
        int last_word;
-       int ret_val = 0;
+       void *ptr;
        u16 i;

if (eeprom->len == 0)

@@ -569,6 +569,10 @@ static int e1000_set_eeprom(struct net_device *netdev,

max_len = hw->nvm.word_size * 2; + if (check_add_overflow(eeprom->offset, eeprom->len, &total_len) ||

+           total_len > max_len)
+               return -EINVAL;

I would differentiate error codes (EINVAL is already in use for this
function), and in general prefer to use broader range.

What about EFBIG or E2BIG?

+
        first_word = eeprom->offset >> 1;
        last_word = (eeprom->offset + eeprom->len - 1) >> 1;
        eeprom_buff = kmalloc(max_len, GFP_KERNEL);