Re: [PATCH v2] PCI: qcom-ep: Fix runtime PM reference leak in probe error paths

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Double-disable of runtime PM on `devm_pm_runtime_enable()` failure permanently breaks runtime PM.
- [High] Double-put of the runtime PM usage counter due to unhandled fall-through on `pm_runtime_put_sync()` failure.
--

commit c1b17f3fe6107b8e2d84d7608e5bf717e1aa999e
Author: Wentao Liang <vulab@iscas.ac.cn>

PCI: qcom-ep: Fix runtime PM reference leak in probe error paths

This commit updates qcom_pcie_ep_probe() to distinguish between error
paths before and after devm_pm_runtime_enable() succeeds. It adds
manual cleanup for early errors and uses pm_runtime_put_sync() for
later errors to prevent runtime PM reference leaks.

> - On the successful probe path: call pm_runtime_put_sync() to drop
>   the initial reference. The return value of pm_runtime_put_sync()
>   is ignored because errors like -EAGAIN or -EBUSY only indicate
>   that the device cannot be suspended at this moment and do not
>   represent a fatal probe failure.

[Severity: High]
The commit message states that the return value of pm_runtime_put_sync()
is ignored, but the code still checks the return value and jumps to the
error path if it fails.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -892,16 +892,16 @@ static int qcom_pcie_ep_probe(struct platform_device *pdev)
>  	pm_runtime_set_active(dev);
>  	ret = devm_pm_runtime_enable(dev);
>  	if (ret)
> -		return ret;
> +		goto err_manual_cleanup;

[Severity: High]
If devm_pm_runtime_enable() fails (for example, due to a devres memory
allocation failure), it internally guarantees its cleanup action is executed,
which already calls pm_runtime_disable().

By jumping to err_manual_cleanup and calling pm_runtime_disable() again,
will this incorrectly increment the device's disable_depth to 2? Could this
permanently leave runtime PM disabled for the device on subsequent deferred
probe attempts?

[ ... ]

> @@ -932,6 +932,13 @@ static int qcom_pcie_ep_probe(struct platform_device *pdev)
>  err_ep_deinit:
>  	dw_pcie_ep_deinit(&pcie_ep->pci.ep);
>  
> +err_put_ref:
> +	pm_runtime_put_sync(dev);
> +	return ret;

[Severity: High]
Because the patch leaves the original error check intact on the successful
probe path:

    ret = pm_runtime_put_sync(dev);
    if (ret < 0) {
        dev_err(dev, "Failed to suspend device: %d\n", ret);
        goto err_disable_irqs;
    }

If the initial put returns a transient error like -EAGAIN, the usage count
has already been decremented by the PM core. However, the code jumps to
err_disable_irqs, unwinds the probe, and falls through to err_put_ref here.

Could this result in calling pm_runtime_put_sync() a second time, causing
a usage counter underflow?

> +err_manual_cleanup:
> +	pm_runtime_put_noidle(dev);
> +	pm_runtime_disable(dev);
>  	return ret;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624132943.40718-1-vulab@iscas.ac.cn?part=1