Re: [PATCH v1 1/3] rtc: ftrtc010: fix clock resource leak on probe failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The interrupt handler unconditionally returns IRQ_HANDLED in a shared IRQ setup, defeating spurious interrupt detection.
- [High] Sequential reads of distinct RTC hardware counters lack software rollover protection, risking time corruption.
--

commit 6944a6b7d4e12941b5612ab5907724e5485e82a6
Author: Liu Dalin <liudalin@kylinsec.com.cn>

rtc: ftrtc010: fix clock resource leak on probe failure

This commit addresses a resource leak in ftrtc010_rtc_probe() that occurs
when devm_rtc_register_device() fails. It captures the return value of the
registration function and jumps to an error handling path. This ensures
that the clocks obtained via clk_prepare_enable() are properly released.

> diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
> index 02608d3784958..f398d6dc85ae4 100644
> --- a/drivers/rtc/rtc-ftrtc010.c
> +++ b/drivers/rtc/rtc-ftrtc010.c
> @@ -181,7 +181,11 @@ static int ftrtc010_rtc_probe(struct platform_device *pdev)
>  	if (unlikely(ret))
>  		goto err_disable_extclk;
>  
> -	return devm_rtc_register_device(rtc_dev);
> +	ret = devm_rtc_register_device(rtc_dev);
> +	if (ret)
> +		goto err_disable_extclk;
> +
> +	return 0;
>  
>  err_disable_extclk:
>  	clk_disable_unprepare(rtc->extclk);

[Severity: High]
This is a pre-existing issue, but I noticed the interrupt handler
unconditionally returns IRQ_HANDLED:

drivers/rtc/rtc-ftrtc010.c:ftrtc010_rtc_interrupt() {
	return IRQ_HANDLED;
}

Since this interrupt is requested as a shared line just prior to device
registration in ftrtc010_rtc_probe():

	ret = devm_request_irq(dev, rtc->rtc_irq, ftrtc010_rtc_interrupt,
			       IRQF_SHARED, pdev->name, dev);

Could this defeat spurious interrupt detection? If another device on the same
shared IRQ line asserts an interrupt but its handler fails or is not yet
registered, this driver will falsely claim to have handled it. This might
prevent the spurious interrupt detector from disabling the faulty line and
could result in an unrecoverable interrupt storm.

[Severity: High]
This is a pre-existing issue, but does ftrtc010_rtc_read_time() lack software
rollover protection?

drivers/rtc/rtc-ftrtc010.c:ftrtc010_rtc_read_time() {
    ...
	sec  = readl(rtc->rtc_base + FTRTC010_RTC_SECOND);
	min  = readl(rtc->rtc_base + FTRTC010_RTC_MINUTE);
	hour = readl(rtc->rtc_base + FTRTC010_RTC_HOUR);
	days = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
    ...
}

If a hardware tick occurs asynchronously between these distinct readl() calls,
could it mix pre-tick and post-tick units and result in corrupted time? Since
there is no hardware latching indicated and no software retry loop to verify
consistency, an asynchronous tick might cause an incorrect aggregated time to
be passed to the RTC subsystem.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827063019.3342085-1-liudalin@kylinsec.com.cn?part=1