net: usb: lan78xx: Fix lost EEPROM read timeout error(-ETIMEDOUT) in lan78xx_read_raw_eeprom
Subsystems (5)
PATCH 1/1 net: usb: lan78xx: Fix lost EEPROM read timeout error(-ETIMEDOUT) in lan78xx_read_raw_eeprom
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.
---
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(-)
drivers/net/usb/lan78xx.c +7 -4
@@ -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,
Signature · 1 line
2.34.1
Jakub Kicinski
maintainerdrivers/net/usb/lan78xx.c static int lan78xx_read_raw_eeprom(struct lan78xx_net *dev, u32 offset, · line 1088
Quoted 7 lines · On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
drivers/net/usb/lan78xx.c +7
-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;
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"?
Signature · 1 line
pw-bot: cr
Quoted 11 lines · Jakub Kicinski
drivers/net/usb/lan78xx.c static int lan78xx_read_raw_eeprom(struct lan78xx_net *dev, u32 offset, · line 1088
Quoted 7 lines · On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
drivers/net/usb/lan78xx.c +7
-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;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,
Quoted 13 lines · Bhanu Seshu Kumar Valluri
Quoted 11 lines · Jakub Kicinski
drivers/net/usb/lan78xx.c static int lan78xx_read_raw_eeprom(struct lan78xx_net *dev, u32 offset, · line 1088
Quoted 7 lines · On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
drivers/net/usb/lan78xx.c +7
-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;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"?
Quoted 3 lines
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_regreturns 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
Signature · 4 lines
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 |
Quoted 16 lines · Oleksij Rempel
Hi,
Quoted 13 lines · Bhanu Seshu Kumar Valluri
Quoted 11 lines · Jakub Kicinski
drivers/net/usb/lan78xx.c static int lan78xx_read_raw_eeprom(struct lan78xx_net *dev, u32 offset, · line 1088
Quoted 7 lines · On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
drivers/net/usb/lan78xx.c +7
-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;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"?
Quoted 8 lines
Quoted 3 lines
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_regreturns 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.
Quoted 21 lines · Bhanu Seshu Kumar Valluri
Quoted 20 lines · Oleksij Rempel
Hi,
Quoted 16 lines · Bhanu Seshu Kumar Valluri
Quoted 11 lines · Jakub Kicinski
drivers/net/usb/lan78xx.c static int lan78xx_read_raw_eeprom(struct lan78xx_net *dev, u32 offset, · line 1088
Quoted 7 lines · On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
drivers/net/usb/lan78xx.c +7
-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;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_regreturns success and timeout error status would be lost.
Quoted 8 lines
Quoted 4 lines
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?
Signature · 4 lines
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 |
Quoted 27 lines · Oleksij Rempel
Quoted 26 lines · Bhanu Seshu Kumar Valluri
Quoted 24 lines · Oleksij Rempel
Hi,
Quoted 13 lines · Bhanu Seshu Kumar Valluri
Quoted 11 lines · Jakub Kicinski
drivers/net/usb/lan78xx.c static int lan78xx_read_raw_eeprom(struct lan78xx_net *dev, u32 offset, · line 1088
Quoted 7 lines · On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
drivers/net/usb/lan78xx.c +7
-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;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"?
Quoted 3 lines
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_regreturns 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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 32 lines · Bhanu Seshu Kumar Valluri
Quoted 31 lines · Oleksij Rempel
Quoted 29 lines · Bhanu Seshu Kumar Valluri
Quoted 16 lines · Oleksij Rempel
Hi,
Quoted 13 lines · Bhanu Seshu Kumar Valluri
Quoted 11 lines · Jakub Kicinski
Review
Quoted 7 lines · On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
(quoted diff) +7
-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;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"?
Quoted 8 lines
Quoted 3 lines
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_regreturns 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.
Quoted 6 lines
Quoted 3 lines
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.
Signature · 4 lines
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 |
Quoted 37 lines · Oleksij Rempel
Quoted 36 lines · Bhanu Seshu Kumar Valluri
Quoted 34 lines · Oleksij Rempel
Quoted 21 lines · Bhanu Seshu Kumar Valluri
Quoted 20 lines · Oleksij Rempel
Hi,
Quoted 16 lines · Bhanu Seshu Kumar Valluri
Quoted 11 lines · Jakub Kicinski
Review
Quoted 7 lines · On Tue, 30 Sep 2025 14:19:02 +0530 Bhanu Seshu Kumar Valluri wrote:
(quoted diff) +7
-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;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_regreturns success and timeout error status would be lost.Quoted 8 lines
Quoted 4 lines
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?
Quoted 8 lines
Quoted 2 lines
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.
Quoted 29 lines · Bhanu Seshu Kumar Valluri
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:4707Local 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:3766The function
lan78xx_read_raw_eepromfailed 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.
Quoted 8 lines
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 [email protected]Fixes 8b1b2ca83b20 ("net: usb: lan78xx: Improve error handling in EEPROM and OTP operations")Signed-off-by Bhanu Seshu Kumar Valluri <[email protected]>
Signature · 4 lines
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 |
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
Quoted 13 lines · On Tue, 30 Sep 2025 14:19:02 +0530 you 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
Quoted 1 line
[...]
Here is the summary with links:
- net: usb: lan78xx: Fix lost EEPROM read timeout error(-ETIMEDOUT) in lan78xx_read_raw_eeprom
https://git.kernel.org/netdev/net/c/49bdb63ff644
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html