Re: [PATCH] watchdog: airoha: Prevent division by zero when clock frequency is zero

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unvalidated module parameter directly assigned to watchdog timeout leads to 32-bit integer overflow and immediate system reboot.
--

--- Patch [1]: [PATCH] watchdog: airoha: Prevent division by zero when clock frequency is zero ---
commit 3c89565f34ffbc693201939b137238a0f581ebf6
Author: Wayen Yan <[email protected]>

watchdog: airoha: Prevent division by zero when clock frequency is zero

This commit adds a check in airoha_wdt_probe to ensure the clock frequency
is not zero before using it as a divisor. It prevents a division by zero if
the clock provider is not properly configured.

> diff --git a/drivers/watchdog/airoha_wdt.c b/drivers/watchdog/airoha_wdt.c
> --- a/drivers/watchdog/airoha_wdt.c
> +++ b/drivers/watchdog/airoha_wdt.c
> @@ -147,6 +147,9 @@ static int airoha_wdt_probe(struct platform_device *pdev)
>  
>  	/* Watchdog ticks at half the bus rate */
>  	airoha_wdt->wdt_freq = clk_get_rate(bus_clk) / 2;
> +	if (!airoha_wdt->wdt_freq)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "invalid clock frequency\n");
>  
>  	/* Initialize struct watchdog device */
>  	wdog_dev = &airoha_wdt->wdog_dev;
>  	wdog_dev->timeout = heartbeat;

[Severity: High]
This is a pre-existing issue, but does assigning the heartbeat module
parameter directly here bypass watchdog timeout validation?

It appears watchdog_init_timeout() is not used to validate the module
parameter. If an out-of-bounds heartbeat parameter is provided, could this
lead to an integer overflow in airoha_wdt_start()?

drivers/watchdog/airoha_wdt.c:airoha_wdt_start() {
    ...
    val = wdog_dev->timeout * airoha_wdt->wdt_freq;
    writel(val, airoha_wdt->base + WDT_TIMER_LOAD_VALUE);
    ...
}

With a watchdog frequency of 150MHz, a timeout of 29 or greater results in
a multiplication over 4.29 billion, which overflows the 32-bit val
variable. This wrap-around would cause the hardware timer to expire
prematurely before the core ping worker expects to service it, resulting
in an immediate system reset.

Similarly, if heartbeat is 0, is it possible for the timer load value to
become 0 and trigger an immediate reset upon device open?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1