On 10/27/24 03:53, Stefan Wahren wrote:
The WDOG Timeout Value (TOVAL) is a 16 bit value, which is stored at the beginning of a 32 bit register. So add a range check to prevent writing in the reserved register area. Signed-off-by: Stefan Wahren <wahrenst@xxxxxxx> --- drivers/watchdog/imx7ulp_wdt.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c index 0f92d2217088..a7574f9c9150 100644 --- a/drivers/watchdog/imx7ulp_wdt.c +++ b/drivers/watchdog/imx7ulp_wdt.c @@ -48,6 +48,8 @@ #define RETRY_MAX 5 +#define TOVAL_MAX 0xFFFF + static bool nowayout = WATCHDOG_NOWAYOUT; module_param(nowayout, bool, 0000); MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" @@ -192,6 +194,9 @@ static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog, int ret; u32 loop = RETRY_MAX; + if (toval > TOVAL_MAX) + return -EINVAL; +
The whole idea of having max_timeout in struct watchdog_device is to avoid the need for this check. max_timeout should be set to 0xffff / wdt->hw->wdog_clock_rate. It is currently set to 128. With wdt->hw->wdog_clock_rate set to either 125 or 1000, it can indeed overflow. However, checking the value above is wrong. max_timeout should be initialized correctly instead. Even better would be to set max_hw_heartbeat_ms and let the watchdog core handle larger timeouts. Another question is why the driver enables a clock but doesn't use its actual frequency. Guenter
