Re: [PATCH 5.10.y] e1000e: fix EEPROM length types for overflow checks

[Greg KH <gregkh@linuxfoundation.org>]

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