Re: [PATCH RFC] pmdomain: imx: reject a missing bus power domain

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The error handling path in the probe function fails to remove the power domain notifier registered on `bc->bus_power_dev`, leading to a use-after-free in the genpd notifier chain.
--

commit 1bf3e63904f509711299c259aa62b9016d200e03
Author: Slavin Liu <bolin.liu@seu.edu.cn>

pmdomain: imx: reject a missing bus power domain
    
The named-domain attach API may return NULL for an absent name.
Treat that as -ENODEV before publishing bus_power_dev to later unwind
paths, without changing the handling of existing error pointers.

> diff --git a/drivers/pmdomain/imx/imx8m-blk-ctrl.c b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> index 479789009c7f4..9653a8ac9a6e9 100644
> --- a/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> @@ -228,6 +228,8 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	bc->bus_power_dev = dev_pm_domain_attach_by_name(dev, "bus");
> +	if (!bc->bus_power_dev)
> +		return dev_err_probe(dev, -ENODEV, "missing bus power domain\n");

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in the error
handling path of imx8m_blk_ctrl_probe(). If a failure occurs after the power
notifier is added, for example during devm_of_platform_populate():

imx8m_blk_ctrl_probe()
    ...
	ret = dev_pm_genpd_add_notifier(bc->bus_power_dev, &bc->power_nb);
	if (ret) {
		...
		goto cleanup_provider;
	}

	dev_set_drvdata(dev, bc);

	ret = devm_of_platform_populate(dev);
	if (ret)
		goto cleanup_provider;
    ...
cleanup_provider:
	of_genpd_del_provider(dev->of_node);
cleanup_pds:
    ...
	dev_pm_domain_detach(bc->bus_power_dev, true);
	return ret;

The error path detaches the bus power domain but fails to unregister the
notifier. Is it possible this leaves a dangling pointer to bc->power_nb when
the device memory is freed, causing a use-after-free when the bus power
domain transitions power states? Should dev_pm_genpd_remove_notifier() be
called during cleanup?

>  	if (IS_ERR(bc->bus_power_dev)) {
>  		if (PTR_ERR(bc->bus_power_dev) == -ENODEV)
>  			return dev_err_probe(dev, -EPROBE_DEFER,

[ ... ]

> diff --git a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> index d255c20f58e21..e952b4ee75c00 100644
> --- a/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8mp-blk-ctrl.c
> @@ -655,6 +655,8 @@ static int imx8mp_blk_ctrl_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	bc->bus_power_dev = dev_pm_domain_attach_by_name(dev, "bus");
> +	if (!bc->bus_power_dev)
> +		return dev_err_probe(dev, -ENODEV, "missing bus power domain\n");

[Severity: High]
This isn't a bug introduced by this patch, but a similar issue appears in the
error handling path of imx8mp_blk_ctrl_probe(). If bc_data->probe(bc) fails:

imx8mp_blk_ctrl_probe()
    ...
	ret = dev_pm_genpd_add_notifier(bc->bus_power_dev, &bc->power_nb);
	if (ret) {
		...
		goto cleanup_provider;
	}

	if (bc_data->probe) {
		ret = bc_data->probe(bc);
		if (ret)
			goto cleanup_provider;
	}
    ...
cleanup_provider:
	of_genpd_del_provider(dev->of_node);
cleanup_pds:
    ...
	dev_pm_domain_detach(bc->bus_power_dev, true);
	return ret;

Like in the i.MX8M driver, the error path detaches the bus power domain but
doesn't call dev_pm_genpd_remove_notifier() to unregister bc->power_nb. Could
this also lead to a use-after-free when 'bc' is freed?

>  	if (IS_ERR(bc->bus_power_dev))
>  		return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
>  				     "failed to attach bus power domain\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911060929.94481-1-bolin.liu@seu.edu.cn?part=1