Re: [PATCH 03/16] clk: qcom: gcc-sm8450: Tie the CX power domain to controller

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

Pre-existing issues:
- [High] Unbinding the driver causes a devres ordering issue that leads to a lockless linked list Use-After-Free and permanent power leak.
--

--- Patch [3]: [PATCH 03/16] clk: qcom: gcc-sm8450: Tie the CX power domain to controller ---
commit fce258c6d28a3b8028f9c7e9bc3065854572f262
Author: Abel Vesa <[email protected]>

clk: qcom: gcc-sm8450: Tie the CX power domain to controller

Enable runtime PM for the controller so the common GCC probe path resumes
the attached domain while registering clocks, resets and GDSCs. This lets
GDSC consumers propagate their votes through the GCC provider to the CX
parent domain.

> diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
> index b18bb34889ab2..b3ab458048dc8 100644
> --- a/drivers/clk/qcom/gcc-sm8450.c
> +++ b/drivers/clk/qcom/gcc-sm8450.c
> @@ -3399,6 +3399,7 @@ static const struct qcom_cc_desc gcc_sm8450_desc = {
>  	.resets = gcc_sm8450_resets,
>  	.num_resets = ARRAY_SIZE(gcc_sm8450_resets),
>  	.gdscs = gcc_sm8450_gdscs,
> +	.use_rpm = true,

[Severity: High]
This is a pre-existing issue, but will setting use_rpm expose a Use-After-Free
and power leak during driver unbind?

When the driver unbinds, devres cleanup runs qcom_cc_gdsc_unregister(), which
calls gdsc_pm_subdomain_remove(). However, at this point platform_remove() has
already detached the PM domain, making dev->pm_domain NULL.

This causes gdsc_pm_subdomain_remove() in drivers/clk/qcom/gdsc.c to skip the
safe detachment:

	else if (!IS_ERR_OR_NULL(dev->pm_domain))
		pm_genpd_remove_subdomain(pd_to_genpd(dev->pm_domain), &scs[i]->pd);

As a result, an unsafe fallback execution path is taken in
drivers/pmdomain/core.c:genpd_remove():

	list_for_each_entry_safe(link, l, &genpd->child_links, child_node) {
		list_del(&link->parent_node);
		list_del(&link->child_node);
		kfree(link);
	}

This modifies the parent's parent_links list without acquiring the parent's
lock, which could lead to a lockless linked list Use-After-Free. It also
fails to decrement sd_count, which keeps the CX domain powered on forever.

Since devm_add_action_or_reset() in
drivers/clk/qcom/common.c:qcom_cc_really_probe() schedules this cleanup to
happen after dev->pm_domain is cleared, is there a way to address this devres
ordering issue before enabling use_rpm?

>  	.num_gdscs = ARRAY_SIZE(gcc_sm8450_gdscs),
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3