[PATCH] rtc: omap: check clk_prepare_enable() return value

[PATCH] rtc: omap: check clk_prepare_enable() return value

[Жамбакиев Радий Рикардинович]

From: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>

clk_prepare_enable() may fail, e.g. with -EPROBE_DEFER if the clock
provider is not ready yet. The return value is ignored in
omap_rtc_probe(), so the driver continues probing with an unprepared
clock and later calls clk_disable_unprepare() on it, in
omap_rtc_remove() or in the ioremap error path, leading to a
prepare/unprepare imbalance.

Check the return value and propagate the error on failure. No
explicit cleanup is needed on the error path as the clock reference
is managed by devres and the clock was never prepared.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 532409aa1ba8 ("rtc: omap: Add internal clock enabling support")
Signed-off-by: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>
---
 drivers/rtc/rtc-omap.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index 0f90065e352c..97d03ded51db 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c
@@ -758,8 +758,11 @@ static int omap_rtc_probe(struct platform_device *pdev)
 	else
 		rtc->clk = devm_clk_get(&pdev->dev, "int-clk");
 
-	if (!IS_ERR(rtc->clk))
-		clk_prepare_enable(rtc->clk);
+	if (!IS_ERR(rtc->clk)) {
+		ret = clk_prepare_enable(rtc->clk);
+		if (ret)
+			return ret;
+	}
 
 	rtc->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(rtc->base)) {
-- 
2.53.0

Re: [PATCH] rtc: omap: check clk_prepare_enable() return value

[sashiko-bot]

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

Pre-existing issues:
- [High] Unhandled PM runtime activation failure causes kernel panic during probe.
- [High] Use-after-disable and race condition during device removal causes kernel panic.
--

commit d40541deeb08cdcd7e4b8124b2e9843e03d69ef2
Author: Radiy Zhambakiev <r.zhambakiev@prosoftsystems.ru>

rtc: omap: check clk_prepare_enable() return value

This commit updates omap_rtc_probe() to check the return value of
clk_prepare_enable() and propagate the error. This prevents continuing
with an unprepared clock and later causing a prepare/unprepare imbalance
on error paths.

> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index 0f90065e352cb..97d03ded51dbe 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -758,8 +758,11 @@ static int omap_rtc_probe(struct platform_device *pdev)
>  	else
>  		rtc->clk = devm_clk_get(&pdev->dev, "int-clk");
>  
> -	if (!IS_ERR(rtc->clk))
> -		clk_prepare_enable(rtc->clk);
> +	if (!IS_ERR(rtc->clk)) {
> +		ret = clk_prepare_enable(rtc->clk);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	rtc->base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(rtc->base)) {

[Severity: High]
This isn't a bug introduced by this patch, but there is a severe pre-existing
issue in omap_rtc_probe() shortly after this clock initialization:

    pm_runtime_enable(&pdev->dev);
    pm_runtime_get_sync(&pdev->dev);

    rtc->type->unlock(rtc);

Does this code safely handle PM runtime activation failures?

The return value of pm_runtime_get_sync() is ignored before unconditionally
accessing hardware registers. If the PM domain fails to enable due to clock
issues, regulator failure, or device tree misconfiguration, the function
returns a negative error code but the driver proceeds anyway.

It then immediately calls rtc->type->unlock(rtc), which translates to
am3352_rtc_unlock() and executes rtc_writel(). Accessing OMAP peripheral
registers when the power domain is inactive triggers a synchronous external
abort, crashing the system during device probe.

[Severity: High]
This is also a pre-existing issue, but does the devres-managed registration
create a fatal race condition during device removal?

Later in omap_rtc_probe(), the RTC device and IRQs are registered via devm:

    ret = devm_request_irq(&pdev->dev, rtc->irq_timer, rtc_irq, 0,
                           dev_name(&rtc->rtc->dev), rtc);
    ...
    ret = devm_rtc_register_device(rtc->rtc);

However, in omap_rtc_remove(), the driver manually powers off the hardware:

    /* Disable the clock/module */
    pm_runtime_put_sync(&pdev->dev);
    pm_runtime_disable(&pdev->dev);

Because the RTC interface and IRQs are managed by devres, they are not
deregistered until after omap_rtc_remove() returns. This leaves a race
window where the hardware is completely powered down, but the RTC device is
still exposed to userspace via /dev/rtc0 and the IRQ handlers remain active.

If a userspace process issues an ioctl like RTC_RD_TIME, or if a spurious IRQ
arrives during this window, omap_rtc_read_time() or rtc_irq() will attempt to
read unpowered hardware registers. This triggers a synchronous external abort
and panics the kernel upon driver unbind or module removal.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911114350.160356-1-r.zhambakiev@prosoftsystems.ru?part=1