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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] rtc: stm32: NULL pointer dereference in stm32_rtc_pinmux_action_lsco()
- [High] rtc: stm32: clk_lsco and OF provider leak on probe error
--

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

rtc: stm32: fix clock leak on pinctrl probe failure

A failing devm_pinctrl_register_and_init() or pinctrl_enable() in
stm32_rtc_probe() returns directly instead of going through err:,
leaking rtc_ck/pclk -- and since -EPROBE_DEFER is routine here, each
deferred retry leaks another.

Link: https://lore.kernel.org/r/20260914135401.373D91F000FF@smtp.kernel.org

> diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
> index d4ebf3eb54aa9..457f66663ba59 100644
> --- a/drivers/rtc/rtc-stm32.c
> +++ b/drivers/rtc/rtc-stm32.c
> @@ -1137,6 +1137,20 @@ static int stm32_rtc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err;
>  
> +	stm32_rtc_clean_outs(rtc);
> +
> +	ret = devm_pinctrl_register_and_init(&pdev->dev, &stm32_rtc_pdesc, rtc, &pctl);
> +	if (ret) {
> +		dev_err_probe(&pdev->dev, ret, "pinctrl register failed");
> +		goto err;
> +	}
> +
> +	ret = pinctrl_enable(pctl);

[Severity: High]
Does this code trigger a null pointer dereference if the 'lsco' function is
configured as a pinmux hog in the device tree?

Calling pinctrl_enable() synchronously claims hogs, which invokes the pinmux
callback. If 'lsco' is claimed, stm32_rtc_pinmux_action_lsco() will be
called:

stm32_rtc_pinmux_action_lsco() {
    struct device *dev = rtc->rtc_dev->dev.parent;

Since devm_rtc_device_register() hasn't been called yet in the new probe
sequence, rtc->rtc_dev is still NULL, which will cause a kernel panic during
probe.

> +	if (ret) {
> +		dev_err_probe(&pdev->dev, ret, "pinctrl enable failed");
> +		goto err;
> +	}
> +
>  	rtc->irq_alarm = platform_get_irq(pdev, 0);
>  	if (rtc->irq_alarm <= 0) {
>  		ret = rtc->irq_alarm;
>  		goto err;

[Severity: High]
Can a failure here leak the manually registered clock gate and OF provider?

If 'lsco' is claimed by pinctrl_enable() above, it registers a clock gate and
an OF provider:

stm32_rtc_pinmux_action_lsco() {
    ...
    rtc->clk_lsco = clk_register_gate(dev, "rtc_lsco", ...);
    of_clk_add_provider(dev->of_node, of_clk_src_simple_get, rtc->clk_lsco);

Because pinctrl setup was moved earlier, any subsequent error such as
platform_get_irq() failing will jump to the err: label. The err: cleanup path
does not unregister rtc->clk_lsco or the OF provider. This will permanently
leak these resources on routine errors like -EPROBE_DEFER, prevent successful
re-probing, and can cause a use-after-free later since there is no
of_clk_del_provider() call.

>  	}

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