Re: [PATCH v15 1/4] phy: core: Fix use-after-free in phy_get paths

---
 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