Re: [PATCH v2 4/6] arm64: dts: qcom: sm8450: Add missing CX power domain to GCC

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing `pm_genpd_remove()` and error path cleanup in `gdsc_register()` leads to use-after-free in `gpd_list` upon unbind or probe failure.
--

commit e4315c043469a13f84ee75ac94f4e0790692142c
Author: Neil Armstrong <neil.armstrong@linaro.org>

arm64: dts: qcom: sm8450: Add missing CX power domain to GCC

This commit associates the GCC with RPMHPD_CX by adding a power-domains
property to the device tree, preventing under-voltage conditions by allowing
votes to propagate.

> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -935,6 +935,7 @@ soc: soc@0 {
>  		gcc: clock-controller@100000 {
>  			compatible = "qcom,gcc-sm8450";
>  			reg = <0x0 0x00100000 0x0 0x1f4200>;
> +			power-domains = <&rpmhpd RPMHPD_CX>;

[Severity: High]
This is a pre-existing issue, but since this patch configures the power
domains for this device, it brings up a concern in the underlying Qualcomm
GDSC driver framework.

If probe fails or the module is unloaded, does the error handling in
gdsc_register() leave dangling pointers in the core gpd_list?

Looking at drivers/clk/qcom/gdsc.c:gdsc_register(), if a subsequent
gdsc_init() fails, it returns directly:

drivers/clk/qcom/gdsc.c:gdsc_register() {
    for (i = 0; i < num; i++) {
        if (!scs[i])
            continue;
        ...
        ret = gdsc_init(scs[i]);
        if (ret)
            return ret;
    }
}

This seems to leak the previously initialized power domains in the global
gpd_list because it misses calling pm_genpd_remove() for scs[0..i-1].

Similarly, if of_genpd_add_provider_onecell() fails at the end of the
function, it also returns immediately or bypasses domain removal:

drivers/clk/qcom/gdsc.c:gdsc_register() {
    ...
    return of_genpd_add_provider_onecell(dev->of_node, data);

err_pm_subdomain_remove:
    gdsc_pm_subdomain_remove(desc, i);

    return ret;
}

Furthermore, gdsc_unregister() removes subdomains and deletes the provider,
but entirely misses pm_genpd_remove() before the backing memory is freed
on module unload:

drivers/clk/qcom/gdsc.c:gdsc_unregister() {
    ...
    gdsc_pm_subdomain_remove(desc, num);
    of_genpd_del_provider(dev->of_node);
}

Could this result in a use-after-free kernel panic if gpd_list is iterated
(for example, by genpd_power_off_unused() or debugfs) after an unbind or
probe failure?

>  			#clock-cells = <1>;
>  			#reset-cells = <1>;
>  			#power-domain-cells = <1>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-topic-sm8x50-tie-gcc-to-cx-v2-0-6b5752dd4747@linaro.org?part=4