[PATCH 1/2] rtc: m48t59: Propagate errors from optional IRQ lookup

[PATCH 1/2] rtc: m48t59: Propagate errors from optional IRQ lookup

[phucduc.bui]

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, so that failures are properly reported to the caller.
Preserve NO_IRQ for the case where no optional IRQ is available.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/rtc/rtc-m48t59.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
index 4e608bc8bbd3..1150f3f6b008 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;
 
-	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;
+	} else {
+		m48t59->irq = NO_IRQ;
 	}
 
 	m48t59->rtc = devm_rtc_allocate_device(&pdev->dev);
-- 
2.43.0

[PATCH 2/2] rtc: cmos: Propagate errors from optional IRQ lookup

[phucduc.bui]

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

Re: [PATCH 2/2] rtc: cmos: Propagate errors from optional IRQ lookup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Propagating -EPROBE_DEFER from platform_get_irq_optional() causes a permanent device probe failure because rtc-cmos is registered using platform_driver_probe(), which explicitly forbids deferred probing.
- [High] Propagating parsing errors like -EINVAL bypasses the x86 legacy IRQ fallback, breaking RTC probing on systems with malformed ACPI tables.
--

--- Patch [2]: [PATCH 2/2] rtc: cmos: Propagate errors from optional IRQ lookup ---
commit bf54cacd6dce9cc731cd51fdbcbf8b84af0b1e7f
Author: bui duc phuc <phucduc.bui@gmail.com>

rtc: cmos: Propagate errors from optional IRQ lookup

The commit updates the RTC CMOS driver to propagate error codes returned by
platform_get_irq_optional() other than -ENXIO, while preserving -1 for the
case where no optional IRQ is available.

> diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
> index fa04ece151b8b..0a2c7383c8cd4 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;

[Severity: High]
Does propagating -EPROBE_DEFER from platform_get_irq_optional() cause a
permanent device probe failure?

Since rtc-cmos is registered using platform_driver_probe(), which explicitly
forbids deferred probing, returning -EPROBE_DEFER here might cause the driver
core to convert it to -ENXIO and permanently fail the probe instead of
proceeding without an optional IRQ.

[Severity: High]
Does propagating parsing errors like -EINVAL bypass the x86 legacy IRQ
fallback?

On x86 systems with malformed ACPI tables (e.g., specifying IRQ 0, which
platform_get_irq_optional() rejects with -EINVAL), propagating the error
bypasses the fallback logic below that successfully hardcodes the IRQ to
RTC_IRQ if legacy PIC IRQs are supported, potentially breaking RTC probing
on those systems.

> +	if (irq == -ENXIO) {
>  		irq = -1;
>  #ifdef CONFIG_X86
>  		/*

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

Re: [PATCH 1/2] rtc: m48t59: Propagate errors from optional IRQ lookup

[sashiko-bot]

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