Re: [PATCH v2 2/4] phy: core: Add common helper for get phy phandle by index

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free race condition due to momentarily dropping the PHY provider's module reference.
--

commit 82b3a08205293dd6de5d86ce337bb43664f65534
Author: Inochi Amaoto <inochiama@gmail.com>

phy: core: Add common helper for get phy phandle by index

This commit refactors several PHY helpers that use an index to retrieve
the PHY phandle of a device node, adding a common helper function to
allow for future reuse.

> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index ea79913fbb87f..a7d7acb5d3c2a 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
[ ... ]
> @@ -682,21 +682,18 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
>  }
>  
>  /**
> - * of_phy_get() - lookup and obtain a reference to a phy using a device_node.
> + * of_phy_get_by_index() - lookup and obtain a reference to a phy using a
> + * device_node by index.
>   * @np: device_node for which to get the phy
> - * @con_id: name of the phy from device's point of view
> + * @index: index of the phy from device's point of view
>   *
> - * Returns the phy driver, after getting a refcount to it; or
> + * Returns: the phy driver, after getting a refcount to it; or
>   * -ENODEV if there is no such phy. The caller is responsible for
>   * calling of_phy_put() to release that count.
>   */
> -struct phy *of_phy_get(struct device_node *np, const char *con_id)
> +static struct phy *of_phy_get_by_index(struct device_node *np, int index)
>  {
> -	struct phy *phy = NULL;
> -	int index = 0;
> -
> -	if (con_id)
> -		index = of_property_match_string(np, "phy-names", con_id);
> +	struct phy *phy;
>  
>  	phy = _of_phy_get(np, index);
>  	if (IS_ERR(phy))

[Severity: High]
This is a pre-existing issue, but does this pattern risk a use-after-free
race condition if a concurrent module unload occurs?

Looking at _of_phy_get(), it temporarily acquires the PHY provider module
reference, but drops it before returning the bare phy pointer:

_of_phy_get() {
    ...
    phy = phy_provider->of_xlate(phy_provider->dev, &args);

out_put_module:
    module_put(phy_provider->owner);
out_unlock:
    mutex_unlock(&phy_provider_mutex);
    ...
    return phy;
}

By the time execution continues in this helper and calls
try_module_get(phy->ops->owner) slightly further down, the module
reference is no longer held. If a concurrent module unload proceeds during
this brief window, the PHY provider could be unregistered and the phy
struct freed, causing the dereference of phy->ops to access freed memory.

Could this refactoring be an opportunity to address this race window?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904083709.425893-1-inochiama@gmail.com?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy