[PATCH] rtc: ma35d1: fix unchecked IRQ error and IRQ-before-rtcdev ordering

[PATCH] rtc: ma35d1: fix unchecked IRQ error and IRQ-before-rtcdev ordering

[Cong Nguyen]

platform_get_irq()'s return goes straight into rtc->irq_num and then
devm_request_irq()'s unsigned int irq parameter with no check --
passing a negative error (like -EPROBE_DEFER) through that conversion
turns it into a large positive number, so devm_request_irq() just
fails with -EINVAL instead of deferring the probe.

Separately, the IRQ is requested before rtc->rtcdev is allocated and
registered. Since devm teardown is LIFO, unbind or a later probe
failure frees rtcdev before the IRQ, so an alarm firing in that window
has ma35d1_rtc_interrupt() call rtc_update_irq() on freed memory.

Check irq_num for a negative return, and move the IRQ request to
after the RTC device is fully registered.

Fixes: dc0684adf3b6 ("rtc: Add driver for Nuvoton ma35d1 rtc controller")
Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/r/20260914133532.CBFAA1F000FF@smtp.kernel.org
Assisted-by: LLM
Signed-off-by: Cong Nguyen <congnt264@gmail.com>
---
 drivers/rtc/rtc-ma35d1.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-ma35d1.c b/drivers/rtc/rtc-ma35d1.c
index cfcfc28060f6..76d59b313721 100644
--- a/drivers/rtc/rtc-ma35d1.c
+++ b/drivers/rtc/rtc-ma35d1.c
@@ -236,11 +236,8 @@ static int ma35d1_rtc_probe(struct platform_device *pdev)
 	}
 
 	rtc->irq_num = platform_get_irq(pdev, 0);
-
-	ret = devm_request_irq(&pdev->dev, rtc->irq_num, ma35d1_rtc_interrupt,
-			       IRQF_NO_SUSPEND, "ma35d1rtc", rtc);
-	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "Failed to request rtc irq\n");
+	if (rtc->irq_num < 0)
+		return dev_err_probe(&pdev->dev, rtc->irq_num, "failed to get rtc irq\n");
 
 	platform_set_drvdata(pdev, rtc);
 
@@ -258,6 +255,11 @@ static int ma35d1_rtc_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "Failed to register rtc device\n");
 
+	ret = devm_request_irq(&pdev->dev, rtc->irq_num, ma35d1_rtc_interrupt,
+			       IRQF_NO_SUSPEND, "ma35d1rtc", rtc);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Failed to request rtc irq\n");
+
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH] rtc: ma35d1: fix unchecked IRQ error and IRQ-before-rtcdev ordering

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] rtc: devm_request_irq() called after devm_rtc_register_device()
- [Low] rtc: redundant dev_err_probe() on platform_get_irq() failure
--

commit 65db24f320f8394c839e4022831930f15fb56e95
Author: Cong Nguyen <congnt264@gmail.com>
rtc: ma35d1: fix unchecked IRQ error and IRQ-before-rtcdev ordering

This commit checks the return value of platform_get_irq() to prevent passing
a negative error code to devm_request_irq(). It also moves the IRQ
request to after the RTC device is registered, intending to fix the teardown
order.

Link: https://lore.kernel.org/r/20260914133532.CBFAA1F000FF@smtp.kernel.org

> diff --git a/drivers/rtc/rtc-ma35d1.c b/drivers/rtc/rtc-ma35d1.c
> --- a/drivers/rtc/rtc-ma35d1.c
> +++ b/drivers/rtc/rtc-ma35d1.c
> @@ -236,11 +236,8 @@ static int ma35d1_rtc_probe(struct platform_device *pdev)
>  	}
>  
>  	rtc->irq_num = platform_get_irq(pdev, 0);
> -
> -	ret = devm_request_irq(&pdev->dev, rtc->irq_num, ma35d1_rtc_interrupt,
> -			       IRQF_NO_SUSPEND, "ma35d1rtc", rtc);
> -	if (ret)
> -		return dev_err_probe(&pdev->dev, ret, "Failed to request rtc irq\n");
> +	if (rtc->irq_num < 0)
> +		return dev_err_probe(&pdev->dev, rtc->irq_num, "failed to get rtc irq\n");

[Severity: Low]
Does this cause a redundant error message to be printed? platform_get_irq()
already calls dev_err_probe() internally when it fails. Returning the error
code directly without another dev_err_probe() might be preferred.

[ ... ]

> @@ -258,6 +255,11 @@ static int ma35d1_rtc_probe(struct platform_device *pdev)
>  	if (ret)
>  		return dev_err_probe(&pdev->dev, ret, "Failed to register rtc device\n");
>  
> +	ret = devm_request_irq(&pdev->dev, rtc->irq_num, ma35d1_rtc_interrupt,
> +			       IRQF_NO_SUSPEND, "ma35d1rtc", rtc);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Failed to request rtc irq\n");
> +
>  	return 0;
>  }

[Severity: High]
Does placing devm_request_irq() after devm_rtc_register_device() introduce a
race condition?

Since devres teardown is last-in, first-out, during an unbind or a probe
failure, the IRQ would be freed before the RTC device is unregistered.
This leaves the device exposed to userspace without an active interrupt
handler, potentially leading to missed alarms or unhandled interrupt storms.

Would it be better to place devm_request_irq() between
devm_rtc_allocate_device() and devm_rtc_register_device() in
ma35d1_rtc_probe()?

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