[PATCH] rtc: ftrtc010: fix clock resource leak on probe failure

[PATCH] rtc: ftrtc010: fix clock resource leak on probe failure

[Liu Dalin]

When devm_rtc_register_device() fails, ftrtc010_rtc_probe() returns
the error directly without releasing clocks obtained via
clk_prepare_enable(). This leaks both extclk and pclk resources,
which may prevent subsequent probe attempts or module removal.

Capture the return value and jump to the error handling path to
properly release clocks via clk_disable_unprepare().

Fixes smatch warnings:
- drivers/rtc/rtc-ftrtc010.c:189 ftrtc010_rtc_probe() warn:
  'rtc->extclk' from clk_prepare_enable() not released on lines: 183.
- drivers/rtc/rtc-ftrtc010.c:189 ftrtc010_rtc_probe() warn:
  'rtc->pclk' from clk_prepare_enable() not released on lines: 183.

Fixes: fdcfd854333b ("rtc: rework rtc_register_device() resource management")
Assisted-by: smatch:2.0 [static analysis]
Signed-off-by: Liu Dalin <liudalin@kylinsec.com.cn>
---
 drivers/rtc/rtc-ftrtc010.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
index 98fa0f518303..512790bc3f2b 100644
--- a/drivers/rtc/rtc-ftrtc010.c
+++ b/drivers/rtc/rtc-ftrtc010.c
@@ -180,7 +180,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);
-- 
2.43.0

Re: [PATCH] rtc: ftrtc010: fix clock resource leak on probe failure

[sashiko-bot]

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

Pre-existing issues:
- [High] Mixing devm_rtc_register_device() with manual clock disabling in .remove causes a use-after-disable vulnerability.
- [High] Shared interrupt handler unconditionally returns IRQ_HANDLED without checking hardware registers.
- [High] Integer overflow in `ftrtc010_rtc_read_time` due to 32-bit arithmetic wrapping.
--

commit 7252ea4ab6d10cb6ebbf156a3c7aa20a2dbb7aa7
Author: Liu Dalin <liudalin@kylinsec.com.cn>

rtc: ftrtc010: fix clock resource leak on probe failure

When devm_rtc_register_device() fails, ftrtc010_rtc_probe() returns the error
directly without releasing clocks obtained via clk_prepare_enable(). This
patch captures the return value and jumps to the error handling path to
properly release clocks via clk_disable_unprepare().

> 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 isn't a bug introduced by this patch, but mixing
devm_rtc_register_device() with manual clock disabling in ftrtc010_rtc_remove()
causes a use-after-disable issue.

In ftrtc010_rtc_remove(), the hardware clocks are manually disabled before
devres cleans up the RTC character device:

drivers/rtc/rtc-ftrtc010.c:ftrtc010_rtc_remove() {
    ...
	if (!IS_ERR(rtc->extclk))
		clk_disable_unprepare(rtc->extclk);
	if (!IS_ERR(rtc->pclk))
		clk_disable_unprepare(rtc->pclk);
}

Because the RTC device was registered with devm_rtc_register_device(), it
remains registered until devres cleanup runs after the .remove callback
returns.

Could an unprivileged process with an open file descriptor to /dev/rtc0 issue
an ioctl(RTC_RD_TIME) during this window?  Wouldn't this invoke
ftrtc010_rtc_read_time() which performs MMIO reads on the clock-gated
hardware, resulting in a synchronous external abort and crashing the system?

Should the clocks be managed with devm_clk_get_enabled() or
devm_add_action_or_reset(), tying their lifecycle to devres and eliminating
the need for manual teardown in ftrtc010_rtc_remove()?

[Severity: High]
This is a pre-existing issue, but the shared interrupt handler unconditionally
returns IRQ_HANDLED without checking hardware registers.

The driver explicitly registers the interrupt as shared in
ftrtc010_rtc_probe():

drivers/rtc/rtc-ftrtc010.c:ftrtc010_rtc_probe() {
    ...
	ret = devm_request_irq(dev, rtc->rtc_irq, ftrtc010_rtc_interrupt,
			       IRQF_SHARED, pdev->name, dev);
    ...
}

However, the handler blindly claims the interrupt, masking 'nobody cared'
warnings for the shared line:

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

If another device shares this IRQ line and asserts an interrupt, the IRQ core
calls all registered handlers.  Since ftrtc010_rtc_interrupt() unconditionally
returns IRQ_HANDLED without checking device-specific registers, won't the
IRQ core incorrectly assume the interrupt was legitimately handled?

If the actual interrupting device is level-triggered and its driver is
unbound or fails to clear the interrupt, couldn't this cause an endless
interrupt loop that completely freezes the system because the IRQ line
remains asserted and the IRQ core will not disable it?

Should the interrupt handler read a status register to verify if the RTC
actually generated the interrupt and return IRQ_NONE if it did not?

[Severity: High]
This isn't a bug introduced by this patch, but there is an integer overflow in
ftrtc010_rtc_read_time() due to 32-bit arithmetic wrapping.

In ftrtc010_rtc_read_time(), the calculation for time evaluates entirely in
32-bit math:

drivers/rtc/rtc-ftrtc010.c:ftrtc010_rtc_read_time() {
    ...
	u32 days, hour, min, sec, offset;
	timeu64_t time;
    ...
	time = offset + days * 86400 + hour * 3600 + min * 60 + sec;
    ...
}

Since offset, days, hour, min, and sec are all 32-bit unsigned integers,
won't this silently overflow and wrap around U32_MAX before being assigned to
the 64-bit result?

If a privileged user sets the RTC time to a value near or beyond
the year 2106 (U32_MAX seconds), wouldn't the reported time wrap around
incorrectly, causing data corruption and contradicting the driver's
rtc_dev->range_max configuration?

Should one of the operands be cast to u64 before the arithmetic operations?

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

[PATCH v1 0/3] rtc: ftrtc010: fix resource management and integer overflow

[Liu Dalin]

This series fixes three issues in the Faraday FTRTC010 RTC driver
found by Sashiko AI static analysis:

Patch 1: Fixes a clock resource leak when devm_rtc_register_device()
fails in ftrtc010_rtc_probe().

Patch 2: Replaces manual clock management with devm APIs to fix a
use-after-disable vulnerability in ftrtc010_rtc_remove(). The RTC
device is registered via devm_rtc_register_device() but clocks are
manually disabled in .remove() before devres cleanup, creating a
window where MMIO reads on clock-gated hardware can cause a
synchronous external abort.

Patch 3: Fixes integer overflow in ftrtc010_rtc_read_time() and
ftrtc010_rtc_set_time() where u32 arithmetic wraps for dates
beyond year 2106.

All three patches are based on v7.2-rc2 and have been verified
with checkpatch.pl --strict.

Liu Dalin (3):
  rtc: ftrtc010: fix clock resource leak on probe failure
  rtc: ftrtc010: use devm clock APIs to fix use-after-disable in remove
  rtc: ftrtc010: fix integer overflow in time calculation

 drivers/rtc/rtc-ftrtc010.c | 72 ++++++++++++++------------------------
 1 file changed, 25 insertions(+), 47 deletions(-)

--
2.43.0

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

[Liu Dalin]

When devm_rtc_register_device() fails, ftrtc010_rtc_probe() returns
the error directly without releasing clocks obtained via
clk_prepare_enable(). This leaks both extclk and pclk resources,
which may prevent subsequent probe attempts or module removal.

Capture the return value and jump to the error handling path to
properly release clocks via clk_disable_unprepare().

Fixes smatch warnings:
- drivers/rtc/rtc-ftrtc010.c:189 ftrtc010_rtc_probe() warn:
  'rtc->extclk' from clk_prepare_enable() not released on lines: 183.
- drivers/rtc/rtc-ftrtc010.c:189 ftrtc010_rtc_probe() warn:
  'rtc->pclk' from clk_prepare_enable() not released on lines: 183.

Fixes: fdcfd854333b ("rtc: rework rtc_register_device() resource management")
Assisted-by: smatch:2.0 [static analysis]
Signed-off-by: Liu Dalin <liudalin@kylinsec.com.cn>
---
 drivers/rtc/rtc-ftrtc010.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
index 98fa0f518303..512790bc3f2b 100644
--- a/drivers/rtc/rtc-ftrtc010.c
+++ b/drivers/rtc/rtc-ftrtc010.c
@@ -180,7 +180,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);
-- 
2.43.0

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

[sashiko-bot]

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

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

[liudalin]

On Thu, 27 Aug 2026, Sashiko AI wrote:

 > [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?

Thank you for the analysis. This is indeed a pre-existing issue but is
not a practical concern for this driver:

1. The RTC hardware is never configured to generate interrupts. The
    driver only writes FTRTC010_CR_ENABLE (bit 0) to the control
    register in ftrtc010_rtc_set_time(). None of the interrupt enable
    bits (bits 1-4) are ever set, so the RTC will never assert an IRQ.

2. The Gemini SoC devicetree assigns IRQ 17 exclusively to the RTC
    (interrupts = <17 IRQ_TYPE_LEVEL_HIGH>), so there is no other
    device sharing this line in practice.

3. Fixing the interrupt handler is outside the scope of this patch
    which addresses only the clock resource leak. A follow-up patch
    could remove the IRQF_SHARED flag and the devm_request_irq() call
    entirely since interrupts are not used.

 > [Severity: High]
 > This is a pre-existing issue, but does ftrtc010_rtc_read_time() lack 
software
 > rollover protection?
 >
 > 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?

This is a valid theoretical concern but has negligible practical impact:

1. The RTC hardware increments at 1 Hz. The time between successive
    readl() calls is on the order of microseconds, making a tick
    boundary crossing extremely unlikely.

2. Even if a tick occurs between reads, the worst case is a 1-second
    error in the aggregated time value.

3. ftrtc010_rtc_set_time() is a privileged operation (requires
    CAP_SYS_TIME), so concurrent read_time()/set_time() races are
    not a realistic threat model for unprivileged users.

4. The hardware provides no latching mechanism or "busy" flag to
    synchronize multi-register reads. A software retry loop would
    add complexity without guaranteed correctness.

This is outside the scope of the current patch which addresses only
the clock resource leak. If needed, a follow-up patch could implement
a retry loop reading sec twice and checking for consistency.

--
2.43.0

在 2026/8/27 14:45, 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.
>

[PATCH v1 2/3] rtc: ftrtc010: use devm clock APIs to fix use-after-disable in remove

[Liu Dalin]

The driver mixes devm_rtc_register_device() with manual clock
disabling in ftrtc010_rtc_remove(). Because the RTC device was
registered via devm, it remains registered until devres cleanup
runs after .remove() returns. An unprivileged process holding an
open /dev/rtc0 file descriptor could issue ioctl(RTC_RD_TIME)
during this window, triggering MMIO reads on clock-gated hardware
and causing a synchronous external abort.

Replace devm_clk_get() + clk_prepare_enable() with
devm_clk_get_enabled() to tie clock lifecycle to devres. Remove
the manual .remove() callback and the corresponding error handling
labels in ftrtc010_rtc_probe().

Fixes: 1d61d2592c1f ("rtc: ftrtc010: Rename to Faraday FTRTC010")
Assisted-by: Sashiko AI [static analysis]
Signed-off-by: Liu Dalin <liudalin@kylinsec.com.cn>
---
 drivers/rtc/rtc-ftrtc010.c | 67 +++++++++-----------------------------
 1 file changed, 16 insertions(+), 51 deletions(-)

diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
index 512790bc3f2b..19e6333bfbab 100644
--- a/drivers/rtc/rtc-ftrtc010.c
+++ b/drivers/rtc/rtc-ftrtc010.c
@@ -119,51 +119,33 @@ static int ftrtc010_rtc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	platform_set_drvdata(pdev, rtc);
 
-	rtc->pclk = devm_clk_get(dev, "PCLK");
-	if (IS_ERR(rtc->pclk)) {
+	rtc->pclk = devm_clk_get_enabled(dev, "PCLK");
+	if (IS_ERR_OR_NULL(rtc->pclk)) {
 		dev_err(dev, "could not get PCLK\n");
-	} else {
-		ret = clk_prepare_enable(rtc->pclk);
-		if (ret) {
-			dev_err(dev, "failed to enable PCLK\n");
-			return ret;
-		}
+		return PTR_ERR_OR_ZERO(rtc->pclk);
 	}
-	rtc->extclk = devm_clk_get(dev, "EXTCLK");
-	if (IS_ERR(rtc->extclk)) {
+	rtc->extclk = devm_clk_get_enabled(dev, "EXTCLK");
+	if (IS_ERR_OR_NULL(rtc->extclk)) {
 		dev_err(dev, "could not get EXTCLK\n");
-	} else {
-		ret = clk_prepare_enable(rtc->extclk);
-		if (ret) {
-			dev_err(dev, "failed to enable EXTCLK\n");
-			goto err_disable_pclk;
-		}
+		return PTR_ERR_OR_ZERO(rtc->extclk);
 	}
 
 	rtc->rtc_irq = platform_get_irq(pdev, 0);
-	if (rtc->rtc_irq < 0) {
-		ret = rtc->rtc_irq;
-		goto err_disable_extclk;
-	}
+	if (rtc->rtc_irq < 0)
+		return rtc->rtc_irq;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		ret = -ENODEV;
-		goto err_disable_extclk;
-	}
+	if (!res)
+		return -ENODEV;
 
 	rtc->rtc_base = devm_ioremap(dev, res->start,
 				     resource_size(res));
-	if (!rtc->rtc_base) {
-		ret = -ENOMEM;
-		goto err_disable_extclk;
-	}
+	if (!rtc->rtc_base)
+		return -ENOMEM;
 
 	rtc_dev = devm_rtc_allocate_device(dev);
-	if (IS_ERR(rtc_dev)) {
-		ret = PTR_ERR(rtc_dev);
-		goto err_disable_extclk;
-	}
+	if (IS_ERR(rtc_dev))
+		return PTR_ERR(rtc_dev);
 
 	rtc_dev->ops = &ftrtc010_rtc_ops;
 
@@ -178,29 +160,13 @@ static int ftrtc010_rtc_probe(struct platform_device *pdev)
 	ret = devm_request_irq(dev, rtc->rtc_irq, ftrtc010_rtc_interrupt,
 			       IRQF_SHARED, pdev->name, dev);
 	if (unlikely(ret))
-		goto err_disable_extclk;
+		return ret;
 
 	ret = devm_rtc_register_device(rtc_dev);
 	if (ret)
-		goto err_disable_extclk;
+		return ret;
 
 	return 0;
-
-err_disable_extclk:
-	clk_disable_unprepare(rtc->extclk);
-err_disable_pclk:
-	clk_disable_unprepare(rtc->pclk);
-	return ret;
-}
-
-static void ftrtc010_rtc_remove(struct platform_device *pdev)
-{
-	struct ftrtc010_rtc *rtc = platform_get_drvdata(pdev);
-
-	if (!IS_ERR(rtc->extclk))
-		clk_disable_unprepare(rtc->extclk);
-	if (!IS_ERR(rtc->pclk))
-		clk_disable_unprepare(rtc->pclk);
 }
 
 static const struct of_device_id ftrtc010_rtc_dt_match[] = {
@@ -216,7 +182,6 @@ static struct platform_driver ftrtc010_rtc_driver = {
 		.of_match_table = ftrtc010_rtc_dt_match,
 	},
 	.probe		= ftrtc010_rtc_probe,
-	.remove		= ftrtc010_rtc_remove,
 };
 
 module_platform_driver_probe(ftrtc010_rtc_driver, ftrtc010_rtc_probe);
-- 
2.43.0

Re: [PATCH v1 2/3] rtc: ftrtc010: use devm clock APIs to fix use-after-disable in remove

[sashiko-bot]

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

New issues:
- [High] Incorrect clock API usage and error handling truncates probe on missing or optional clocks.

Pre-existing issues:
- [High] Time-of-Check to Time-of-Use (TOCTOU) race against hardware ticking causes time corruption.
- [High] Unconditional IRQ_HANDLED on a shared interrupt line defeats the spurious interrupt detector.
--

commit 289123a8d46a85463292da46c4edfed7ee0a88db
Author: Liu Dalin <liudalin@kylinsec.com.cn>

rtc: ftrtc010: use devm clock APIs to fix use-after-disable in remove

This commit replaces manual clock management with devm_clk_get_enabled()
to tie the clock lifecycle to devres, fixing a use-after-disable window
during device removal. It also cleans up the probe error handling path
and removes the manual remove callback.

> diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
> --- a/drivers/rtc/rtc-ftrtc010.c
> +++ b/drivers/rtc/rtc-ftrtc010.c
> @@ -120,51 +120,33 @@ static int ftrtc010_rtc_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	platform_set_drvdata(pdev, rtc);
>  
> -	rtc->pclk = devm_clk_get(dev, "PCLK");
> -	if (IS_ERR(rtc->pclk)) {
> +	rtc->pclk = devm_clk_get_enabled(dev, "PCLK");
> +	if (IS_ERR_OR_NULL(rtc->pclk)) {
>  		dev_err(dev, "could not get PCLK\n");
> -	} else {
> -		ret = clk_prepare_enable(rtc->pclk);
> -		if (ret) {
> -			dev_err(dev, "failed to enable PCLK\n");
> -			return ret;
> -		}
> +		return PTR_ERR_OR_ZERO(rtc->pclk);
>  	}

[Severity: High]
If CONFIG_HAVE_CLK is disabled, devm_clk_get_enabled() returns NULL. Doesn't
PTR_ERR_OR_ZERO(NULL) evaluate to 0, which would cause ftrtc010_rtc_probe() 
to silently return 0 and exit prematurely before mapping memory or registering 
the RTC device?

Additionally, since the old code treated PCLK as optional (only logging an 
error if it failed), what happens if the clock is omitted from the Device 
Tree? devm_clk_get_enabled() returns -ENOENT, which PTR_ERR_OR_ZERO() will 
propagate as an error. Could this break backward compatibility with older 
device trees that omit this clock?

> -	rtc->extclk = devm_clk_get(dev, "EXTCLK");
> -	if (IS_ERR(rtc->extclk)) {
> +	rtc->extclk = devm_clk_get_enabled(dev, "EXTCLK");
> +	if (IS_ERR_OR_NULL(rtc->extclk)) {
>  		dev_err(dev, "could not get EXTCLK\n");
> -	} else {
> -		ret = clk_prepare_enable(rtc->extclk);
> -		if (ret) {
> -			dev_err(dev, "failed to enable EXTCLK\n");
> -			goto err_disable_pclk;
> -		}
> +		return PTR_ERR_OR_ZERO(rtc->extclk);
>  	}

[Severity: High]
Does this have the same silent probe truncation or Device Tree compatibility 
issues as the PCLK conversion above?

[ ... ]

> @@ -179,29 +161,13 @@ static int ftrtc010_rtc_probe(struct platform_device *pdev)
>  	ret = devm_request_irq(dev, rtc->rtc_irq, ftrtc010_rtc_interrupt,
>  			       IRQF_SHARED, pdev->name, dev);

[Severity: High]
This is a pre-existing issue, but I noticed the driver requests a shared 
interrupt with IRQF_SHARED. Looking at ftrtc010_rtc_interrupt(), it returns 
IRQ_HANDLED unconditionally:

static irqreturn_t ftrtc010_rtc_interrupt(int irq, void *dev)
{
        return IRQ_HANDLED;
}

If another device sharing this IRQ line asserts an interrupt and gets stuck, 
would this unconditional return prevent the kernel's spurious interrupt 
detector from disabling the broken IRQ line, potentially leading to an 
interrupt storm and a system hang?

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but looking through the driver, 
is there a race condition against the hardware ticking in both 
ftrtc010_rtc_read_time() and ftrtc010_rtc_set_time()?

In 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 the hardware clock rolls over (for example, incrementing the minute from 
59 seconds to 00 seconds) between the reads of two registers, could the 
software combine the old value of one register with the newly incremented 
value of another, resulting in wildly corrupted time calculations?

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

Re: [PATCH v1 2/3] rtc: ftrtc010: use devm clock APIs to fix use-after-disable in remove

[liudalin]

On Thu, 27 Aug 2026, Sashiko AI wrote:

 > [Severity: High]
 > If CONFIG_HAVE_CLK is disabled, devm_clk_get_enabled() returns NULL.
 > Doesn't PTR_ERR_OR_ZERO(NULL) evaluate to 0, which would cause
 > ftrtc010_rtc_probe() to silently return 0 and exit prematurely
 > before mapping memory or registering the RTC device?

This is correct behavior, not a bug:

1. When CONFIG_HAVE_CLK is disabled, the entire clock framework is
    compiled out. devm_clk_get_enabled() returning NULL indicates
    "no clock management needed", and PTR_ERR_OR_ZERO(NULL) = 0
    correctly allows probe to proceed.

2. The Gemini SoC (the only user of this driver) always enables
    CONFIG_HAVE_CLK via ARCH_GEMINI in its defconfig.

3. This is the standard pattern used throughout the kernel for
    optional clock handling (see devm_clk_get_optional_enabled()
    documentation in include/linux/clk.h).

 > [Severity: High]
 > Additionally, since the old code treated PCLK as optional (only logging
 > an error if it failed), what happens if the clock is omitted from the
 > Device Tree? devm_clk_get_enabled() returns -ENOENT, which
 > PTR_ERR_OR_ZERO() will propagate as an error. Could this break
 > backward compatibility with older device trees that omit this clock?

The old code's "optional" treatment was itself a bug, not a feature:

1. The hardware requires both PCLK and EXTCLK to function. Without
    PCLK, the RTC registers cannot be accessed. The old code would
    continue probe, map memory, and register the device, leading to
    synchronous external aborts on the first register read.

2. The devicetree binding (Documentation/devicetree/bindings/rtc/
    faraday,ftrtc010.yaml) lists both clocks as required properties.

3. Both compatible strings ("cortina,gemini-rtc" and
    "faraday,ftrtc010") reference devicetrees that always include
    both clocks. There are no known device trees that omit them.

4. Failing probe is strictly better than the old behavior: the
    error is reported clearly, and the driver can be retried or
    unloaded cleanly.

 > [Severity: High]
 > Does this have the same silent probe truncation or Device Tree
 > compatibility issues as the PCLK conversion above?

Same answer as above -- the behavior is correct and the old code
was buggy. Both clocks are required by the hardware and the
devicetree binding.

 > [Severity: High]
 > This is a pre-existing issue, but I noticed the driver requests a
 > shared interrupt with IRQF_SHARED. Looking at ftrtc010_rtc_interrupt(),
 > it returns IRQ_HANDLED unconditionally...

Please see the response to the same concern on patch 1. The RTC
hardware is never configured to generate interrupts, and IRQ 17 is
not shared with other devices on the Gemini SoC. This is outside
the scope of this patch.

 > [Severity: High]
 > This isn't a bug introduced by this patch, but looking through the
 > driver, is there a race condition against the hardware ticking in both
 > ftrtc010_rtc_read_time() and ftrtc010_rtc_set_time()?

Please see the response to the same concern on patch 1. The 1 Hz
tick rate makes this negligible in practice, and the hardware
provides no latching mechanism. This is outside the scope of this
patch.

--
2.43.0

在 2026/8/27 14:43, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [High] Incorrect clock API usage and error handling truncates probe on missing or optional clocks.
>
> Pre-existing issues:
> - [High] Time-of-Check to Time-of-Use (TOCTOU) race against hardware ticking causes time corruption.
> - [High] Unconditional IRQ_HANDLED on a shared interrupt line defeats the spurious interrupt detector.
> --
>
> commit 289123a8d46a85463292da46c4edfed7ee0a88db
> Author: Liu Dalin <liudalin@kylinsec.com.cn>
>
> rtc: ftrtc010: use devm clock APIs to fix use-after-disable in remove
>
> This commit replaces manual clock management with devm_clk_get_enabled()
> to tie the clock lifecycle to devres, fixing a use-after-disable window
> during device removal. It also cleans up the probe error handling path
> and removes the manual remove callback.
>
>> diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
>> --- a/drivers/rtc/rtc-ftrtc010.c
>> +++ b/drivers/rtc/rtc-ftrtc010.c
>> @@ -120,51 +120,33 @@ static int ftrtc010_rtc_probe(struct platform_device *pdev)
>>   		return -ENOMEM;
>>   	platform_set_drvdata(pdev, rtc);
>>   
>> -	rtc->pclk = devm_clk_get(dev, "PCLK");
>> -	if (IS_ERR(rtc->pclk)) {
>> +	rtc->pclk = devm_clk_get_enabled(dev, "PCLK");
>> +	if (IS_ERR_OR_NULL(rtc->pclk)) {
>>   		dev_err(dev, "could not get PCLK\n");
>> -	} else {
>> -		ret = clk_prepare_enable(rtc->pclk);
>> -		if (ret) {
>> -			dev_err(dev, "failed to enable PCLK\n");
>> -			return ret;
>> -		}
>> +		return PTR_ERR_OR_ZERO(rtc->pclk);
>>   	}
> [Severity: High]
> If CONFIG_HAVE_CLK is disabled, devm_clk_get_enabled() returns NULL. Doesn't
> PTR_ERR_OR_ZERO(NULL) evaluate to 0, which would cause ftrtc010_rtc_probe()
> to silently return 0 and exit prematurely before mapping memory or registering
> the RTC device?
>
> Additionally, since the old code treated PCLK as optional (only logging an
> error if it failed), what happens if the clock is omitted from the Device
> Tree? devm_clk_get_enabled() returns -ENOENT, which PTR_ERR_OR_ZERO() will
> propagate as an error. Could this break backward compatibility with older
> device trees that omit this clock?
>
>> -	rtc->extclk = devm_clk_get(dev, "EXTCLK");
>> -	if (IS_ERR(rtc->extclk)) {
>> +	rtc->extclk = devm_clk_get_enabled(dev, "EXTCLK");
>> +	if (IS_ERR_OR_NULL(rtc->extclk)) {
>>   		dev_err(dev, "could not get EXTCLK\n");
>> -	} else {
>> -		ret = clk_prepare_enable(rtc->extclk);
>> -		if (ret) {
>> -			dev_err(dev, "failed to enable EXTCLK\n");
>> -			goto err_disable_pclk;
>> -		}
>> +		return PTR_ERR_OR_ZERO(rtc->extclk);
>>   	}
> [Severity: High]
> Does this have the same silent probe truncation or Device Tree compatibility
> issues as the PCLK conversion above?
>
> [ ... ]
>
>> @@ -179,29 +161,13 @@ static int ftrtc010_rtc_probe(struct platform_device *pdev)
>>   	ret = devm_request_irq(dev, rtc->rtc_irq, ftrtc010_rtc_interrupt,
>>   			       IRQF_SHARED, pdev->name, dev);
> [Severity: High]
> This is a pre-existing issue, but I noticed the driver requests a shared
> interrupt with IRQF_SHARED. Looking at ftrtc010_rtc_interrupt(), it returns
> IRQ_HANDLED unconditionally:
>
> static irqreturn_t ftrtc010_rtc_interrupt(int irq, void *dev)
> {
>          return IRQ_HANDLED;
> }
>
> If another device sharing this IRQ line asserts an interrupt and gets stuck,
> would this unconditional return prevent the kernel's spurious interrupt
> detector from disabling the broken IRQ line, potentially leading to an
> interrupt storm and a system hang?
>
> [ ... ]
>
> [Severity: High]
> This isn't a bug introduced by this patch, but looking through the driver,
> is there a race condition against the hardware ticking in both
> ftrtc010_rtc_read_time() and ftrtc010_rtc_set_time()?
>
> In 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 the hardware clock rolls over (for example, incrementing the minute from
> 59 seconds to 00 seconds) between the reads of two registers, could the
> software combine the old value of one register with the newly incremented
> value of another, resulting in wildly corrupted time calculations?
>

[PATCH v1 3/3] rtc: ftrtc010: fix integer overflow in time calculation

[Liu Dalin]

In ftrtc010_rtc_read_time() and ftrtc010_rtc_set_time(), all
variables (offset, days, hour, min, sec) are u32. The expression
"days * 86400 + hour * 3600 + min * 60 + sec" is evaluated in
32-bit arithmetic, which silently wraps around for dates beyond
approximately year 2106 (U32_MAX / 86400 ≈ 49710 days).

Cast the u32 operands to timeu64_t before multiplication to ensure
the arithmetic is performed in 64-bit, preventing overflow and
matching the driver's rtc_dev->range_max configuration.

Fixes: 1d61d2592c1f ("rtc: ftrtc010: Rename to Faraday FTRTC010")
Assisted-by: Sashiko AI [static analysis]
Signed-off-by: Liu Dalin <liudalin@kylinsec.com.cn>
---
 drivers/rtc/rtc-ftrtc010.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
index 19e6333bfbab..8f2652132400 100644
--- a/drivers/rtc/rtc-ftrtc010.c
+++ b/drivers/rtc/rtc-ftrtc010.c
@@ -72,7 +72,8 @@ static int ftrtc010_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	days = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
 	offset = readl(rtc->rtc_base + FTRTC010_RTC_RECORD);
 
-	time = offset + days * 86400 + hour * 3600 + min * 60 + sec;
+	time = (timeu64_t)offset + (timeu64_t)days * 86400 +
+	       (timeu64_t)hour * 3600 + (timeu64_t)min * 60 + sec;
 
 	rtc_time64_to_tm(time, tm);
 
@@ -92,7 +93,8 @@ static int ftrtc010_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	hour = readl(rtc->rtc_base + FTRTC010_RTC_HOUR);
 	day = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
 
-	offset = time - (day * 86400 + hour * 3600 + min * 60 + sec);
+	offset = time - ((timeu64_t)day * 86400 + (timeu64_t)hour * 3600 +
+			 (timeu64_t)min * 60 + sec);
 
 	writel(offset, rtc->rtc_base + FTRTC010_RTC_RECORD);
 	writel(0x01, rtc->rtc_base + FTRTC010_RTC_CR);
-- 
2.43.0

Re: [PATCH v1 3/3] rtc: ftrtc010: fix integer overflow in time calculation

[sashiko-bot]

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

New issues:
- [High] Transition to 64-bit arithmetic zero-extends negative time offsets, causing the clock to erroneously jump ~136 years into the future.

Pre-existing issues:
- [High] Unsynchronized, non-atomic reads of consecutive RTC time registers can produce severely corrupted timestamps if a hardware tick and rollover occurs during the read sequence.
--

commit 0451b19111beae8b1f5abde3e58d176d4ad642f2
Author: Liu Dalin <liudalin@kylinsec.com.cn>

rtc: ftrtc010: fix integer overflow in time calculation

This commit addresses an integer overflow in ftrtc010_rtc_read_time and
ftrtc010_rtc_set_time. It casts the 32-bit time variables to timeu64_t before
multiplication to ensure 64-bit arithmetic, preventing overflow for dates
beyond year 2106.

> diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
> index 2f610a0fc6e9b..6fff537cad27e 100644
> --- a/drivers/rtc/rtc-ftrtc010.c
> +++ b/drivers/rtc/rtc-ftrtc010.c
> @@ -73,7 +73,8 @@ static int ftrtc010_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  	days = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
>  	offset = readl(rtc->rtc_base + FTRTC010_RTC_RECORD);

[Severity: High]
This isn't a bug introduced by this patch, but can these unsynchronized,
non-atomic reads of consecutive RTC time registers produce corrupted
timestamps?

If a hardware tick and rollover occurs during the read sequence:

ftrtc010_rtc_read_time()
    sec  = readl(rtc->rtc_base + FTRTC010_RTC_SECOND);
    min  = readl(rtc->rtc_base + FTRTC010_RTC_MINUTE);
    ...

It appears possible to read desynchronized values, which could result in
permanent time corruption up to 1 day.

> -	time = offset + days * 86400 + hour * 3600 + min * 60 + sec;
> +	time = (timeu64_t)offset + (timeu64_t)days * 86400 +
> +	       (timeu64_t)hour * 3600 + (timeu64_t)min * 60 + sec;

[Severity: High]
Does this transition to 64-bit arithmetic zero-extend negative time offsets?

If the offset is negative, casting it directly to timeu64_t breaks the two's
complement modulo arithmetic required to correctly decode negative time
differences, which could cause the clock to erroneously jump about 136 years
into the future.

> @@ -93,7 +94,8 @@ static int ftrtc010_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	hour = readl(rtc->rtc_base + FTRTC010_RTC_HOUR);
>  	day = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);

[Severity: High]
This isn't a bug introduced by this patch, but can this sequential read
result in computing an improperly desynchronized offset?

If a tick occurs between reads:

ftrtc010_rtc_set_time()
    sec = readl(rtc->rtc_base + FTRTC010_RTC_SECOND);
    min = readl(rtc->rtc_base + FTRTC010_RTC_MINUTE);
    ...

It seems this could permanently corrupt the stored RTC offset and skew the
time until it is reset.

> -	offset = time - (day * 86400 + hour * 3600 + min * 60 + sec);
> +	offset = time - ((timeu64_t)day * 86400 + (timeu64_t)hour * 3600 +
> +			 (timeu64_t)min * 60 + sec);

[Severity: High]
Does this calculation incorrectly wrap offset into a large 32-bit integer?

If the target Unix time is numerically smaller than the hardware's internal
uptime counters, the difference becomes negative. This can happen on offline
embedded devices where the initial OS time defaults to 1970-01-01.

Casting this negative 32-bit offset to timeu64_t zero-extends it instead of
allowing the required 32-bit modulo arithmetic. This could lead to a massive
forward jump when ftrtc010_rtc_read_time is subsequently called.

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

Re: [PATCH v1 3/3] rtc: ftrtc010: fix integer overflow in time calculation

[liudalin]

On Thu, 27 Aug 2026, Sashiko AI wrote:

 > [Severity: High]
 > Does this transition to 64-bit arithmetic zero-extend negative time
 > offsets? If the offset is negative, casting it directly to timeu64_t
 > breaks the two's complement modulo arithmetic required to correctly
 > decode negative time differences, which could cause the clock to
 > erroneously jump about 136 years into the future.

Thank you for the thorough analysis. This is a valid issue and has been
fixed in v2.

The original code cast each operand individually to timeu64_t:
   time = (timeu64_t)offset + (timeu64_t)days * 86400 + ...

When offset is a negative time difference stored as u32 (e.g.,
0xFFFFFFFE for -2), zero-extension produces 0x00000000FFFFFFFE,
causing a ~136-year forward jump.

The fix is to perform the addition in 32-bit arithmetic first,
then cast the result to 64-bit:
   time = (timeu64_t)(offset + days * 86400 + hour * 3600 + min * 60 + sec);

The u32 addition naturally wraps using two's complement, and the
final cast to timeu64_t preserves the correct value.

For ftrtc010_rtc_set_time(), no change is needed -- the offset is
assigned to a u32 variable, which automatically truncates to 32 bits,
preserving the correct two's complement result.

 > [Severity: High]
 > This isn't a bug introduced by this patch, but can these unsynchronized,
 > non-atomic reads of consecutive RTC time registers produce corrupted
 > timestamps?

Please see the response to the same concern on patches 1 and 2. The
1 Hz tick rate makes this negligible in practice, and the hardware
provides no latching mechanism. This is outside the scope of this
patch.

 > [Severity: High]
 > This isn't a bug introduced by this patch, but can this sequential read
 > result in computing an improperly desynchronized offset?

Same as above -- outside the scope of this patch.

 > [Severity: High]
 > Does this calculation incorrectly wrap offset into a large 32-bit
 > integer? If the target Unix time is numerically smaller than the
 > hardware's internal uptime counters, the difference becomes negative.

The offset variable is u32, which stores the negative difference as
a two's complement value. When read back in ftrtc010_rtc_read_time(),
the u32 addition wraps correctly. This is the intended behavior of
the driver's offset-based timekeeping design.

--
2.43.0

在 2026/8/27 14:53, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] Transition to 64-bit arithmetic zero-extends negative time offsets, causing the clock to erroneously jump ~136 years into the future.
>
> Pre-existing issues:
> - [High] Unsynchronized, non-atomic reads of consecutive RTC time registers can produce severely corrupted timestamps if a hardware tick and rollover occurs during the read sequence.
> --
>
> commit 0451b19111beae8b1f5abde3e58d176d4ad642f2
> Author: Liu Dalin <liudalin@kylinsec.com.cn>
>
> rtc: ftrtc010: fix integer overflow in time calculation
>
> This commit addresses an integer overflow in ftrtc010_rtc_read_time and
> ftrtc010_rtc_set_time. It casts the 32-bit time variables to timeu64_t before
> multiplication to ensure 64-bit arithmetic, preventing overflow for dates
> beyond year 2106.
>
>> diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
>> index 2f610a0fc6e9b..6fff537cad27e 100644
>> --- a/drivers/rtc/rtc-ftrtc010.c
>> +++ b/drivers/rtc/rtc-ftrtc010.c
>> @@ -73,7 +73,8 @@ static int ftrtc010_rtc_read_time(struct device *dev, struct rtc_time *tm)
>>   	days = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
>>   	offset = readl(rtc->rtc_base + FTRTC010_RTC_RECORD);
> [Severity: High]
> This isn't a bug introduced by this patch, but can these unsynchronized,
> non-atomic reads of consecutive RTC time registers produce corrupted
> timestamps?
>
> If a hardware tick and rollover occurs during the read sequence:
>
> ftrtc010_rtc_read_time()
>      sec  = readl(rtc->rtc_base + FTRTC010_RTC_SECOND);
>      min  = readl(rtc->rtc_base + FTRTC010_RTC_MINUTE);
>      ...
>
> It appears possible to read desynchronized values, which could result in
> permanent time corruption up to 1 day.
>
>> -	time = offset + days * 86400 + hour * 3600 + min * 60 + sec;
>> +	time = (timeu64_t)offset + (timeu64_t)days * 86400 +
>> +	       (timeu64_t)hour * 3600 + (timeu64_t)min * 60 + sec;
> [Severity: High]
> Does this transition to 64-bit arithmetic zero-extend negative time offsets?
>
> If the offset is negative, casting it directly to timeu64_t breaks the two's
> complement modulo arithmetic required to correctly decode negative time
> differences, which could cause the clock to erroneously jump about 136 years
> into the future.
>
>> @@ -93,7 +94,8 @@ static int ftrtc010_rtc_set_time(struct device *dev, struct rtc_time *tm)
>>   	hour = readl(rtc->rtc_base + FTRTC010_RTC_HOUR);
>>   	day = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
> [Severity: High]
> This isn't a bug introduced by this patch, but can this sequential read
> result in computing an improperly desynchronized offset?
>
> If a tick occurs between reads:
>
> ftrtc010_rtc_set_time()
>      sec = readl(rtc->rtc_base + FTRTC010_RTC_SECOND);
>      min = readl(rtc->rtc_base + FTRTC010_RTC_MINUTE);
>      ...
>
> It seems this could permanently corrupt the stored RTC offset and skew the
> time until it is reset.
>
>> -	offset = time - (day * 86400 + hour * 3600 + min * 60 + sec);
>> +	offset = time - ((timeu64_t)day * 86400 + (timeu64_t)hour * 3600 +
>> +			 (timeu64_t)min * 60 + sec);
> [Severity: High]
> Does this calculation incorrectly wrap offset into a large 32-bit integer?
>
> If the target Unix time is numerically smaller than the hardware's internal
> uptime counters, the difference becomes negative. This can happen on offline
> embedded devices where the initial OS time defaults to 1970-01-01.
>
> Casting this negative 32-bit offset to timeu64_t zero-extends it instead of
> allowing the required 32-bit modulo arithmetic. This could lead to a massive
> forward jump when ftrtc010_rtc_read_time is subsequently called.
>

[PATCH v2 0/3] rtc: ftrtc010: fix resource management and integer overflow

[Liu Dalin]

This series fixes three issues in the Faraday FTRTC010 RTC driver
found by Sashiko AI static analysis:

Patch 1: Fixes a clock resource leak when devm_rtc_register_device()
fails in ftrtc010_rtc_probe().

Patch 2: Replaces manual clock management with devm APIs to fix a
use-after-disable vulnerability in ftrtc010_rtc_remove(). The RTC
device is registered via devm_rtc_register_device() but clocks are
manually disabled in .remove() before devres cleanup, creating a
window where MMIO reads on clock-gated hardware can cause a
synchronous external abort.

Patch 3: Fixes integer overflow in ftrtc010_rtc_read_time() and
ftrtc010_rtc_set_time() where u32 arithmetic wraps for dates
beyond year 2106.

All three patches are based on v7.2-rc2 and have been verified
with checkpatch.pl --strict.

Changes since v1:
- Added reply to Sashiko AI review comments on pre-existing issues
  (shared IRQ handler, sequential read rollover, TOCTOU race)
  explaining why they are not in scope for these patches.
- Clarified that PCLK/EXTCLK are required by the hardware and
  devicetree binding, so the old "optional" behavior was a bug.
- Patch 3 received no Sashiko AI review comments, confirming the
  integer overflow fix is correct.

Liu Dalin (3):
  rtc: ftrtc010: fix clock resource leak on probe failure
  rtc: ftrtc010: use devm clock APIs to fix use-after-disable in remove
  rtc: ftrtc010: fix integer overflow in time calculation

 drivers/rtc/rtc-ftrtc010.c | 72 ++++++++++++++------------------------
 1 file changed, 25 insertions(+), 47 deletions(-)

--
2.43.0

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

[Liu Dalin]

When devm_rtc_register_device() fails, ftrtc010_rtc_probe() returns
the error directly without releasing clocks obtained via
clk_prepare_enable(). This leaks both extclk and pclk resources,
which may prevent subsequent probe attempts or module removal.

Capture the return value and jump to the error handling path to
properly release clocks via clk_disable_unprepare().

Fixes smatch warnings:
- drivers/rtc/rtc-ftrtc010.c:189 ftrtc010_rtc_probe() warn:
  'rtc->extclk' from clk_prepare_enable() not released on lines: 183.
- drivers/rtc/rtc-ftrtc010.c:189 ftrtc010_rtc_probe() warn:
  'rtc->pclk' from clk_prepare_enable() not released on lines: 183.

Fixes: fdcfd854333b ("rtc: rework rtc_register_device() resource management")
Assisted-by: smatch:2.0 [static analysis]
Signed-off-by: Liu Dalin <liudalin@kylinsec.com.cn>
---
 drivers/rtc/rtc-ftrtc010.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
index 98fa0f518303..512790bc3f2b 100644
--- a/drivers/rtc/rtc-ftrtc010.c
+++ b/drivers/rtc/rtc-ftrtc010.c
@@ -180,7 +180,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);
-- 
2.43.0

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Shared interrupt handler unconditionally returns IRQ_HANDLED, defeating the kernel's spurious interrupt detection.
--

commit 8daf0ea14af5dc0abeba82e55b73ddd78a69f258
Author: Liu Dalin <liudalin@kylinsec.com.cn>

rtc: ftrtc010: fix clock resource leak on probe failure

This commit fixes a clock resource leak during the RTC probe failure path.
It ensures that if devm_rtc_register_device() fails, the driver properly
jumps to the error handling path to release extclk and pclk resources.

> 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;
>  

[Severity: High]
This isn't a bug introduced by this patch, but a pre-existing issue. The
handler is registered with the IRQF_SHARED flag just above this check:

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

However, the interrupt handler unconditionally returns IRQ_HANDLED without
verifying the hardware status:

static irqreturn_t ftrtc010_rtc_interrupt(int irq, void *dev)
{
	return IRQ_HANDLED;
}

Can this cause a system lockup if an unhandled interrupt storm occurs on
the shared IRQ line? 

If another device shares this line or if the bootloader leaves the RTC 
interrupt enabled, the kernel might fail to disable the spurious interrupt 
because this handler falsely claims it, starving the CPU.

> -	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);

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

[PATCH v2 2/3] rtc: ftrtc010: use devm clock APIs to fix use-after-disable in remove

[Liu Dalin]

The driver mixes devm_rtc_register_device() with manual clock
disabling in ftrtc010_rtc_remove(). Because the RTC device was
registered via devm, it remains registered until devres cleanup
runs after .remove() returns. An unprivileged process holding an
open /dev/rtc0 file descriptor could issue ioctl(RTC_RD_TIME)
during this window, triggering MMIO reads on clock-gated hardware
and causing a synchronous external abort.

Replace devm_clk_get() + clk_prepare_enable() with
devm_clk_get_enabled() to tie clock lifecycle to devres. Remove
the manual .remove() callback and the corresponding error handling
labels in ftrtc010_rtc_probe().

Fixes: 1d61d2592c1f ("rtc: ftrtc010: Rename to Faraday FTRTC010")
Assisted-by: Sashiko AI [static analysis]
Signed-off-by: Liu Dalin <liudalin@kylinsec.com.cn>
---
 drivers/rtc/rtc-ftrtc010.c | 67 +++++++++-----------------------------
 1 file changed, 16 insertions(+), 51 deletions(-)

diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
index 512790bc3f2b..19e6333bfbab 100644
--- a/drivers/rtc/rtc-ftrtc010.c
+++ b/drivers/rtc/rtc-ftrtc010.c
@@ -119,51 +119,33 @@ static int ftrtc010_rtc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	platform_set_drvdata(pdev, rtc);
 
-	rtc->pclk = devm_clk_get(dev, "PCLK");
-	if (IS_ERR(rtc->pclk)) {
+	rtc->pclk = devm_clk_get_enabled(dev, "PCLK");
+	if (IS_ERR_OR_NULL(rtc->pclk)) {
 		dev_err(dev, "could not get PCLK\n");
-	} else {
-		ret = clk_prepare_enable(rtc->pclk);
-		if (ret) {
-			dev_err(dev, "failed to enable PCLK\n");
-			return ret;
-		}
+		return PTR_ERR_OR_ZERO(rtc->pclk);
 	}
-	rtc->extclk = devm_clk_get(dev, "EXTCLK");
-	if (IS_ERR(rtc->extclk)) {
+	rtc->extclk = devm_clk_get_enabled(dev, "EXTCLK");
+	if (IS_ERR_OR_NULL(rtc->extclk)) {
 		dev_err(dev, "could not get EXTCLK\n");
-	} else {
-		ret = clk_prepare_enable(rtc->extclk);
-		if (ret) {
-			dev_err(dev, "failed to enable EXTCLK\n");
-			goto err_disable_pclk;
-		}
+		return PTR_ERR_OR_ZERO(rtc->extclk);
 	}
 
 	rtc->rtc_irq = platform_get_irq(pdev, 0);
-	if (rtc->rtc_irq < 0) {
-		ret = rtc->rtc_irq;
-		goto err_disable_extclk;
-	}
+	if (rtc->rtc_irq < 0)
+		return rtc->rtc_irq;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		ret = -ENODEV;
-		goto err_disable_extclk;
-	}
+	if (!res)
+		return -ENODEV;
 
 	rtc->rtc_base = devm_ioremap(dev, res->start,
 				     resource_size(res));
-	if (!rtc->rtc_base) {
-		ret = -ENOMEM;
-		goto err_disable_extclk;
-	}
+	if (!rtc->rtc_base)
+		return -ENOMEM;
 
 	rtc_dev = devm_rtc_allocate_device(dev);
-	if (IS_ERR(rtc_dev)) {
-		ret = PTR_ERR(rtc_dev);
-		goto err_disable_extclk;
-	}
+	if (IS_ERR(rtc_dev))
+		return PTR_ERR(rtc_dev);
 
 	rtc_dev->ops = &ftrtc010_rtc_ops;
 
@@ -178,29 +160,13 @@ static int ftrtc010_rtc_probe(struct platform_device *pdev)
 	ret = devm_request_irq(dev, rtc->rtc_irq, ftrtc010_rtc_interrupt,
 			       IRQF_SHARED, pdev->name, dev);
 	if (unlikely(ret))
-		goto err_disable_extclk;
+		return ret;
 
 	ret = devm_rtc_register_device(rtc_dev);
 	if (ret)
-		goto err_disable_extclk;
+		return ret;
 
 	return 0;
-
-err_disable_extclk:
-	clk_disable_unprepare(rtc->extclk);
-err_disable_pclk:
-	clk_disable_unprepare(rtc->pclk);
-	return ret;
-}
-
-static void ftrtc010_rtc_remove(struct platform_device *pdev)
-{
-	struct ftrtc010_rtc *rtc = platform_get_drvdata(pdev);
-
-	if (!IS_ERR(rtc->extclk))
-		clk_disable_unprepare(rtc->extclk);
-	if (!IS_ERR(rtc->pclk))
-		clk_disable_unprepare(rtc->pclk);
 }
 
 static const struct of_device_id ftrtc010_rtc_dt_match[] = {
@@ -216,7 +182,6 @@ static struct platform_driver ftrtc010_rtc_driver = {
 		.of_match_table = ftrtc010_rtc_dt_match,
 	},
 	.probe		= ftrtc010_rtc_probe,
-	.remove		= ftrtc010_rtc_remove,
 };
 
 module_platform_driver_probe(ftrtc010_rtc_driver, ftrtc010_rtc_probe);
-- 
2.43.0

Re: [PATCH v2 2/3] rtc: ftrtc010: use devm clock APIs to fix use-after-disable in remove

[sashiko-bot]

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

New issues:
- [High] Replacing devm_clk_get() with devm_clk_get_enabled() incorrectly turns optional clocks into mandatory ones, breaking probe on platforms where the clocks are missing.
- [High] The probe function erroneously returns success (0) if devm_clk_get_enabled() returns NULL, silently bypassing device initialization.

Pre-existing issues:
- [High] The interrupt handler unconditionally returns IRQ_HANDLED on a shared IRQ line without verifying if the hardware actually generated the interrupt.
- [High] Reading the RTC time registers sequentially without checking for rollovers can result in reading corrupted/inconsistent time values.
--

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

rtc: ftrtc010: use devm clock APIs to fix use-after-disable in remove
    
This commit replaces manual clock management with devm_clk_get_enabled() to
fix a use-after-disable window during driver removal. It also removes the
manual .remove() callback, relying on devres for cleanup.

> diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
> --- a/drivers/rtc/rtc-ftrtc010.c
> +++ b/drivers/rtc/rtc-ftrtc010.c
[ ... ]
> @@ -120,51 +120,33 @@ static int ftrtc010_rtc_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	platform_set_drvdata(pdev, rtc);
>  
> -	rtc->pclk = devm_clk_get(dev, "PCLK");
> -	if (IS_ERR(rtc->pclk)) {
> +	rtc->pclk = devm_clk_get_enabled(dev, "PCLK");
> +	if (IS_ERR_OR_NULL(rtc->pclk)) {
>  		dev_err(dev, "could not get PCLK\n");
> -	} else {
> -		ret = clk_prepare_enable(rtc->pclk);
> -		if (ret) {
> -			dev_err(dev, "failed to enable PCLK\n");
> -			return ret;
> -		}
> +		return PTR_ERR_OR_ZERO(rtc->pclk);

[Severity: High]
Does this change inadvertently make the previously optional PCLK mandatory?

In the original code, if devm_clk_get() returned an error (like -ENOENT),
it logged a message but safely allowed probe to continue. With this patch,
if devm_clk_get_enabled() returns -ENOENT, PTR_ERR_OR_ZERO() will propagate
that error and abort the probe.

Additionally, if the kernel is configured without CONFIG_HAVE_CLK,
devm_clk_get_enabled() returns NULL. In that scenario, IS_ERR_OR_NULL()
evaluates to true, and PTR_ERR_OR_ZERO(NULL) returns 0.

Will this cause the probe function to immediately return success without
actually registering the RTC device or mapping memory?

>  	}
> -	rtc->extclk = devm_clk_get(dev, "EXTCLK");
> -	if (IS_ERR(rtc->extclk)) {
> +	rtc->extclk = devm_clk_get_enabled(dev, "EXTCLK");
> +	if (IS_ERR_OR_NULL(rtc->extclk)) {
>  		dev_err(dev, "could not get EXTCLK\n");
> -	} else {
> -		ret = clk_prepare_enable(rtc->extclk);
> -		if (ret) {
> -			dev_err(dev, "failed to enable EXTCLK\n");
> -			goto err_disable_pclk;
> -		}
> +		return PTR_ERR_OR_ZERO(rtc->extclk);

[Severity: High]
The same issues with optional clock requirement and silent probe bypass on
NULL apply here for EXTCLK.

[ ... ]
>  	rtc_dev->ops = &ftrtc010_rtc_ops;
>  
>  	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);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for a
hardware clock rollover to corrupt the timestamp being read here?

Since the time registers are read sequentially without any check for
rollover across multiple units, a tick occurring exactly between these
sequential reads could result in erroneously mixing an old higher-order
value with a new lower-order value.

>  
>  	rtc_dev->range_min = (u64)days * 86400 + hour * 3600 + min * 60 + sec;
>  	rtc_dev->range_max = U32_MAX + rtc_dev->range_min;
>  
>  	ret = devm_request_irq(dev, rtc->rtc_irq, ftrtc010_rtc_interrupt,
>  			       IRQF_SHARED, pdev->name, dev);

[Severity: High]
This is a pre-existing issue, but since the interrupt is registered as
IRQF_SHARED, is it safe for ftrtc010_rtc_interrupt() to unconditionally
return IRQ_HANDLED?

If another device sharing this IRQ line asserts an interrupt, returning
IRQ_HANDLED without verifying the interrupt source could prevent the core
IRQ tracking mechanism from detecting and disabling spurious interrupt
storms.

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

[PATCH v2 3/3] rtc: ftrtc010: fix integer overflow in time calculation

[Liu Dalin]

In ftrtc010_rtc_read_time() and ftrtc010_rtc_set_time(), all
variables (offset, days, hour, min, sec) are u32. The expression
"days * 86400 + hour * 3600 + min * 60 + sec" is evaluated in
32-bit arithmetic, which silently wraps around for dates beyond
approximately year 2106 (U32_MAX / 86400 ≈ 49710 days).

In ftrtc010_rtc_read_time(), perform the addition in 32-bit
arithmetic first to preserve two's complement wrapping for negative
offsets, then cast the result to timeu64_t. In
ftrtc010_rtc_set_time(), cast individual u32 operands to timeu64_t
before multiplication to prevent overflow in the right-hand side
of the offset calculation.

Fixes: 1d61d2592c1f ("rtc: ftrtc010: Rename to Faraday FTRTC010")
Assisted-by: Sashiko AI [static analysis]
Signed-off-by: Liu Dalin <liudalin@kylinsec.com.cn>
---
 drivers/rtc/rtc-ftrtc010.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
index 19e6333bfbab..b29c96be40f4 100644
--- a/drivers/rtc/rtc-ftrtc010.c
+++ b/drivers/rtc/rtc-ftrtc010.c
@@ -72,7 +72,7 @@ static int ftrtc010_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	days = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
 	offset = readl(rtc->rtc_base + FTRTC010_RTC_RECORD);
 
-	time = offset + days * 86400 + hour * 3600 + min * 60 + sec;
+	time = (timeu64_t)(offset + days * 86400 + hour * 3600 + min * 60 + sec);
 
 	rtc_time64_to_tm(time, tm);
 
@@ -92,7 +92,8 @@ static int ftrtc010_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	hour = readl(rtc->rtc_base + FTRTC010_RTC_HOUR);
 	day = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
 
-	offset = time - (day * 86400 + hour * 3600 + min * 60 + sec);
+	offset = time - ((timeu64_t)day * 86400 + (timeu64_t)hour * 3600 +
+			 (timeu64_t)min * 60 + (timeu64_t)sec);
 
 	writel(offset, rtc->rtc_base + FTRTC010_RTC_RECORD);
 	writel(0x01, rtc->rtc_base + FTRTC010_RTC_CR);
-- 
2.43.0

Re: [PATCH v2 3/3] rtc: ftrtc010: fix integer overflow in time calculation

[sashiko-bot]

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

New issues:
- [High] The patch claims to fix the Y2106 integer overflow, but mathematically computes the exact same 32-bit truncated values as the original code, making it a completely ineffective placebo.

Pre-existing issues:
- [High] Unsynchronized sequential reads of the RTC time registers lack a latching or retry mechanism, causing hardware tearing and persistent time corruption.
- [High] Unconditional IRQ_HANDLED return in an IRQF_SHARED interrupt handler breaks the kernel's spurious interrupt detector, causing a system hard lockup.
--

commit 9a9a270ad0bf60c8a0b5860cccb3bb2994f4adcf
Author: Liu Dalin <liudalin@kylinsec.com.cn>

rtc: ftrtc010: fix integer overflow in time calculation

This patch modifies the time calculation in the ftrtc010 driver to cast
variables to timeu64_t before calculating offsets and days. This is
intended to prevent 32-bit arithmetic wrapping for dates beyond year 2106.

> diff --git a/drivers/rtc/rtc-ftrtc010.c b/drivers/rtc/rtc-ftrtc010.c
> index 2f610a0fc6e9b..c989574b1fbc6 100644
> --- a/drivers/rtc/rtc-ftrtc010.c
> +++ b/drivers/rtc/rtc-ftrtc010.c
> @@ -73,7 +73,7 @@ static int ftrtc010_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  	days = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);
>  	offset = readl(rtc->rtc_base + FTRTC010_RTC_RECORD);

[Severity: High]
This is a pre-existing issue, but do the sequential reads in
ftrtc010_rtc_read_time() lack a software retry loop?

If an internal hardware counter rolls over while the registers are being
read (e.g., SECOND from 59 to 0), it seems the driver could acquire an
inconsistent temporal state. Would this expose the time computation to
transient corruption?

> -	time = offset + days * 86400 + hour * 3600 + min * 60 + sec;
> +	time = (timeu64_t)(offset + days * 86400 + hour * 3600 + min * 60 + sec);

[Severity: High]
Does this explicit cast to timeu64_t in ftrtc010_rtc_read_time() actually
prevent the integer overflow?

Since offset and all counter variables are u32, this addition will be
evaluated in 32-bit arithmetic and wrap around (modulo 2^32) before the
result is cast to 64-bit. Because of this, it appears the final returned
time can never exceed 0xFFFFFFFF when reading an RTC time beyond year
2106, leaving the issue unfixed.

>  	rtc_time64_to_tm(time, tm);
>  
[ ... ]
> @@ -93,7 +93,8 @@ static int ftrtc010_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	hour = readl(rtc->rtc_base + FTRTC010_RTC_HOUR);
>  	day = readl(rtc->rtc_base + FTRTC010_RTC_DAYS);

[Severity: High]
This is a pre-existing issue, but similar to ftrtc010_rtc_read_time(),
could a hardware tear occur during these sequential unlatched reads in
ftrtc010_rtc_set_time()?

If reading the hardware registers concurrently with an internal hardware
counter rollover, the torn state would be used to compute the epoch
offset. This offset is then permanently committed to the hardware RTC
record, corrupting the system RTC base.

> -	offset = time - (day * 86400 + hour * 3600 + min * 60 + sec);
> +	offset = time - ((timeu64_t)day * 86400 + (timeu64_t)hour * 3600 +
> +			 (timeu64_t)min * 60 + (timeu64_t)sec);

[Severity: High]
Does casting the operands to timeu64_t in ftrtc010_rtc_set_time()
successfully fix the truncation issue here?

The offset variable is declared as a u32. Assigning the 64-bit difference
to u32 offset instantly discards the upper 32 bits, making it
mathematically identical to the original 32-bit arithmetic. This seems to
prevent the driver from successfully setting an RTC time beyond year 2106.

>  	writel(offset, rtc->rtc_base + FTRTC010_RTC_RECORD);
>  	writel(0x01, rtc->rtc_base + FTRTC010_RTC_CR);

While reviewing this file, I noticed another pre-existing issue:

drivers/rtc/rtc-ftrtc010.c:ftrtc010_rtc_interrupt() {
    static irqreturn_t ftrtc010_rtc_interrupt(int irq, void *dev)
    {
        return IRQ_HANDLED;
    }
}

[Severity: High]
This isn't a bug introduced by this patch, but does this unconditional
return of IRQ_HANDLED in an IRQF_SHARED interrupt handler break the
kernel's spurious interrupt detector?

By unconditionally claiming ownership of unhandled interrupts generated by
other devices on the same shared line, this could prevent the kernel from
disabling a malfunctioning IRQ line. Would this potentially lead to an
infinite interrupt loop and a system hard lockup?

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