Re: [PATCH v12 10/14] pmdomain: rockchip: cycle optional power-domain resets on power-on

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Massive Use-After-Free of genpd structures if probe fails and pm_genpd_remove() fails during error cleanup.
--

commit 82ba89727b40b24aa0ac406b6e9c71489c988d78
Author: Jiaxing Hu <gahing@gahingwoo.com>

    pmdomain: rockchip: cycle optional power-domain resets on power-on
    
    Some Rockchip domains come out of power-on with their bus interface in an
    undefined state. On the RK3576 NPU this shows up as a hang on the first
    register access after the domain is switched on, and pulsing the domain's
    resets at this point clears it.

> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c

[ ... ]

> @@ -949,6 +975,7 @@ static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
>  
>  	clk_bulk_unprepare(pd->num_clks, pd->clks);
>  	clk_bulk_put(pd->num_clks, pd->clks);
> +	reset_control_put(pd->resets);
>  
>  	/* protect the zeroing of pm->num_clks */
>  	mutex_lock(&pd->pmu->mutex);

[Severity: High]
This is a pre-existing issue, but does this error cleanup path leave a freed
structure in the global gpd_list if probe fails?

During error cleanup, rockchip_pm_domain_cleanup() iterates forward through
the domains array:

    for (i = 0; i < pmu->genpd_data.num_domains; i++) {
        genpd = pmu->genpd_data.domains[i];
        if (genpd) {
            pd = to_rockchip_pd(genpd);
            rockchip_pm_remove_one_domain(pd);
        }
    }

Since parent domains typically appear before their children in the array, the
parent is visited first. 

When rockchip_pm_remove_one_domain() attempts to remove the parent domain,
pm_genpd_remove() fails with -EBUSY because the child is still attached:

    ret = pm_genpd_remove(&pd->genpd);
    if (ret < 0)
        dev_err(pd->pmu->dev, "failed to remove domain '%s' : %d...",

The domain is left in the global gpd_list and the function continues without
retrying.

Because rockchip_pm_add_one_domain() allocates the domain structure with
devm_kzalloc():

    pd = devm_kzalloc(pmu->dev, sizeof(*pd), GFP_KERNEL);

When probe returns an error, such as -EPROBE_DEFER, devres automatically frees
the memory backing the parent domain. 

Can this leave a dangling pointer in the global gpd_list that could trigger a
use-after-free or kernel panic when the list is next traversed, such as
during pm_genpd_poweroff_unused()? Should rockchip_pm_domain_cleanup() be
updated to iterate backwards to ensure children are removed before their
parents?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912065053.1519165-1-gahing@gahingwoo.com?part=10