Re: [PATCH] watchdog: at91sam9_wdt: prevent timer rearm during teardown

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the watchdog interrupt handler unconditionally returns IRQ_HANDLED, breaking spurious interrupt detection for shared IRQ lines.
--

--- Patch [1]: [PATCH] watchdog: at91sam9_wdt: prevent timer rearm during teardown ---
commit ed1f4752c24ee2e6219380915010fe895108b4d9
Author: Hongyan Xu <[email protected]>
Subject: watchdog: at91sam9_wdt: prevent timer rearm during teardown

This commit replaces timer_delete() with timer_shutdown_sync() on the 
teardown paths. This ensures the timer callback cannot rearm the timer 
after driver removal or probe failure, avoiding a potential use-after-free 
on devm-allocated memory.

> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> index aba66b8e9d033..80ba04df54adf 100644
> --- a/drivers/watchdog/at91sam9_wdt.c
> +++ b/drivers/watchdog/at91sam9_wdt.c
> @@ -242,7 +242,7 @@ static int at91_wdt_init(struct platform_device *pdev, struct at91wdt *wdt)
>  	return 0;
>  
>  out_stop_timer:
> -	timer_delete(&wdt->timer);
> +	timer_shutdown_sync(&wdt->timer);
>  	return err;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but while looking at the driver
initialization, I noticed a potential issue with how the shared interrupt is
handled.

Since at91_wdt_init() registers the interrupt with IRQF_SHARED, could
wdt_interrupt() cause problems by unconditionally returning IRQ_HANDLED?

drivers/watchdog/at91sam9_wdt.c:wdt_interrupt() {
    ...
    if (wdt_read(wdt, AT91_WDT_SR)) {
        pr_crit("at91sam9 WDT software reset\n");
        emergency_restart();
        pr_crit("Reboot didn't ?????\n");
    }

    return IRQ_HANDLED;
}

If another device sharing this IRQ line triggers an interrupt and fails
to clear it, this handler will still return IRQ_HANDLED even when
AT91_WDT_SR is zero. 

Will this prevent the core kernel's spurious IRQ detector from disabling
the broken interrupt line, potentially leading to an interrupt storm and
system lockup? 

Should this return IRQ_NONE when the watchdog did not generate the
interrupt?

[ ... ]

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