Dear Vitaly, dear Mikael,
Thank you for the patch.
Am 07.08.25 um 06:05 schrieb Vitaly Lifshits:
Fix a possible heap overflow in e1000_set_eeprom function by adding
input validation for the requested length of the change in the EEPROM.
Do you have a reproducer for this issue?
Fixes: bc7f75fa9788 ("[E1000E]: New pci-express e1000 driver (currently for ICH9
devices only)")
Co-developed-by: Mikael Wessel <[email protected]>
Signed-off-by: Mikael Wessel <[email protected]>
Signed-off-by: Vitaly Lifshits <[email protected]>
---
v2: Use check_add_overflow for boundary checking
---
drivers/net/ethernet/intel/e1000e/ethtool.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c
b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 9364bc2b4eb1..2bfbc7fd9559 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -550,6 +550,7 @@ static int e1000_set_eeprom(struct net_device *netdev,
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
u16 *eeprom_buff;
+ u32 total_len;
Use `unsigned int`? Although the type of `max_len` is signed for
whatever reason.
void *ptr;
int max_len;
int first_word;
@@ -569,6 +570,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;
+
A few places in the Linux kernel also add `unlikely` in front of it.
Would that make sense here?
Also, how is the error case logged?
first_word = eeprom->offset >> 1;
last_word = (eeprom->offset + eeprom->len - 1) >> 1;
eeprom_buff = kmalloc(max_len, GFP_KERNEL);
Kind regards,
Paul
