[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
> 

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

[Yalagada Pavan Kumar]

Hi Andrew,

I just wanted to bring this to your attention.

bug: [https://syzkaller.appspot.com/bug?extid=e0abb1d45ac291ebebeb]

I originally sent this patch on July 7, sent v2 yesterday based on the review feedback.

I noticed that another patch with the same change was posted yesterday, and v2 of that
patch has now been sent to net-next today.

Since my original patch predates that patch, could you please let me know which version
you would prefer to proceed with?

I'm happy to continue with my patch and address any further review comments if needed.

Thanks,
Pavan