drivers/net/usb/lan78xx.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
Syzbot reported read of uninitialized variable BUG with following call stack.
lan78xx 8-1:1.0 (unnamed net_device) (uninitialized): EEPROM read operation timeout
=====================================================
BUG: KMSAN: uninit-value in lan78xx_read_eeprom drivers/net/usb/lan78xx.c:1095 [inline]
BUG: KMSAN: uninit-value in lan78xx_init_mac_address drivers/net/usb/lan78xx.c:1937 [inline]
BUG: KMSAN: uninit-value in lan78xx_reset+0x999/0x2cd0 drivers/net/usb/lan78xx.c:3241
lan78xx_read_eeprom drivers/net/usb/lan78xx.c:1095 [inline]
lan78xx_init_mac_address drivers/net/usb/lan78xx.c:1937 [inline]
lan78xx_reset+0x999/0x2cd0 drivers/net/usb/lan78xx.c:3241
lan78xx_bind+0x711/0x1690 drivers/net/usb/lan78xx.c:3766
lan78xx_probe+0x225c/0x3310 drivers/net/usb/lan78xx.c:4707
Local variable sig.i.i created at:
lan78xx_read_eeprom drivers/net/usb/lan78xx.c:1092 [inline]
lan78xx_init_mac_address drivers/net/usb/lan78xx.c:1937 [inline]
lan78xx_reset+0x77e/0x2cd0 drivers/net/usb/lan78xx.c:3241
lan78xx_bind+0x711/0x1690 drivers/net/usb/lan78xx.c:3766
The function lan78xx_read_raw_eeprom failed to properly propagate EEPROM
read timeout errors (-ETIMEDOUT). In the fallthrough path, it first
attempted to restore the pin configuration for LED outputs and then
returned only the status of that restore operation, discarding the
original timeout error.
As a result, callers could mistakenly treat the data buffer as valid
even though the EEPROM read had actually timed out with no data or partial
data.
To fix this, handle errors in restoring the LED pin configuration separately.
If the restore succeeds, return any prior EEPROM timeout error correctly
to the caller.
Reported-by: syzbot+62ec8226f01cb4ca19d9@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=62ec8226f01cb4ca19d9
Fixes: 8b1b2ca83b20 ("net: usb: lan78xx: Improve error handling in EEPROM and OTP operations")
Signed-off-by: Bhanu Seshu Kumar Valluri <bhanuseshukumar@gmail.com>
---
Note: The patch is compiled and tested using EVB-LAN7800LC.
The Sysbot doesn't have C reproducer to get the patch tested by sysbot.
drivers/net/usb/lan78xx.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 1ff25f57329a..d75502ebbc0d 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1079,10 +1079,13 @@ static int lan78xx_read_raw_eeprom(struct lan78xx_net *dev, u32 offset,
}
read_raw_eeprom_done:
- if (dev->chipid == ID_REV_CHIP_ID_7800_)
- return lan78xx_write_reg(dev, HW_CFG, saved);
-
- return 0;
+ if (dev->chipid == ID_REV_CHIP_ID_7800_) {
+ int rc = lan78xx_write_reg(dev, HW_CFG, saved);
+ /* If USB fails, there is nothing to do */
+ if (rc < 0)
+ return rc;
+ }
+ return ret;
}
static int lan78xx_read_eeprom(struct lan78xx_net *dev, u32 offset,
--
2.34.1On Tue, Sep 30, 2025 at 02:19:02PM +0530, Bhanu Seshu Kumar Valluri wrote:
> Syzbot reported read of uninitialized variable BUG with following call stack.
>
> lan78xx 8-1:1.0 (unnamed net_device) (uninitialized): EEPROM read operation timeout
> =====================================================
> BUG: KMSAN: uninit-value in lan78xx_read_eeprom drivers/net/usb/lan78xx.c:1095 [inline]
> BUG: KMSAN: uninit-value in lan78xx_init_mac_address drivers/net/usb/lan78xx.c:1937 [inline]
> BUG: KMSAN: uninit-value in lan78xx_reset+0x999/0x2cd0 drivers/net/usb/lan78xx.c:3241
> lan78xx_read_eeprom drivers/net/usb/lan78xx.c:1095 [inline]
> lan78xx_init_mac_address drivers/net/usb/lan78xx.c:1937 [inline]
> lan78xx_reset+0x999/0x2cd0 drivers/net/usb/lan78xx.c:3241
> lan78xx_bind+0x711/0x1690 drivers/net/usb/lan78xx.c:3766
> lan78xx_probe+0x225c/0x3310 drivers/net/usb/lan78xx.c:4707
>
> Local variable sig.i.i created at:
> lan78xx_read_eeprom drivers/net/usb/lan78xx.c:1092 [inline]
> lan78xx_init_mac_address drivers/net/usb/lan78xx.c:1937 [inline]
> lan78xx_reset+0x77e/0x2cd0 drivers/net/usb/lan78xx.c:3241
> lan78xx_bind+0x711/0x1690 drivers/net/usb/lan78xx.c:3766
>
> The function lan78xx_read_raw_eeprom failed to properly propagate EEPROM
> read timeout errors (-ETIMEDOUT). In the fallthrough path, it first
> attempted to restore the pin configuration for LED outputs and then
> returned only the status of that restore operation, discarding the
> original timeout error.
>
> As a result, callers could mistakenly treat the data buffer as valid
> even though the EEPROM read had actually timed out with no data or partial
> data.
>
> To fix this, handle errors in restoring the LED pin configuration separately.
> If the restore succeeds, return any prior EEPROM timeout error correctly
> to the caller.
>
> Reported-by: syzbot+62ec8226f01cb4ca19d9@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=62ec8226f01cb4ca19d9
> Fixes: 8b1b2ca83b20 ("net: usb: lan78xx: Improve error handling in EEPROM and OTP operations")
> Signed-off-by: Bhanu Seshu Kumar Valluri <bhanuseshukumar@gmail.com>
Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
> + if (dev->chipid == ID_REV_CHIP_ID_7800_) {
> + int rc = lan78xx_write_reg(dev, HW_CFG, saved);
> + /* If USB fails, there is nothing to do */
> + if (rc < 0)
> + return rc;
> + }
> + return ret;
I don't think you need to add and handle rc here separately?
rc can only be <= so save the answer to ret and "fall thru"?
--
pw-bot: cr
On 01/10/25 06:09, Jakub Kicinski wrote:
> On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
>> + if (dev->chipid == ID_REV_CHIP_ID_7800_) {
>> + int rc = lan78xx_write_reg(dev, HW_CFG, saved);
>> + /* If USB fails, there is nothing to do */
>> + if (rc < 0)
>> + return rc;
>> + }
>> + return ret;
>
> I don't think you need to add and handle rc here separately?
> rc can only be <= so save the answer to ret and "fall thru"?
The fall thru path might have been reached with ret holding EEPROM read timeout
error status. So if ret is used instead of rc it might over write the ret with 0 when
lan78xx_write_reg returns success and timeout error status would be lost.
Thank you.
Hi,
On Wed, Oct 01, 2025 at 10:07:21AM +0530, Bhanu Seshu Kumar Valluri wrote:
> On 01/10/25 06:09, Jakub Kicinski wrote:
> > On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
> >> + if (dev->chipid == ID_REV_CHIP_ID_7800_) {
> >> + int rc = lan78xx_write_reg(dev, HW_CFG, saved);
> >> + /* If USB fails, there is nothing to do */
> >> + if (rc < 0)
> >> + return rc;
> >> + }
> >> + return ret;
> >
> > I don't think you need to add and handle rc here separately?
> > rc can only be <= so save the answer to ret and "fall thru"?
>
> The fall thru path might have been reached with ret holding EEPROM read timeout
> error status. So if ret is used instead of rc it might over write the ret with 0 when
> lan78xx_write_reg returns success and timeout error status would be lost.
Ack, I see. It may happen if communication with EEPROM will fail. The same
would happen on write path too. Is it happened with real HW or it is
some USB emulation test? For me it is interesting why EEPROM is timed
out.
Best Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 01/10/25 13:12, Oleksij Rempel wrote:
> Hi,
>
> On Wed, Oct 01, 2025 at 10:07:21AM +0530, Bhanu Seshu Kumar Valluri wrote:
>> On 01/10/25 06:09, Jakub Kicinski wrote:
>>> On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
>>>> + if (dev->chipid == ID_REV_CHIP_ID_7800_) {
>>>> + int rc = lan78xx_write_reg(dev, HW_CFG, saved);
>>>> + /* If USB fails, there is nothing to do */
>>>> + if (rc < 0)
>>>> + return rc;
>>>> + }
>>>> + return ret;
>>>
>>> I don't think you need to add and handle rc here separately?
>>> rc can only be <= so save the answer to ret and "fall thru"?
>>
>> The fall thru path might have been reached with ret holding EEPROM read timeout
>> error status. So if ret is used instead of rc it might over write the ret with 0 when
>> lan78xx_write_reg returns success and timeout error status would be lost.
>
> Ack, I see. It may happen if communication with EEPROM will fail. The same
> would happen on write path too. Is it happened with real HW or it is
> some USB emulation test? For me it is interesting why EEPROM is timed
> out.
The sysbot's log with message "EEPROM read operation timeout" confirms that EEPROM read
timeout occurring. I tested the same condition on EVB-LAN7800LC by simulating
timeout during probe.
Thanks.
On Wed, Oct 01, 2025 at 01:40:56PM +0530, Bhanu Seshu Kumar Valluri wrote:
> On 01/10/25 13:12, Oleksij Rempel wrote:
> > Hi,
> >
> > On Wed, Oct 01, 2025 at 10:07:21AM +0530, Bhanu Seshu Kumar Valluri wrote:
> >> On 01/10/25 06:09, Jakub Kicinski wrote:
> >>> On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
> >>>> + if (dev->chipid == ID_REV_CHIP_ID_7800_) {
> >>>> + int rc = lan78xx_write_reg(dev, HW_CFG, saved);
> >>>> + /* If USB fails, there is nothing to do */
> >>>> + if (rc < 0)
> >>>> + return rc;
> >>>> + }
> >>>> + return ret;
> >>>
> >>> I don't think you need to add and handle rc here separately?
> >>> rc can only be <= so save the answer to ret and "fall thru"?
> >>
> >> The fall thru path might have been reached with ret holding EEPROM read timeout
> >> error status. So if ret is used instead of rc it might over write the ret with 0 when
> >> lan78xx_write_reg returns success and timeout error status would be lost.
> >
> > Ack, I see. It may happen if communication with EEPROM will fail. The same
> > would happen on write path too. Is it happened with real HW or it is
> > some USB emulation test? For me it is interesting why EEPROM is timed
> > out.
>
> The sysbot's log with message "EEPROM read operation timeout" confirms that EEPROM read
> timeout occurring. I tested the same condition on EVB-LAN7800LC by simulating
> timeout during probe.
Do you simulating timeout during probe by modifying the code, or it is
real HW issue?
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 01/10/25 13:54, Oleksij Rempel wrote:
> On Wed, Oct 01, 2025 at 01:40:56PM +0530, Bhanu Seshu Kumar Valluri wrote:
>> On 01/10/25 13:12, Oleksij Rempel wrote:
>>> Hi,
>>>
>>> On Wed, Oct 01, 2025 at 10:07:21AM +0530, Bhanu Seshu Kumar Valluri wrote:
>>>> On 01/10/25 06:09, Jakub Kicinski wrote:
>>>>> On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
>>>>>> + if (dev->chipid == ID_REV_CHIP_ID_7800_) {
>>>>>> + int rc = lan78xx_write_reg(dev, HW_CFG, saved);
>>>>>> + /* If USB fails, there is nothing to do */
>>>>>> + if (rc < 0)
>>>>>> + return rc;
>>>>>> + }
>>>>>> + return ret;
>>>>>
>>>>> I don't think you need to add and handle rc here separately?
>>>>> rc can only be <= so save the answer to ret and "fall thru"?
>>>>
>>>> The fall thru path might have been reached with ret holding EEPROM read timeout
>>>> error status. So if ret is used instead of rc it might over write the ret with 0 when
>>>> lan78xx_write_reg returns success and timeout error status would be lost.
>>>
>>> Ack, I see. It may happen if communication with EEPROM will fail. The same
>>> would happen on write path too. Is it happened with real HW or it is
>>> some USB emulation test? For me it is interesting why EEPROM is timed
>>> out.
>>
>> The sysbot's log with message "EEPROM read operation timeout" confirms that EEPROM read
>> timeout occurring. I tested the same condition on EVB-LAN7800LC by simulating
>> timeout during probe.
>
> Do you simulating timeout during probe by modifying the code, or it is
> real HW issue?
>
On my real hardware timeout didn't occur. So I simulated it once by modifying the code
to confirm the BUG. The BUG has occurred confirming syzbot finding.
Thanks.
On Wed, Oct 01, 2025 at 02:01:24PM +0530, Bhanu Seshu Kumar Valluri wrote:
> On 01/10/25 13:54, Oleksij Rempel wrote:
> > On Wed, Oct 01, 2025 at 01:40:56PM +0530, Bhanu Seshu Kumar Valluri wrote:
> >> On 01/10/25 13:12, Oleksij Rempel wrote:
> >>> Hi,
> >>>
> >>> On Wed, Oct 01, 2025 at 10:07:21AM +0530, Bhanu Seshu Kumar Valluri wrote:
> >>>> On 01/10/25 06:09, Jakub Kicinski wrote:
> >>>>> On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
> >>>>>> + if (dev->chipid == ID_REV_CHIP_ID_7800_) {
> >>>>>> + int rc = lan78xx_write_reg(dev, HW_CFG, saved);
> >>>>>> + /* If USB fails, there is nothing to do */
> >>>>>> + if (rc < 0)
> >>>>>> + return rc;
> >>>>>> + }
> >>>>>> + return ret;
> >>>>>
> >>>>> I don't think you need to add and handle rc here separately?
> >>>>> rc can only be <= so save the answer to ret and "fall thru"?
> >>>>
> >>>> The fall thru path might have been reached with ret holding EEPROM read timeout
> >>>> error status. So if ret is used instead of rc it might over write the ret with 0 when
> >>>> lan78xx_write_reg returns success and timeout error status would be lost.
> >>>
> >>> Ack, I see. It may happen if communication with EEPROM will fail. The same
> >>> would happen on write path too. Is it happened with real HW or it is
> >>> some USB emulation test? For me it is interesting why EEPROM is timed
> >>> out.
> >>
> >> The sysbot's log with message "EEPROM read operation timeout" confirms that EEPROM read
> >> timeout occurring. I tested the same condition on EVB-LAN7800LC by simulating
> >> timeout during probe.
> >
> > Do you simulating timeout during probe by modifying the code, or it is
> > real HW issue?
> >
>
> On my real hardware timeout didn't occur. So I simulated it once by modifying the code
> to confirm the BUG. The BUG has occurred confirming syzbot finding.
Ok, thank you!
Can you please add similar change to lan78xx_write_raw_eeprom. syzbot
will find it soon or later.
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 01/10/25 14:08, Oleksij Rempel wrote:
> On Wed, Oct 01, 2025 at 02:01:24PM +0530, Bhanu Seshu Kumar Valluri wrote:
>> On 01/10/25 13:54, Oleksij Rempel wrote:
>>> On Wed, Oct 01, 2025 at 01:40:56PM +0530, Bhanu Seshu Kumar Valluri wrote:
>>>> On 01/10/25 13:12, Oleksij Rempel wrote:
>>>>> Hi,
>>>>>
>>>>> On Wed, Oct 01, 2025 at 10:07:21AM +0530, Bhanu Seshu Kumar Valluri wrote:
>>>>>> On 01/10/25 06:09, Jakub Kicinski wrote:
>>>>>>> On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
>>>>>>>> + if (dev->chipid == ID_REV_CHIP_ID_7800_) {
>>>>>>>> + int rc = lan78xx_write_reg(dev, HW_CFG, saved);
>>>>>>>> + /* If USB fails, there is nothing to do */
>>>>>>>> + if (rc < 0)
>>>>>>>> + return rc;
>>>>>>>> + }
>>>>>>>> + return ret;
>>>>>>>
>>>>>>> I don't think you need to add and handle rc here separately?
>>>>>>> rc can only be <= so save the answer to ret and "fall thru"?
>>>>>>
>>>>>> The fall thru path might have been reached with ret holding EEPROM read timeout
>>>>>> error status. So if ret is used instead of rc it might over write the ret with 0 when
>>>>>> lan78xx_write_reg returns success and timeout error status would be lost.
>>>>>
>>>>> Ack, I see. It may happen if communication with EEPROM will fail. The same
>>>>> would happen on write path too. Is it happened with real HW or it is
>>>>> some USB emulation test? For me it is interesting why EEPROM is timed
>>>>> out.
>>>>
>>>> The sysbot's log with message "EEPROM read operation timeout" confirms that EEPROM read
>>>> timeout occurring. I tested the same condition on EVB-LAN7800LC by simulating
>>>> timeout during probe.
>>>
>>> Do you simulating timeout during probe by modifying the code, or it is
>>> real HW issue?
>>>
>>
>> On my real hardware timeout didn't occur. So I simulated it once by modifying the code
>> to confirm the BUG. The BUG has occurred confirming syzbot finding.
>
> Ok, thank you!
>
> Can you please add similar change to lan78xx_write_raw_eeprom. syzbot
> will find it soon or later.
>
Ok. I will try to send a separate patch for that.
Thank you.
© 2016 - 2025 Red Hat, Inc.