Re: [PATCH v2 6/7] pmdomain: riscv: Add RPMI performance domains as power domains

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pmdomain: devres UAF on genpd removal via sysfs unbind
--

commit bcb5ae58c392c6ab2a9b0f57c2292a07b1a39866
Author: Joshua Yeong <joshua.yeong@starfivetech.com>
Subject: pmdomain: riscv: Add RPMI performance domains as power domains

This commit registers the performance domains of the RISC-V platform management
interface (RPMI) performance service group as generic power domains. Devices
that are not CPUs can reference these domains through the "power-domains"
property and drive them using the OPP library.

> diff --git a/drivers/pmdomain/riscv/riscv-rpmi-perf-domain.c b/drivers/pmdomain/riscv/riscv-rpmi-perf-domain.c
> new file mode 100644
> index 0000000000000..fc5c512fe0697
> --- /dev/null
> +++ b/drivers/pmdomain/riscv/riscv-rpmi-perf-domain.c
> @@ -0,0 +1,265 @@

[ ... ]

> +static int rpmi_perf_genpd_probe(struct platform_device *pdev)
> +{
> +	struct rpmi_perf **mpxy_perf = dev_get_platdata(&pdev->dev);
> +	struct device *dev = &pdev->dev;
> +	struct rpmi_perf_genpd_provider *provider;
> +	struct generic_pm_domain **domains;
> +	u32 i, num_domains, registered = 0;
> +	int ret;
> +
> +	if (!mpxy_perf || !*mpxy_perf)
> +		return -EINVAL;
> +
> +	num_domains = rpmi_perf_num_domains(*mpxy_perf);
> +
> +	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
> +	if (!provider)
> +		return -ENOMEM;
> +
> +	provider->pds = devm_kcalloc(dev, num_domains, sizeof(*provider->pds),
> +				     GFP_KERNEL);

[Severity: High]
Since provider->pds is allocated using devm_kcalloc(), this memory will be
automatically freed as soon as the driver's remove callback returns.

> +	if (!provider->pds)
> +		return -ENOMEM;

[ ... ]

> +static void rpmi_perf_genpd_remove(struct platform_device *pdev)
> +{
> +	struct rpmi_perf_genpd_provider *provider = platform_get_drvdata(pdev);
> +	u32 i;
> +
> +	of_genpd_del_provider(dev_of_node(pdev->dev.parent));
> +
> +	for (i = 0; i < provider->data.num_domains; i++) {
> +		if (provider->data.domains[i])
> +			pm_genpd_remove(provider->data.domains[i]);

[Severity: High]
Does this code ignore the return value of pm_genpd_remove()?

If a user manually unbinds the driver via sysfs while consumer devices are
still attached to the PM domains, pm_genpd_remove() will return -EBUSY and
fail to remove the domain from the core genpd framework's global gpd_list.

Because the memory backing these domains (provider->pds) was allocated with
devres and will be freed immediately after this remove function returns,
could this leave dangling pointers in the global gpd_list?

This might lead to a use-after-free and potential kernel panic when
the gpd_list is traversed later.

> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008091032.2832333-1-joshua.yeong@starfivetech.com?part=6