[PATCH v15 1/4] phy: core: Fix use-after-free in phy_get paths
From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Date: 2026-09-04 11:53:07
Also in:
linux-arm-msm, linux-clk, linux-media, linux-phy, lkml, stable
Subsystem:
generic phy framework, the rest · Maintainers:
Vinod Koul, Linus Torvalds
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@vger.kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
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