Re: [PATCH v13 2/5] phy: core: Add devm_phy_get_by_of_node()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] A TOCTOU race condition exists between looking up the PHY and acquiring its module/device references, leading to a Use-After-Free (UAF).
--

--- Patch [2]: [PATCH v13 2/5] phy: core: Add devm_phy_get_by_of_node() ---
commit 65a24031fe496953334387cf190f1425a8b31168
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

phy: core: Add devm_phy_get_by_of_node()

Add a devm variant of phy_get_by_of_node() to allow for the familiar
pattern of having devres automatically release resources on the driver's
exit path.

> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index ebfad5325072e..7af24f2f4120f 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c

[ ... ]

> @@ -1029,6 +1029,40 @@ struct phy *phy_get_by_of_node(struct device_node *np)
>  }
>  EXPORT_SYMBOL_GPL(phy_get_by_of_node);
>  
> +/**
> + * devm_phy_get_by_of_node() - devm managed lookup and obtain phy reference by device node
> + * @dev: device requesting the PHY
> + * @np: device_node of the PHY provider
> + *
> + * Returns phy associated with the device_node or ERR_PTR. devres manages
> + * releasing resources.
> + */
> +struct phy *devm_phy_get_by_of_node(struct device *dev, struct device_node *np)
> +{
> +	struct phy **ptr, *phy;
> +	struct device_link *link;
> +
> +	ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr)
> +		return ERR_PTR(-ENOMEM);
> +
> +	phy = phy_get_by_of_node(np);

[Severity: High]
This is a pre-existing issue, but does a TOCTOU race condition exist in the
underlying phy_get_by_of_node() that could lead to a use-after-free here?

Looking at phy_get_by_of_node(), it calls _of_phy_get_with_args() to look up
the PHY. Inside _of_phy_get_with_args(), the function temporarily elevates the
provider's module refcount, calls of_xlate() (which returns a bare phy
pointer without incrementing the device refcount), and then drops the module
refcount and unlocks phy_provider_mutex before returning the raw phy pointer.

The caller then accesses phy->ops->owner and calls get_device(&phy->dev):

drivers/phy/phy-core.c:phy_get_by_of_node() {
    ...
	phy = _of_phy_get_with_args(&args);

	if (IS_ERR(phy))
		return phy;

	if (!try_module_get(phy->ops->owner))
		return ERR_PTR(-EPROBE_DEFER);

	get_device(&phy->dev);
    ...
}

If the PHY provider module is unloaded or its driver is unbound via sysfs
concurrently in the window between _of_phy_get_with_args() returning and the
try_module_get() call, the phy struct could be unregistered and freed.

Could this cause the subsequent dereferences of phy->ops and phy->dev to
access freed memory?

> +	if (IS_ERR(phy)) {
> +		devres_free(ptr);
> +		return phy;
> +	}
> +
> +	*ptr = phy;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-b4-linux-next-25-03-13-dtsi-x1e80100-camss-v13-0-ae811e2f0799@linaro.org?part=2

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