Re: [PATCH] watchdog: mediatek: Enable pretimeout support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Null pointer dereference if watchdog interrupt fires during probe before driver data is set.
- [High] Unconditional spinlock acquisition in restart handler introduces a deadlock risk during kernel panic.
--

--- Patch [1]: [PATCH] watchdog: mediatek: Enable pretimeout support ---
commit d882d4fa54231095050976209e4b882cc24fd54d
Author: Wanming Gao <wanming.gao@mediatek.com>
Subject: watchdog: mediatek: Enable pretimeout support

The watchdog pretimeout interrupt needs to be deasserted after the
hardware pretimeout event is received. Add IRQ level control handling
to the MediaTek watchdog interrupt handler so the interrupt is cleared
before notifying the watchdog core.

> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 91d110646e16f..654194272e854 100644
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
[ ... ]
> @@ -230,14 +230,17 @@ static int mtk_wdt_restart(struct watchdog_device *wdt_dev,
>  {
>  	struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdt_dev);
>  	void __iomem *wdt_base;
> +	unsigned long flags;
>  	u32 reg;
>  
>  	wdt_base = mtk_wdt->wdt_base;
>  
>  	/* Enable reset in order to issue a system reset instead of an IRQ */
> +	spin_lock_irqsave(&mtk_wdt->lock, flags);

[Severity: High]
Does acquiring this spinlock unconditionally risk a deadlock during a system
panic?

The restart handler is called during emergency restarts. If another CPU was
stopped via an IPI while holding mtk_wdt->lock (such as during the 70
microsecond delay in mtk_wdt_deassert_irq()), the panicking CPU will spin
infinitely here and hang the emergency reboot.

>  	reg = readl(wdt_base + WDT_MODE);
>  	reg &= ~WDT_MODE_IRQ_EN;
>  	writel(reg | WDT_MODE_KEY, wdt_base + WDT_MODE);
> +	spin_unlock_irqrestore(&mtk_wdt->lock, flags);
>  
>  	while (1) {
>  		writel(WDT_SWRST_KEY, wdt_base + WDT_SWRST);
[ ... ]
> +static void mtk_wdt_deassert_irq(struct watchdog_device *wdd)
> +{
> +	struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdd);

[Severity: High]
Will watchdog_get_drvdata() return NULL here if a pending interrupt fires
during probe?

In mtk_wdt_probe(), devm_request_irq() is called to enable the interrupt before
watchdog_set_drvdata() initializes the driver data. If the bootloader left the
pretimeout interrupt enabled and it fires immediately during probe,
mtk_wdt_isr() will run and call mtk_wdt_deassert_irq().

This would result in watchdog_get_drvdata() returning NULL, followed by a NULL
pointer dereference when accessing mtk_wdt->wdt_base.

> +	void __iomem *wdt_base = mtk_wdt->wdt_base;
> +	unsigned long flags;
> +	u32 reg;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824132914.2662346-1-wanming.gao@mediatek.com?part=1