RE: [Intel-wired-lan] [PATCH v2] e100: prevent shift-out-of-bounds in e100_eeprom_load()

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@xxxxxxxxxx> On Behalf
> Of pavankumaryalagada@xxxxxxxxx
> Sent: Monday, August 10, 2026 10:34 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@xxxxxxxxx>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@xxxxxxxxx>
> Cc: andrew+netdev@xxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx;
> kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; intel-wired-lan@xxxxxxxxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> skhan@xxxxxxxxxxxxxxxxxxx;
> syzbot+e0abb1d45ac291ebebeb@xxxxxxxxxxxxxxxxxxxxxxxxx; Yalagada Pavan
> Kumar <pavankumaryalagada@xxxxxxxxx>
> Subject: [Intel-wired-lan] [PATCH v2] e100: prevent shift-out-of-
> bounds in e100_eeprom_load()
> 
> From: Yalagada Pavan Kumar <pavankumaryalagada@xxxxxxxxx>
> 
> When reading the EEPROM address length, e100_eeprom_read() can return
> an invalid length (0 or >= 16). Passing an invalid addr_len to bit-
> shift operations causes a shift out-of-bounds, triggering a kernel
> panic or UBSAN warning.
> 
> Validate addr_len after reading it from EEPROM in both
> e100_eeprom_load() and e100_eeprom_save(), and return -EIO if the
> value is out of bounds.
> 
> Reported-by: syzbot+e0abb1d45ac291ebebeb@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=e0abb1d45ac291ebebeb
> Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@xxxxxxxxx>
> ---
> Tested in QEMU using syzbot c reproducer.
> 
> v2:
> - Return -EIO instead of -EINVAL for an invalid EEPROM address length.
> - Validate addr_len in e100_eeprom_save() as well.
> - Drop the unnecessary 1U change in the shift.
> - Update the commit message.
> 
> v1:
> https://lore.kernel.org/all/20260807145626.52692-1-
> pavankumaryalagada@xxxxxxxxx/T/
> ---
>  drivers/net/ethernet/intel/e100.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e100.c
> b/drivers/net/ethernet/intel/e100.c
> index 1de5cd41ea0c..464dc2d6cdb5 100644
> --- a/drivers/net/ethernet/intel/e100.c
> +++ b/drivers/net/ethernet/intel/e100.c
> @@ -775,10 +775,10 @@ static int e100_eeprom_load(struct nic *nic)
>  		netif_err(nic, probe, nic->netdev,
>  			"Invalid EEPROM address length %u\n",
>  			addr_len);
> -		return -EINVAL;
> +		return -EIO;
>  	}
> 
> -	nic->eeprom_wc = 1U << addr_len;
> +	nic->eeprom_wc = 1 << addr_len;
Why do you drop U suffix? For me it looks like you trade one warning for another.
I'm for explicit (u16)BIT(addr_len), what do you think?


> 
>  	for (addr = 0; addr < nic->eeprom_wc; addr++) {
>  		nic->eeprom[addr] = e100_eeprom_read(nic, &addr_len,
> addr); @@ -804,6 +804,14 @@ static int e100_eeprom_save(struct nic
> *nic, u16 start, u16 count)
> 
>  	/* Try reading with an 8-bit addr len to discover actual addr
> len */
>  	e100_eeprom_read(nic, &addr_len, 0);
> +
> +	if (!addr_len || addr_len >= 16) {
> +		netif_err(nic, probe, nic->netdev,
> +			"Invalid EEPROM address length %u\n",
> +			addr_len);
> +		return -EIO;
> +	}
> +
>  	nic->eeprom_wc = 1 << addr_len;
I'm for explicit (u16)BIT(addr_len) here too, what do you think?

> 
>  	if (start + count >= nic->eeprom_wc)
> --
> 2.43.0