[PATCH] rtc: pxa: fix IRQ leak on probe failure after pxa_rtc_open()

[PATCH] rtc: pxa: fix IRQ leak on probe failure after pxa_rtc_open()

[Cong Nguyen]

pxa_rtc_probe() calls pxa_rtc_open(dev), discarding its return value.
On success that requests both the 1Hz and alarm IRQs. If a later step
fails (sa1100_rtc_init() or devm_rtc_device_register()), probe returns
without releasing them -- and pxa_rtc_irq() then dereferences the
devm-freed pxa_rtc on any pending/spurious interrupt, plus a retry
fails with -EBUSY since the IRQs are never released. Not theoretical:
commit 34127b3632b2 ("rtc: pxa: fix null pointer dereference") documents
sa1100_rtc_init() failing this way on a real Zaurus SL-C1000.

Check pxa_rtc_open()'s return, and route both later failures through
pxa_rtc_release() via a new err_release label, mirroring the existing
remove() cleanup.

Fixes: 3cdf4ad9633e ("rtc: pxa: convert to use shared sa1100 functions")
Assisted-by: Claude:claude-opus-4
Signed-off-by: Cong Nguyen <congnt264@gmail.com>
---
 drivers/rtc/rtc-pxa.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-pxa.c b/drivers/rtc/rtc-pxa.c
index 62ee6b8f9bcd..e01e683c2f61 100644
--- a/drivers/rtc/rtc-pxa.c
+++ b/drivers/rtc/rtc-pxa.c
@@ -338,7 +338,9 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	pxa_rtc_open(dev);
+	ret = pxa_rtc_open(dev);
+	if (ret)
+		return ret;
 
 	sa1100_rtc->rcnr = pxa_rtc->base + 0x0;
 	sa1100_rtc->rtsr = pxa_rtc->base + 0x8;
@@ -347,7 +349,7 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
 	ret = sa1100_rtc_init(pdev, sa1100_rtc);
 	if (ret) {
 		dev_err(dev, "Unable to init SA1100 RTC sub-device\n");
-		return ret;
+		goto err_release;
 	}
 
 	rtsr_clear_bits(pxa_rtc, RTSR_PIALE | RTSR_RDALE1 | RTSR_HZE);
@@ -357,12 +359,16 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
 	if (IS_ERR(pxa_rtc->rtc)) {
 		ret = PTR_ERR(pxa_rtc->rtc);
 		dev_err(dev, "Failed to register RTC device -> %d\n", ret);
-		return ret;
+		goto err_release;
 	}
 
 	device_init_wakeup(dev, true);
 
 	return 0;
+
+err_release:
+	pxa_rtc_release(dev);
+	return ret;
 }
 
 static void __exit pxa_rtc_remove(struct platform_device *pdev)
-- 
2.25.1

Re: [PATCH] rtc: pxa: fix IRQ leak on probe failure after pxa_rtc_open()

[sashiko-bot]

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

Pre-existing issues:
- [High] The driver incorrectly registers two separate RTC class devices for the same hardware, leading to uncoordinated concurrent access and register corruption.
--

commit 0b0a0ee3975e585ff222c5362ed76732cdee7bd2
Author: Cong Nguyen <congnt264@gmail.com>

rtc: pxa: fix IRQ leak on probe failure after pxa_rtc_open()

This commit addresses an IRQ leak in pxa_rtc_probe() where the 1Hz and alarm
IRQs requested by pxa_rtc_open() are not released if sa1100_rtc_init() or
devm_rtc_device_register() fail later in the probe sequence. It routes
failures through a new err_release label to properly call pxa_rtc_release().

> diff --git a/drivers/rtc/rtc-pxa.c b/drivers/rtc/rtc-pxa.c
> index 62ee6b8f9bcd6..e01e683c2f615 100644
> --- a/drivers/rtc/rtc-pxa.c
> +++ b/drivers/rtc/rtc-pxa.c
[ ... ]
> @@ -347,7 +349,7 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
>  	ret = sa1100_rtc_init(pdev, sa1100_rtc);
>  	if (ret) {
>  		dev_err(dev, "Unable to init SA1100 RTC sub-device\n");
> -		return ret;
> +		goto err_release;
>  	}
>  
>  	rtsr_clear_bits(pxa_rtc, RTSR_PIALE | RTSR_RDALE1 | RTSR_HZE);
> @@ -357,12 +359,16 @@ static int __init pxa_rtc_probe(struct platform_device *pdev)
>  	if (IS_ERR(pxa_rtc->rtc)) {

[Severity: High]
This is a pre-existing issue, but does this lead to uncoordinated concurrent
access and register corruption?

Looking at drivers/rtc/rtc-pxa.c:pxa_rtc_probe(), the code first delegates
to sa1100_rtc_init(), which registers a primary RTC class device and
initializes its own spinlock (info->lock).

Then pxa_rtc_probe() registers a second concurrent RTC class device
(pxa_rtc->rtc) with a completely independent spinlock (pxa_rtc->lock) for
the exact same hardware registers.

Concurrent userspace interactions (via ioctls on /dev/rtc0 and /dev/rtc1)
will race during read-modify-write operations on the hardware. Could this
cause data corruption of shared RTC hardware registers (like RTSR),
inconsistent state, and incorrect interrupt routing since events only
update one of the devices?

>  		ret = PTR_ERR(pxa_rtc->rtc);
>  		dev_err(dev, "Failed to register RTC device -> %d\n", ret);
> -		return ret;
> +		goto err_release;
>  	}
>  
>  	device_init_wakeup(dev, true);
>  
>  	return 0;
> +
> +err_release:
> +	pxa_rtc_release(dev);
> +	return ret;
>  }

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