Re: [PATCH RFC 11/16] clk: qcom: gdsc: Make GDSC parenting optional

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk: qcom: missing PM domain cleanup on gdsc_register() error paths leads to UAF
--

commit c8bac551fa3005340251f161ce7de48e8d323366
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

clk: qcom: gdsc: Make GDSC parenting optional

Allows callers of gdsc_register() to suppress parents inferred from the
controller device and its attached domain list.

> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index b99d70149af34..2d0803e1eb3b1 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
[ ... ]
> @@ -654,13 +655,17 @@ int gdsc_register(struct gdsc_desc *desc,
>  
>  	/* Add subdomains */
>  	for (i = 0; i < num; i++) {
> +		ret = 0;
> +
>  		if (!scs[i])
>  			continue;
> +
>  		if (scs[i]->parent)
>  			ret = pm_genpd_add_subdomain(scs[i]->parent, &scs[i]->pd);
> -		else if (!IS_ERR_OR_NULL(dev->pm_domain))
> +		else if (!desc->no_external_parent &&
> +			 !IS_ERR_OR_NULL(dev->pm_domain))
>  			ret = pm_genpd_add_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);
> -		else if (desc->pd_list)
> +		else if (!desc->no_external_parent && desc->pd_list)
>  			ret = gdsc_add_subdomain_list(desc->pd_list, &scs[i]->pd);
>  
>  		if (ret)

[Severity: High]
Does this error path correctly clean up all initialized PM domains?

If pm_genpd_add_subdomain() fails, the code jumps to err_pm_subdomain_remove:

	if (ret)
		goto err_pm_subdomain_remove;
...
err_pm_subdomain_remove:
	gdsc_pm_subdomain_remove(desc, i);

	return ret;

This correctly rolls back the subdomains, but doesn't seem to call
pm_genpd_remove() for the generic PM domains that were successfully
initialized earlier via gdsc_init() -> pm_genpd_init().

Similarly, earlier in gdsc_register(), if gdsc_init() fails, the function
returns immediately:

	for (i = 0; i < num; i++) {
		if (!scs[i])
			continue;
		...
		ret = gdsc_init(scs[i]);
		if (ret)
			return ret;
	}

And later, if of_genpd_add_provider_onecell() fails:

	return of_genpd_add_provider_onecell(dev->of_node, data);

Both of these return paths exit without cleaning up previously initialized
domains.

Since these domains are statically added to the global gpd_list but tied
to devm-allocated structures, could failing the probe (e.g., via
EPROBE_DEFER) leave dangling pointers in the core PM domain list and cause
a use-after-free when traversing gpd_list later?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-topic-qc_clk_dvfs-v1-0-3041b788e9f7@oss.qualcomm.com?part=11