[PATCH v15 1/4] phy: core: Fix use-after-free in phy_get paths
Loic Poulain
loic.poulain at oss.qualcomm.com
Fri Sep 4 05:40:20 PDT 2026
On Fri, Sep 4, 2026 at 1:53 PM Bryan O'Donoghue
<bryan.odonoghue at linaro.org> wrote:
>
> Sashiko asked during a patch review if the existing usage pattern had a
> race condition; specifically in of_phy_get() if it was possible between
> returning from _of_phy_get() and running try_module_get() that a module
> might be unbound leading to use-after-free.
>
> Looking at the code this appears to be so, there is no linkage between the
> phy and module under a synchronisation primitive.
>
> Using the phy_provider_mutex in phy_get() will ensure there is a link between
> the returned phy pointer and the module_get() bumping the module reference
> count.
>
> Amend phy_get(), of_phy_get() and devm_of_phy_get_by_index() to fix the
> same usage pattern.
>
> phy_provider_unregister() must take the phy_provider_mutex so amending
> phy_get()/of_phy_get() to take that same mutex guarantees there is no
> use-after-free.
>
> Fixes: ff764963479a1 ("drivers: phy: add generic PHY framework")
> Cc: stable at vger.kernel.org
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org>
Reviewed-by: Loic Poulain <loic.poulain at oss.qualcomm.com>
> ---
> drivers/phy/phy-core.c | 45 ++++++++++++++++++++++++++++++---------------
> 1 file changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 21aaf2f76e53e..89addd732bff3 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -124,13 +124,13 @@ static struct phy *phy_find(struct device *dev, const char *con_id)
> const char *dev_id = dev_name(dev);
> struct phy_lookup *p, *pl = NULL;
>
> - mutex_lock(&phy_provider_mutex);
> + lockdep_assert_held(&phy_provider_mutex);
> +
> list_for_each_entry(p, &phys, node)
> if (!strcmp(p->dev_id, dev_id) && !strcmp(p->con_id, con_id)) {
> pl = p;
> break;
> }
> - mutex_unlock(&phy_provider_mutex);
>
> return pl ? pl->phy : ERR_PTR(-ENODEV);
> }
> @@ -624,6 +624,8 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
> struct phy *phy = NULL;
> struct of_phandle_args args;
>
> + lockdep_assert_held(&phy_provider_mutex);
> +
> ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
> index, &args);
> if (ret)
> @@ -635,11 +637,10 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
> goto out_put_node;
> }
>
> - mutex_lock(&phy_provider_mutex);
> phy_provider = of_phy_provider_lookup(args.np);
> if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
> phy = ERR_PTR(-EPROBE_DEFER);
> - goto out_unlock;
> + goto out_put_node;
> }
>
> if (!of_device_is_available(args.np)) {
> @@ -653,8 +654,6 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
> out_put_module:
> module_put(phy_provider->owner);
>
> -out_unlock:
> - mutex_unlock(&phy_provider_mutex);
> out_put_node:
> of_node_put(args.np);
>
> @@ -678,15 +677,21 @@ struct phy *of_phy_get(struct device_node *np, const char *con_id)
> if (con_id)
> index = of_property_match_string(np, "phy-names", con_id);
>
> + mutex_lock(&phy_provider_mutex);
> +
> phy = _of_phy_get(np, index);
> if (IS_ERR(phy))
> - return phy;
> + goto out_unlock;
>
> - if (!try_module_get(phy->ops->owner))
> - return ERR_PTR(-EPROBE_DEFER);
> + if (!try_module_get(phy->ops->owner)) {
> + phy = ERR_PTR(-EPROBE_DEFER);
> + goto out_unlock;
> + }
>
> get_device(&phy->dev);
>
> +out_unlock:
> + mutex_unlock(&phy_provider_mutex);
> return phy;
> }
> EXPORT_SYMBOL_GPL(of_phy_get);
> @@ -786,6 +791,7 @@ struct phy *phy_get(struct device *dev, const char *string)
> struct phy *phy;
> struct device_link *link;
>
> + mutex_lock(&phy_provider_mutex);
> if (dev->of_node) {
> if (string)
> index = of_property_match_string(dev->of_node, "phy-names",
> @@ -796,15 +802,18 @@ struct phy *phy_get(struct device *dev, const char *string)
> } else {
> if (string == NULL) {
> dev_WARN(dev, "missing string\n");
> - return ERR_PTR(-EINVAL);
> + phy = ERR_PTR(-EINVAL);
> + goto out_unlock;
> }
> phy = phy_find(dev, string);
> }
> if (IS_ERR(phy))
> - return phy;
> + goto out_unlock;
>
> - if (!try_module_get(phy->ops->owner))
> - return ERR_PTR(-EPROBE_DEFER);
> + if (!try_module_get(phy->ops->owner)) {
> + phy = ERR_PTR(-EPROBE_DEFER);
> + goto out_unlock;
> + }
>
> get_device(&phy->dev);
>
> @@ -813,6 +822,8 @@ struct phy *phy_get(struct device *dev, const char *string)
> dev_dbg(dev, "failed to create device link to %s\n",
> dev_name(phy->dev.parent));
>
> +out_unlock:
> + mutex_unlock(&phy_provider_mutex);
> return phy;
> }
> EXPORT_SYMBOL_GPL(phy_get);
> @@ -961,15 +972,17 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
> if (!ptr)
> return ERR_PTR(-ENOMEM);
>
> + mutex_lock(&phy_provider_mutex);
> phy = _of_phy_get(np, index);
> if (IS_ERR(phy)) {
> devres_free(ptr);
> - return phy;
> + goto out_unlock;
> }
>
> if (!try_module_get(phy->ops->owner)) {
> devres_free(ptr);
> - return ERR_PTR(-EPROBE_DEFER);
> + phy = ERR_PTR(-EPROBE_DEFER);
> + goto out_unlock;
> }
>
> get_device(&phy->dev);
> @@ -982,6 +995,8 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
> dev_dbg(dev, "failed to create device link to %s\n",
> dev_name(phy->dev.parent));
>
> +out_unlock:
> + mutex_unlock(&phy_provider_mutex);
> return phy;
> }
> EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index);
>
> --
> 2.55.0
>
More information about the linux-phy
mailing list