Re: [PATCH] watchdog: npcm: fix clock prepare leak when setting timeout on a running watchdog

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] watchdog: npcm: sleep in atomic context in npcm_wdt_restart()
  [1]
--

commit 17842ca84b08ec6bbd438017f39f767a1910936b
Author: Zhao Feng <zhaofeng2@cmss.chinamobile.com>

watchdog: npcm: fix clock prepare leak when setting timeout on a running watchdog

This commit splits register programming out of npcm_wdt_start() into
__npcm_wdt_start() so that npcm_wdt_set_timeout() does not acquire an
extra clock reference when updating the timeout on an active watchdog.

> diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
> index 51348969bf492..31253fd7bad5b 100644
> --- a/drivers/watchdog/npcm_wdt.c
> +++ b/drivers/watchdog/npcm_wdt.c
> @@ -145,13 +145,11 @@ static int npcm_wdt_ping(struct watchdog_device *wdd)
>  	return 0;
>  }
>  
> -static int npcm_wdt_start(struct watchdog_device *wdd)
> +static void __npcm_wdt_start(struct watchdog_device *wdd)
>  {
>  	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
>  	u32 val;
>  
> -	clk_prepare_enable(wdt->clk);
> -
>  	if (wdd->timeout < 2)
>  		val = 0x800;
>  	else if (wdd->timeout < 3)
> @@ -176,6 +174,15 @@ static int npcm_wdt_start(struct watchdog_device *wdd)
>  	val |= NPCM_WTRE | NPCM_WTE | NPCM_WTR | NPCM_WTIE;
>  
>  	writel(val, wdt->reg);
> +}
> +
> +static int npcm_wdt_start(struct watchdog_device *wdd)
> +{
> +	struct npcm_wdt *wdt = to_npcm_wdt(wdd);
> +
> +	clk_prepare_enable(wdt->clk);
> +
> +	__npcm_wdt_start(wdd);
>  
>  	return 0;
>  }

[Severity: High]
This problem wasn't introduced by this patch, but can npcm_wdt_restart()
sleep in atomic context when calling clk_prepare_enable(wdt->clk)
[1]?

drivers/watchdog/npcm_wdt.c:npcm_wdt_restart() {
	struct npcm_wdt *wdt = to_npcm_wdt(wdd);

	/* For reset, we start the WDT clock and leave it running. */
	clk_prepare_enable(wdt->clk);
    ...
}

do_kernel_restart() invokes restart handlers via
atomic_notifier_call_chain() under an RCU read lock, whereas
clk_prepare_enable() calls clk_prepare() and takes the sleeping
prepare_lock mutex.  This triggers a scheduling-while-atomic bug and can
deadlock during reboot or panic recovery.

[1] https://sashiko.dev/#/bug/linux-d877d7dc-83e2-4c44-b88a-c3f7ea954cd4

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109071004.1506-1-zhaofeng2@cmss.chinamobile.com?part=1