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

[sashiko-bot@kernel.org]

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