Re: [PATCH 2/2] watchdog: sama5d4: use platform_get_irq_optional()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Race condition leading to NULL pointer dereference if the watchdog interrupt fires before probe completes.
--

commit 81aa11d20f236c5efcd53479d7c7d1601cbc92d0
Author: Rosen Penev <rosenp@gmail.com>
watchdog: sama5d4: use platform_get_irq_optional()

irq_of_parse_and_map() requires irq_dispose_mapping() on failure. Don't
bother with it as platform_get_irq_optional() doesn't need it.

Also handle EPROBE_DEFER.

> diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
> index b7b5b24afb34b..d218fc04b770c 100644
> --- a/drivers/watchdog/sama5d4_wdt.c
> +++ b/drivers/watchdog/sama5d4_wdt.c

[ ... ]

> @@ -287,8 +286,11 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	if (wdt->need_irq) {
> -		irq = irq_of_parse_and_map(dev->of_node, 0);
> -		if (!irq) {
> +		irq = platform_get_irq_optional(pdev, 0);
> +		if (irq == -EPROBE_DEFER)
> +			return irq;
> +
> +		if (irq < 0) {
>  			dev_warn(dev, "failed to get IRQ from DT\n");
>  			wdt->need_irq = false;
>  		}

[Severity: High]
This is a pre-existing issue, but since the IRQ handling is being updated
here, could this lead to a NULL pointer dereference if the interrupt fires
early?

Further down in sama5d4_wdt_probe(), the interrupt is registered and
unmasked, passing pdev as the dev_id context:

drivers/watchdog/sama5d4_wdt.c:sama5d4_wdt_probe() {
        ...
        ret = devm_request_irq(dev, irq, sama5d4_wdt_irq_handler,
                               IRQF_IRQPOLL | IRQF_NO_SUSPEND,
                               pdev->name, pdev);
        ...
        ret = sama5d4_wdt_init(wdt, wdd->timeout);
        ...
}

The sama5d4_wdt_init() function enables the hardware interrupts, creating
a race window because platform_set_drvdata(pdev, wdt) is not called until
the very end of the probe function.

If an interrupt fires before platform_set_drvdata() completes (for example,
if left pending by the bootloader), the interrupt handler will retrieve a NULL
pointer:

drivers/watchdog/sama5d4_wdt.c:sama5d4_wdt_irq_handler() {
        struct sama5d4_wdt *wdt = platform_get_drvdata(dev_id);
        ...
        if (wdt->sam9x60_support)
        ...
}

Should platform_set_drvdata() be moved up before the interrupt is
requested, or should devm_request_irq() be passed the wdt pointer directly
instead of pdev?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604010542.23177-1-rosenp@gmail.com?part=2