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

[sashiko-bot@kernel.org]

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