Re: [PATCH net v3] e100: prevent shift-out-of-bounds in EEPROM access
Tony Nguyen <[email protected]>
| Newsgroups | org.osuosl.intel-wired-lan,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
On 8/16/2026 4:37 AM, [email protected] wrote: > From: Yalagada Pavan Kumar <[email protected]> > > When reading the EEPROM address length, e100_eeprom_read() can return > an invalid length. This value is then used as a shift count when > calculating eeprom_wc, resulting in a shift-out-of-bounds UBSAN warning. > > Stop EEPROM address probing if the detected address length exceeds > the initial address length to prevent addr_len from underflowing. > > Validate addr_len after reading it from the EEPROM in both > e100_eeprom_load() and e100_eeprom_save(), and return -EIO > if the value is zero or greater than 8. > > Reported-by: [email protected] > Closes: https://syzkaller.appspot.com/bug?extid=e0abb1d45ac291ebebeb > Signed-off-by: Yalagada Pavan Kumar <[email protected]> > --- > v3: > - Prevent addr_len underflow during EEPROM probing. > - Use BIT() and explicitly cast the result to u16 when assigning eeprom_wc. > - Update the commit message and subject to reflect the scope of the fix. > - verify the UBSAN failure with syzkaller reproducer before the fix > and its absence after the fix. > > v2: https://lore.kernel.org/all/[email protected]/ > - 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/[email protected]/ > --- > drivers/net/ethernet/intel/e100.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c > index 464dc2d6cdb5..5165f1d3c98f 100644 > --- a/drivers/net/ethernet/intel/e100.c > +++ b/drivers/net/ethernet/intel/e100.c > @@ -746,10 +746,12 @@ static __le16 e100_eeprom_read(struct nic *nic, u16 *addr_len, u16 addr) > if (!(ctrl & eedo) && i > 16) { > u16 len = i - 16; > > - if (len > *addr_len) > + if (len > *addr_len) { > *addr_len = 0; > - else > - *addr_len -= len; > + break; > + } > + > + *addr_len -= len; > i = 17; It seems each version you are sending is change on top of the previous version. Please send all the changes in a single patch so that it can be reviewed, and applied, standalone. Also, Andrew had requested on the other patch that this go via *-next since the issue is a theoretical one. Can you target this to iwl-next? Thanks, Tony > } >