Re: [PATCH v4 6/8] pmdomain: Make genpd_get_from_provider() public

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pmdomain: UAF due to un-refcounted genpd pointer export
--

commit a7a3a37d2be118bab420ebff1f13629dfa676eb6
Author: Geert Uytterhoeven <geert+renesas@glider.be>

pmdomain: Make genpd_get_from_provider() public

This commit makes genpd_get_from_provider() public as
of_genpd_get_from_provider(), allowing PM Domain drivers to look up
generic_pm_domain structures directly.

> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 88c886814a5b9..4136368456d99 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -3018,8 +3018,7 @@ EXPORT_SYMBOL_GPL(of_genpd_del_provider);
>   * Returns a valid pointer to struct generic_pm_domain on success or ERR_PTR()
>   * on failure.
>   */
> -static struct generic_pm_domain *genpd_get_from_provider(
> -					const struct of_phandle_args *genpdspec)
> +struct generic_pm_domain *of_genpd_get_from_provider(const struct of_phandle_args *genpdspec)
>  {
>  	struct generic_pm_domain *genpd = ERR_PTR(-EPROBE_DEFER);
>  	struct of_genpd_provider *provider;
> @@ -3041,6 +3040,7 @@ static struct generic_pm_domain *genpd_get_from_provider(
>  
>  	return genpd;
>  }
> +EXPORT_SYMBOL_GPL(of_genpd_get_from_provider);

[Severity: High]
Can exporting this API lead to a use-after-free regression?

of_genpd_get_from_provider() returns an unpinned, un-refcounted pointer to
struct generic_pm_domain whose lifetime is managed by the provider. While the
commit message compares this to the clock subsystem, clock pointers are
refcounted or uniquely allocated.

The follow-up r8a78000-mdlc patch demonstrates caching these genpd pointers in
its power_map without acquiring any locks (since gpd_list_lock is internal).
Because the SCMI provider is looked up dynamically via
of_find_compatible_node(), fw_devlink cannot automatically detect the
dependency to create a device link.

If the SCMI driver is unbound (via sysfs or module unload), pm_genpd_remove()
will free the genpd structures, leaving dangling pointers. Furthermore,
overwriting the attach_dev and detach_dev function pointers of the SCMI genpd
objects will lead to invalid execution if the module is unloaded.

[ ... ]

> @@ -3113,7 +3113,7 @@ int of_genpd_add_child_ids(struct device_node *np,
>  			goto err_unwind;
>  
>  		/* Get the parent domain */
> -		parent_genpd = genpd_get_from_provider(&parent_args);
> +		parent_genpd = of_genpd_get_from_provider(&parent_args);

[Severity: High]
This problem wasn't introduced by this patch, but does this internal caller
have a time-of-check to time-of-use (TOCTOU) race?

Fetching parent_genpd locklessly in of_genpd_add_child_ids() allows a
concurrent pm_genpd_remove() to free the domain before gpd_list_lock is
subsequently acquired.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792163045.git.geert+renesas@glider.be?part=6