Thread (10 messages) flat view 10 messages, 3 authors, 12h ago

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

From: sashiko-bot@kernel.org
Date: 2026-09-04 08:48:34

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 [off-list ref]

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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help