Re: [PATCH 4/4] clk: renesas: convert drivers over to use subsys_platform_driver()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing of_genpd_del_provider() on error paths leads to stale OF PM domain provider registrations and Critical Use-After-Free.
--

commit 26447fc60447ea2456fb17817135325feb86bdfb
Author: Brian Masney <bmasney@redhat.com>
Subject: clk: renesas: convert drivers over to use subsys_platform_driver()

Reduce boilerplate code in the drivers and convert them over to use
subsys_platform_driver().

> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
> index 80f4403ea2baf..87b555e38980a 100644
> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
[ ... ]
> -static int __init cpg_mssr_init(void)
> -{
> -	return platform_driver_probe(&cpg_mssr_driver, cpg_mssr_probe);
> -}
> -
> -subsys_initcall(cpg_mssr_init);
> +subsys_platform_driver_probe(cpg_mssr_driver, cpg_mssr_probe);

[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free
on the error path for cpg_mssr_probe().

In cpg_mssr_add_clk_domain(), the OF provider is successfully registered:

    return of_genpd_add_provider_simple(np, genpd);

However, there doesn't seem to be a corresponding devres cleanup registered
for this. If cpg_mssr_probe() fails later on, the devres cleanup routine
will call pm_genpd_remove() to unregister the PM domain.

Because of_genpd_add_provider_simple() sets genpd->has_provider = true,
pm_genpd_remove() will abort with -EBUSY and fail to remove the domain
from the global gpd_list.

Devres will then free the devm-allocated driver structure, which contains
the genpd struct.

Does this leave the global gpd_list and of_genpd_providers lists with
dangling pointers to the freed genpd struct, leading to memory corruption
when those lists are accessed later?

> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
> index 7f8decef92fd9..f6293c968bcf9 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
[ ... ]
> -static int __init rzg2l_cpg_init(void)
> -{
> -	return platform_driver_probe(&rzg2l_cpg_driver, rzg2l_cpg_probe);
> -}
> -
> -subsys_initcall(rzg2l_cpg_init);
> +subsys_platform_driver_probe(rzg2l_cpg_driver, rzg2l_cpg_probe);

[Severity: High]
This is the same pre-existing issue as above, but affecting the error path 
in rzg2l_cpg_probe().

In rzg2l_cpg_add_clk_domain(), the OF provider is registered with
of_genpd_add_provider_simple(np, genpd) but lacks a devres cleanup action.

If the probe fails after this point, could this lead to the same resource
leak and use-after-free scenario when the PM domain lists are subsequently
accessed?

> diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
> index 63f13fe4e4b0e..7ce94680c79a2 100644
> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c
[ ... ]
> -static int __init rzv2h_cpg_init(void)
> -{
> -	return platform_driver_probe(&rzv2h_cpg_driver, rzv2h_cpg_probe);
> -}
> -
> -subsys_initcall(rzv2h_cpg_init);
> +subsys_platform_driver_probe(rzv2h_cpg_driver, rzv2h_cpg_probe);

[Severity: High]
Similarly, this pre-existing issue appears to affect rzv2h_cpg_probe().

In rzv2h_cpg_add_pm_domains(), the provider is registered via
of_genpd_add_provider_simple(np, &pd->genpd) without a cleanup handler.

Would a probe failure in this driver also result in dangling pointers in
the global PM domain lists?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-subsys_initcall-v1-0-cbccf4cd4288@redhat.com?part=4