Re: [PATCH] rtc: stm32: fix clock leak on pinctrl probe failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Routing pinctrl failures to the manual clock cleanup path creates a critical window where devm-managed resources (IRQ, RTC device) remain active while hardware clocks are disabled, risking a synchronous external abort (bus fault).
--

commit 813d70ca28c122c9814bc7903a9c96494f127aea
Author: Cong Nguyen <congnt264@gmail.com>

rtc: stm32: fix clock leak on pinctrl probe failure

This commit addresses a clock leak during pinctrl probe failure. It updates
the error handling paths for devm_pinctrl_register_and_init() and
pinctrl_enable() to route through the existing err label, ensuring rtc_ck
and pclk are unwound properly.

> diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
> index d4ebf3eb54aa9..e7a942f6c1906 100644
> --- a/drivers/rtc/rtc-stm32.c
> +++ b/drivers/rtc/rtc-stm32.c

[ ... ]

> @@ -1175,12 +1175,16 @@ static int stm32_rtc_probe(struct platform_device *pdev)
>  	stm32_rtc_clean_outs(rtc);
>  
>  	ret = devm_pinctrl_register_and_init(&pdev->dev, &stm32_rtc_pdesc, rtc, &pctl);
> -	if (ret)
> -		return dev_err_probe(&pdev->dev, ret, "pinctrl register failed");
> +	if (ret) {
> +		dev_err_probe(&pdev->dev, ret, "pinctrl register failed");
> +		goto err;

[Severity: High]
Does this new error path create a window for a synchronous external abort?

When pinctrl initialization fails (for example, returning -EPROBE_DEFER),
this now jumps to the err label where rtc_ck and pclk are manually disabled.

However, the RTC device and the alarm IRQ were registered earlier in the
probe function using devres-managed functions:

stm32_rtc_probe()
    rtc->rtc_dev = devm_rtc_device_register(...);
    ret = devm_request_threaded_irq(..., stm32_rtc_alarm_irq, ...);

Because devres resources are not released until after the probe function
returns, there is a window where the clocks are disabled but the IRQ handler
and RTC device remain active.

Could the still-registered IRQ fire during this cleanup window? If that
happens, or if userspace accesses the RTC device, driver callbacks like
stm32_rtc_alarm_irq() might attempt to access hardware registers without
the APB clock enabled, resulting in a bus fault.

> +	}
>  
>  	ret = pinctrl_enable(pctl);
> -	if (ret)
> -		return dev_err_probe(&pdev->dev, ret, "pinctrl enable failed");
> +	if (ret) {
> +		dev_err_probe(&pdev->dev, ret, "pinctrl enable failed");
> +		goto err;
> +	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914132914.2371788-1-congnt264@gmail.com?part=1