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

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

[pavankumaryalagada]

From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>

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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e0abb1d45ac291ebebeb
Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
---
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@gmail.com/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;
 
 	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;
 
 	if (start + count >= nic->eeprom_wc)
-- 
2.43.0

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@osuosl.org> On Behalf
> Of pavankumaryalagada@gmail.com
> Sent: Monday, August 10, 2026 10:34 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>
> Cc: andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> skhan@linuxfoundation.org;
> syzbot+e0abb1d45ac291ebebeb@syzkaller.appspotmail.com; Yalagada Pavan
> Kumar <pavankumaryalagada@gmail.com>
> Subject: [Intel-wired-lan] [PATCH v2] e100: prevent shift-out-of-
> bounds in e100_eeprom_load()
> 
> From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> 
> 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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e0abb1d45ac291ebebeb
> Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> ---
> 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@gmail.com/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

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

[Yalagada Pavan Kumar]

On Mon, Aug 10, 2026 at 08:49:42AM +0000, Loktionov, Aleksandr wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> > Of pavankumaryalagada@gmail.com
> > Sent: Monday, August 10, 2026 10:34 AM
> > To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> > Przemyslaw <przemyslaw.kitszel@intel.com>
> > Cc: andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> > kuba@kernel.org; pabeni@redhat.com; intel-wired-lan@lists.osuosl.org;
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> > skhan@linuxfoundation.org;
> > syzbot+e0abb1d45ac291ebebeb@syzkaller.appspotmail.com; Yalagada Pavan
> > Kumar <pavankumaryalagada@gmail.com>
> > Subject: [Intel-wired-lan] [PATCH v2] e100: prevent shift-out-of-
> > bounds in e100_eeprom_load()
> > 
> > From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> > 
> > 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@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=e0abb1d45ac291ebebeb
> > Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> > ---
> > 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@gmail.com/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 dropped the `U` suffix in v2 based on previous review. My understanding from that
review was that the `U` suffix is not what prevents the shift-out-of-bounds issue.

The problem is that `addr_len` can underflow as a `u16` and become a large value
such as 65529.

In that case, both `1 << addr_len` and `1U << addr_len` would have an invalid shift count.
Validating addr_len before calculating `eeprom_wc` is what prevents the invalid shift.

The reason i used `1U << addr_len` in v1 was to make the left operand unsigned.

However, after validating `addr_len` to supported range, the maximum shift is 8, so the U
suffix is not needed to prevent signed overflow.

I also tested the reproducer with all three forms:
	1U << addr_len
	1 << addr_len
	(u16)BIT(addr_len)

They all produced the same result with reproducer because the invalid address length is
rejected before the shift is performed.

Regarding the validation range, I initially used
	if (!addr_len || addr_len >= 16)

because the reported value was 65529 and this was sufficient to reject
it before the shift.

However, after reviewing the Intel 8255x Software Developer Manual,
i found that the EEPROM address field is 6 bits for a 64-regsiter EEPROM
and 8 bits for a 256-register EEPROM.

the driver also has:
	__le16 eeprom[256];

and calculates the EEPROM word count from address length.

Therefore, i think if (!addr_len || addr_len > 8) is more
appropriate validation than `>= 16`.
it validates the actual supported EEPROM address length.

> I'm for explicit (u16)BIT(addr_len), what do you think?

for the calculation itself, i agree with using:
	nic->eeprom_wc = (u16)BIT(addr_len);

1 << addr_len means shifting the value 1 by addr_len bits

for example: 
an 8-bit EEPROM address length gives 1 << 8 or 256 EEPROM words.

BIT(addr_len) expresses this bit operation explicitly, and the
(u16) cast makes result type match nic->eeprom_wc.

I will therefore change both e100_eeprom_load() and e100_eeprom_save()
to validate with addr_len > 8 and use (u16)BIT(addr_len).

Please let me know if you agree with using addr_len > 8 based on
the EEPROM address length limitation.

Also, would you prefer (u16)BIT(addr_len) for calculating eeprom_wc
or if you would prefer to keep the original shift expression?

Thank you!
-Pavan
> 
> 
> > 
> >  	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
>