From: Ziyan Fu fuzy5@lenovo.com
[ Upstream commit 40efc43eb7ffb5a4e2f998c13b8cfb555e671b92 ]
The driver probes with the invalid timeout value when 'iTCO_wdt_set_timeout()' fails, as its return value is not checked. In this case, when executing "wdctl", we may get:
Device: /dev/watchdog0 Timeout: 30 seconds Timeleft: 613 seconds
The timeout value is the value of "heartbeat" or "WATCHDOG_TIMEOUT", and the timeleft value is calculated from the register value we actually read (0xffff) by masking with 0x3ff and converting ticks to seconds (* 6 / 10).
Add error handling to return the failure code if 'iTCO_wdt_set_timeout()' fails, ensuring the driver probe fails and prevents invalid operation.
Signed-off-by: Ziyan Fu fuzy5@lenovo.com Reviewed-by: Guenter Roeck linux@roeck-us.net Link: https://lore.kernel.org/r/20250704073518.7838-1-13281011316@163.com Signed-off-by: Guenter Roeck linux@roeck-us.net Signed-off-by: Wim Van Sebroeck wim@linux-watchdog.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
**Backport Status: YES**
This commit should be backported to stable kernel trees for the following reasons:
## Bug Fix Analysis
1. **Critical Bug Being Fixed**: The commit fixes a probe failure handling bug where the driver continues to initialize with an invalid timeout value when `iTCO_wdt_set_timeout()` fails. This can lead to: - Watchdog operating with incorrect timeout values - Misleading information reported to userspace (timeout shows 30s but actual timer reads 0xffff) - Potential system instability due to watchdog misbehavior
2. **Clear Error Path Issue**: Looking at the code changes in drivers/watchdog/iTCO_wdt.c:579-584, the original code ignored the return value of the fallback `iTCO_wdt_set_timeout()` call. If this second attempt to set the default timeout also fails, the driver would continue initialization with a potentially corrupted watchdog state.
3. **Hardware Interaction Failure**: The `iTCO_wdt_set_timeout()` function (lines 351-395) performs hardware register writes and verifies them. A failure here means: - Hardware register writes failed to take effect - Verification showed the written value didn't match what was read back - The watchdog timer is in an undefined/invalid state
## Stable Backport Criteria
1. **Small and Contained Fix**: The change is minimal - only 4 lines added: - Captures return value of fallback timeout setting - Adds error checking and proper error return - Adds appropriate error logging
2. **No Architectural Changes**: This is a straightforward error handling fix that doesn't change any interfaces or introduce new features.
3. **Fixes Real User-Visible Issue**: The commit message shows this causes real problems visible to users through `wdctl` command showing incorrect timeout/timeleft values.
4. **Low Risk of Regression**: The change only adds error checking where none existed before. If the timeout setting succeeds (normal case), behavior is unchanged. If it fails, the driver now properly fails to probe instead of continuing with invalid state.
5. **Critical Subsystem**: Watchdog drivers are critical for system reliability and recovery. A malfunctioning watchdog that reports incorrect values or operates with wrong timeouts can lead to unexpected system reboots or failure to reboot when needed.
6. **Part of a Series of Fixes**: This appears to be from the same author (Ziyan Fu) who previously fixed a related issue in commit 158f9f2f7152 about updating the heartbeat value. This suggests ongoing work to fix error handling in this driver.
The fix prevents the driver from operating in an invalid state when hardware initialization fails, which is exactly the type of bug fix that stable kernels should include to maintain system reliability.
drivers/watchdog/iTCO_wdt.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c index 9ab769aa0244..4ab3405ef8e6 100644 --- a/drivers/watchdog/iTCO_wdt.c +++ b/drivers/watchdog/iTCO_wdt.c @@ -577,7 +577,11 @@ static int iTCO_wdt_probe(struct platform_device *pdev) /* Check that the heartbeat value is within it's range; if not reset to the default */ if (iTCO_wdt_set_timeout(&p->wddev, heartbeat)) { - iTCO_wdt_set_timeout(&p->wddev, WATCHDOG_TIMEOUT); + ret = iTCO_wdt_set_timeout(&p->wddev, WATCHDOG_TIMEOUT); + if (ret != 0) { + dev_err(dev, "Failed to set watchdog timeout (%d)\n", WATCHDOG_TIMEOUT); + return ret; + } dev_info(dev, "timeout value out of range, using %d\n", WATCHDOG_TIMEOUT); heartbeat = WATCHDOG_TIMEOUT;