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

"Loktionov, Aleksandr" <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-kernel,org.osuosl.intel-wired-lan
Message-ID <DS4PPF7551E65529B793FF5258AF910943FE5DE2@DS4PPF7551E6552.namprd11.prod.outlook.com>

> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf
> Of [email protected]
> Sent: Monday, August 10, 2026 10:34 AM
> To: Nguyen, Anthony L <[email protected]>; Kitszel,
> Przemyslaw <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected];
> [email protected]; Yalagada Pavan
> Kumar <[email protected]>
> Subject: [Intel-wired-lan] [PATCH v2] e100: prevent shift-out-of-
> bounds in e100_eeprom_load()
> 
> From: Yalagada Pavan Kumar <[email protected]>
> 
> 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: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=e0abb1d45ac291ebebeb
> Signed-off-by: Yalagada Pavan Kumar <[email protected]>
> ---
> 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-
> [email protected]/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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.