* [PATCH 2/2] rtc: cmos: Propagate errors from optional IRQ lookup
2026-08-10 5:15 [PATCH 1/2] rtc: m48t59: Propagate errors from optional IRQ lookup phucduc.bui
@ 2026-08-10 5:15 ` phucduc.bui
2026-08-10 5:30 ` sashiko-bot
2026-08-10 5:29 ` [PATCH 1/2] rtc: m48t59: " sashiko-bot
1 sibling, 1 reply; 4+ messages in thread
From: phucduc.bui @ 2026-08-10 5:15 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc, linux-kernel, bui duc phuc
From: bui duc phuc <phucduc.bui@gmail.com>
platform_get_irq_optional() returns a positive IRQ number on success or
a negative error code on failure. For an optional IRQ, -ENXIO indicates
that no optional IRQ is available, while other errors should be propagated.
Propagate all error codes returned by platform_get_irq_optional() other
than -ENXIO, while preserving -1 for the case where no optional IRQ is
available.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
drivers/rtc/rtc-cmos.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index fa04ece151b8..0a2c7383c8cd 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -1436,7 +1436,9 @@ static int __init cmos_platform_probe(struct platform_device *pdev)
else
resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
irq = platform_get_irq_optional(pdev, 0);
- if (irq < 0) {
+ if (irq < 0 && irq != -ENXIO)
+ return irq;
+ if (irq == -ENXIO) {
irq = -1;
#ifdef CONFIG_X86
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/2] rtc: m48t59: Propagate errors from optional IRQ lookup
2026-08-10 5:15 [PATCH 1/2] rtc: m48t59: Propagate errors from optional IRQ lookup phucduc.bui
2026-08-10 5:15 ` [PATCH 2/2] rtc: cmos: " phucduc.bui
@ 2026-08-10 5:29 ` sashiko-bot
1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-10 5:29 UTC (permalink / raw)
To: phucduc.bui; +Cc: linux-rtc, Alexandre Belloni
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Improper use of devm_kzalloc for pdev->dev.platform_data leads to Use-After-Free on deferred probe.
- [High] The shared IRQ handler is registered before its required context (drvdata, spinlock) is initialized, risking a NULL pointer dereference.
--
--- Patch [1]: [PATCH 1/2] rtc: m48t59: Propagate errors from optional IRQ lookup ---
commit 9d48ac67a0a6bc0aac19eba1f8852bc6afa6e0da
Author: bui duc phuc <phucduc.bui@gmail.com>
rtc: m48t59: Propagate errors from optional IRQ lookup
This commit updates m48t59_rtc_probe() to propagate error codes other than
-ENXIO from platform_get_irq_optional(). Previously, these errors were ignored
and the driver fell back to polling mode.
> diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
> index 4e608bc8bbd36..1150f3f6b0084 100644
> --- a/drivers/rtc/rtc-m48t59.c
> +++ b/drivers/rtc/rtc-m48t59.c
> @@ -404,15 +404,17 @@ static int m48t59_rtc_probe(struct platform_device *pdev)
> * the mode without IRQ.
> */
> m48t59->irq = platform_get_irq_optional(pdev, 0);
> - if (m48t59->irq <= 0)
> - m48t59->irq = NO_IRQ;
> + if (m48t59->irq < 0 && m48t59->irq != -ENXIO)
> + return m48t59->irq;
[Severity: High]
This is a pre-existing issue, but this new error return path exposes it: if
this returns -EPROBE_DEFER, will it lead to a use-after-free on the next probe
attempt?
Earlier in m48t59_rtc_probe(), if the device is memory-mapped and platform
data is not provided, pdata is allocated with devm_kzalloc() and assigned to
the device:
drivers/rtc/rtc-m48t59.c:m48t59_rtc_probe() {
...
pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
return -ENOMEM;
/* Ensure we only kmalloc platform data once */
pdev->dev.platform_data = pdata;
...
}
When the probe fails and returns -EPROBE_DEFER here, devres will free the
pdata allocation. However, pdev->dev.platform_data is not cleared. On the next
probe attempt, dev_get_platdata() will return this dangling pointer, bypassing
reallocation and causing the driver to dereference freed memory.
>
> - if (m48t59->irq != NO_IRQ) {
> + if (m48t59->irq > 0) {
> ret = devm_request_irq(&pdev->dev, m48t59->irq,
> m48t59_rtc_interrupt, IRQF_SHARED,
> "rtc-m48t59", &pdev->dev);
> if (ret)
> return ret;
[Severity: High]
This isn't a bug introduced by this patch, but since we are modifying the IRQ
setup, is it safe to request a shared interrupt here before the driver data
and spinlocks are initialized?
If another device shares this interrupt line and triggers an interrupt
immediately after devm_request_irq(), the handler m48t59_rtc_interrupt() will
execute before m48t59_rtc_probe() has finished setting up:
drivers/rtc/rtc-m48t59.c:m48t59_rtc_probe() {
...
spin_lock_init(&m48t59->lock);
platform_set_drvdata(pdev, m48t59);
...
}
Because platform_set_drvdata() hasn't been called yet, dev_get_drvdata() in
the interrupt handler will return null, resulting in a null pointer
dereference when it tries to lock it:
drivers/rtc/rtc-m48t59.c:m48t59_rtc_interrupt() {
struct m48t59_private *m48t59 = dev_get_drvdata(dev);
...
spin_lock(&m48t59->lock);
...
}
Could this cause a crash on platforms with a shared interrupt?
> + } else {
> + m48t59->irq = NO_IRQ;
> }
>
> m48t59->rtc = devm_rtc_allocate_device(&pdev->dev);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260810051504.26912-1-phucduc.bui@gmail.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread