On Wed, Sep 10, 2025 at 05:31:38PM +0000, Eliav Farber wrote:
Fix a compilation failure when warnings are treated as errors:
drivers/net/ethernet/intel/e1000e/ethtool.c: In function ‘e1000_set_eeprom’: ./include/linux/overflow.h:71:15: error: comparison of distinct pointer types lacks a cast [-Werror] 71 | (void) (&__a == __d); \ | ^~ drivers/net/ethernet/intel/e1000e/ethtool.c:582:6: note: in expansion of macro ‘check_add_overflow’ 582 | if (check_add_overflow(eeprom->offset, eeprom->len, &total_len) || | ^~~~~~~~~~~~~~~~~~
To fix this, change total_len and max_len from size_t to u32 in e1000_set_eeprom(). The check_add_overflow() helper requires that the first two operands and the pointer to the result (third operand) all have the same type. On 64-bit builds, using size_t caused a mismatch with the u32 fields eeprom->offset and eeprom->len, leading to type check failures.
Fixes: ce8829d3d44b ("e1000e: fix heap overflow in e1000_set_eeprom") Signed-off-by: Eliav Farber farbere@amazon.com
drivers/net/ethernet/intel/e1000e/ethtool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c index 4aca854783e2..584378291f3f 100644 --- a/drivers/net/ethernet/intel/e1000e/ethtool.c +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c @@ -559,7 +559,7 @@ 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;
u32 total_len, max_len; u16 *eeprom_buff; int ret_val = 0; int first_word;-- 2.47.3
Why is this not needed in Linus's tree?
Kernel 5.10.243 enforces the same type, but this enforcement is absent from 5.15.192 and later: /* * For simplicity and code hygiene, the fallback code below insists on * a, b and *d having the same type (similar to the min() and max() * macros), whereas gcc's type-generic overflow checkers accept * different types. Hence we don't just make check_add_overflow an * alias for __builtin_add_overflow, but add type checks similar to * below. */ #define check_add_overflow(a, b, d) __must_check_overflow(({ \
Also, why is it not cc: stable@vger.kernel.org?
Added to cc.
--- Regards, Eliav
-----Original Message----- From: Intel-wired-lan intel-wired-lan-bounces@osuosl.org On Behalf Of Farber, Eliav Sent: Thursday, September 11, 2025 8:14 AM To: Greg KH gregkh@linuxfoundation.org Cc: jesse.brandeburg@intel.com; Nguyen, Anthony L anthony.l.nguyen@intel.com; davem@davemloft.net; kuba@kernel.org; Lifshits, Vitaly vitaly.lifshits@intel.com; post@mikaelkw.online; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux- kernel@vger.kernel.org; Chocron, Jonathan jonnyc@amazon.com; stable@vger.kernel.org; Farber, Eliav farbere@amazon.com Subject: Re: [Intel-wired-lan] [PATCH 5.10.y] e1000e: fix EEPROM length types for overflow checks
...
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c index 4aca854783e2..584378291f3f 100644 --- a/drivers/net/ethernet/intel/e1000e/ethtool.c +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c @@ -559,7 +559,7 @@ 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;
u32 total_len, max_len;
I'd like to recommend adding a comment to prevent future regressions, like: /* Use u32 to match types in check_add_overflow() with eeprom->offset and eeprom->len */ Anyway, it's good commit. Reviewed-by: Aleksandr Loktionov aleksandr.loktionov@intel.com
...
Also, why is it not cc: stable@vger.kernel.org?
Added to cc.
Regards, Eliav
On Thu, Sep 11, 2025 at 06:13:33AM +0000, Farber, Eliav wrote:
On Wed, Sep 10, 2025 at 05:31:38PM +0000, Eliav Farber wrote:
Fix a compilation failure when warnings are treated as errors:
drivers/net/ethernet/intel/e1000e/ethtool.c: In function ‘e1000_set_eeprom’: ./include/linux/overflow.h:71:15: error: comparison of distinct pointer types lacks a cast [-Werror] 71 | (void) (&__a == __d); \ | ^~ drivers/net/ethernet/intel/e1000e/ethtool.c:582:6: note: in expansion of macro ‘check_add_overflow’ 582 | if (check_add_overflow(eeprom->offset, eeprom->len, &total_len) || | ^~~~~~~~~~~~~~~~~~
To fix this, change total_len and max_len from size_t to u32 in e1000_set_eeprom(). The check_add_overflow() helper requires that the first two operands and the pointer to the result (third operand) all have the same type. On 64-bit builds, using size_t caused a mismatch with the u32 fields eeprom->offset and eeprom->len, leading to type check failures.
Fixes: ce8829d3d44b ("e1000e: fix heap overflow in e1000_set_eeprom") Signed-off-by: Eliav Farber farbere@amazon.com
drivers/net/ethernet/intel/e1000e/ethtool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c index 4aca854783e2..584378291f3f 100644 --- a/drivers/net/ethernet/intel/e1000e/ethtool.c +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c @@ -559,7 +559,7 @@ 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;
u32 total_len, max_len; u16 *eeprom_buff; int ret_val = 0; int first_word;-- 2.47.3
Why is this not needed in Linus's tree?
Kernel 5.10.243 enforces the same type, but this enforcement is absent from 5.15.192 and later: /*
- For simplicity and code hygiene, the fallback code below insists on
- a, b and *d having the same type (similar to the min() and max()
- macros), whereas gcc's type-generic overflow checkers accept
- different types. Hence we don't just make check_add_overflow an
- alias for __builtin_add_overflow, but add type checks similar to
- below.
*/ #define check_add_overflow(a, b, d) __must_check_overflow(({ \
Yeah, the min() build warning mess is slowly propagating back to older kernels over time as we take these types of fixes backwards. I count 3 such new warnings in the new 5.10 release, not just this single one.
Overall, how about fixing this up so it doesn't happen anymore by backporting the min() logic instead? That should solve this build warning, and keep it from happening again in the future? I did that for newer kernel branches, but never got around to it for these.
thanks,
greg k-h
On Thu, Sep 11, 2025 at 06:13:33AM +0000, Farber, Eliav wrote:
On Wed, Sep 10, 2025 at 05:31:38PM +0000, Eliav Farber wrote:
Fix a compilation failure when warnings are treated as errors:
drivers/net/ethernet/intel/e1000e/ethtool.c: In function ‘e1000_set_eeprom’: ./include/linux/overflow.h:71:15: error: comparison of distinct pointer types lacks a cast [-Werror] 71 | (void) (&__a == __d); \ | ^~ drivers/net/ethernet/intel/e1000e/ethtool.c:582:6: note: in expansion of macro ‘check_add_overflow’ 582 | if (check_add_overflow(eeprom->offset, eeprom->len, &total_len) || | ^~~~~~~~~~~~~~~~~~
To fix this, change total_len and max_len from size_t to u32 in e1000_set_eeprom(). The check_add_overflow() helper requires that the first two operands and the pointer to the result (third operand) all have the same type. On 64-bit builds, using size_t caused a mismatch with the u32 fields eeprom->offset and eeprom->len, leading to type check failures.
Fixes: ce8829d3d44b ("e1000e: fix heap overflow in e1000_set_eeprom") Signed-off-by: Eliav Farber farbere@amazon.com
drivers/net/ethernet/intel/e1000e/ethtool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c index 4aca854783e2..584378291f3f 100644 --- a/drivers/net/ethernet/intel/e1000e/ethtool.c +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c @@ -559,7 +559,7 @@ 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;
u32 total_len, max_len; u16 *eeprom_buff; int ret_val = 0; int first_word;-- 2.47.3
Why is this not needed in Linus's tree?
Kernel 5.10.243 enforces the same type, but this enforcement is absent from 5.15.192 and later: /*
- For simplicity and code hygiene, the fallback code below insists on
- a, b and *d having the same type (similar to the min() and max()
- macros), whereas gcc's type-generic overflow checkers accept
- different types. Hence we don't just make check_add_overflow an
- alias for __builtin_add_overflow, but add type checks similar to
- below.
*/ #define check_add_overflow(a, b, d) __must_check_overflow(({ \
Yeah, the min() build warning mess is slowly propagating back to older kernels over time as we take these types of fixes backwards. I count 3 such new warnings in the new 5.10 release, not just this single one.
Overall, how about fixing this up so it doesn't happen anymore by backporting the min() logic instead? That should solve this build warning, and keep it from happening again in the future? I did that for newer kernel branches, but never got around to it for these.
I did backporting of 4 commits to bring include/linux/overflow.h in line with v5.15.193 in order to pull commit 1d1ac8244c22 ("overflow: Allow mixed type arguments"). I'll also check what can be done for include/linux/minmax.h.
--- Regards, Eliav
On Fri, Sep 12, 2025 at 01:07:35PM +0000, Farber, Eliav wrote:
On Thu, Sep 11, 2025 at 06:13:33AM +0000, Farber, Eliav wrote:
On Wed, Sep 10, 2025 at 05:31:38PM +0000, Eliav Farber wrote:
Fix a compilation failure when warnings are treated as errors:
drivers/net/ethernet/intel/e1000e/ethtool.c: In function ‘e1000_set_eeprom’: ./include/linux/overflow.h:71:15: error: comparison of distinct pointer types lacks a cast [-Werror] 71 | (void) (&__a == __d); \ | ^~ drivers/net/ethernet/intel/e1000e/ethtool.c:582:6: note: in expansion of macro ‘check_add_overflow’ 582 | if (check_add_overflow(eeprom->offset, eeprom->len, &total_len) || | ^~~~~~~~~~~~~~~~~~
To fix this, change total_len and max_len from size_t to u32 in e1000_set_eeprom(). The check_add_overflow() helper requires that the first two operands and the pointer to the result (third operand) all have the same type. On 64-bit builds, using size_t caused a mismatch with the u32 fields eeprom->offset and eeprom->len, leading to type check failures.
Fixes: ce8829d3d44b ("e1000e: fix heap overflow in e1000_set_eeprom") Signed-off-by: Eliav Farber farbere@amazon.com
drivers/net/ethernet/intel/e1000e/ethtool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c index 4aca854783e2..584378291f3f 100644 --- a/drivers/net/ethernet/intel/e1000e/ethtool.c +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c @@ -559,7 +559,7 @@ 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;
u32 total_len, max_len; u16 *eeprom_buff; int ret_val = 0; int first_word;-- 2.47.3
Why is this not needed in Linus's tree?
Kernel 5.10.243 enforces the same type, but this enforcement is absent from 5.15.192 and later: /*
- For simplicity and code hygiene, the fallback code below insists on
- a, b and *d having the same type (similar to the min() and max()
- macros), whereas gcc's type-generic overflow checkers accept
- different types. Hence we don't just make check_add_overflow an
- alias for __builtin_add_overflow, but add type checks similar to
- below.
*/ #define check_add_overflow(a, b, d) __must_check_overflow(({ \
Yeah, the min() build warning mess is slowly propagating back to older kernels over time as we take these types of fixes backwards. I count 3 such new warnings in the new 5.10 release, not just this single one.
Overall, how about fixing this up so it doesn't happen anymore by backporting the min() logic instead? That should solve this build warning, and keep it from happening again in the future? I did that for newer kernel branches, but never got around to it for these.
I did backporting of 4 commits to bring include/linux/overflow.h in line with v5.15.193 in order to pull commit 1d1ac8244c22 ("overflow: Allow mixed type arguments"). I'll also check what can be done for include/linux/minmax.h.
Very cool, thank you!
linux-stable-mirror@lists.linaro.org
-
Farber, Eliav -
Greg KH -
Loktionov, Aleksandr