Re: [PATCH v2 1/4] phy: core: Add common helper to add phy phandle device link

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but there is a Use-After-Free (UAF) vulnerability in the PHY retrieval paths. Functions like `_of_phy_get()` and `phy_find()` locate a `struct phy` and return a pointer to it without holding any reference counts or locks. The caller then directly dereferences this pointer (e.g., `phy->ops->owner` during `try_module_get()`). If the PHY provider module is concurrently unloaded between the lookup and the dereference, `phy_destroy()` is invoked, freeing the PHY and leading to a Use-After-Free.
--

commit b6f92c42973f2e71067eb982ed22ae38c909d6ea
Author: Inochi Amaoto <inochiama@gmail.com>

phy: core: Add common helper to add phy phandle device link

It is very common for adding a device link for phy phandle
for device managed phy helper functions. So add a common
helper for future reuse.

> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 21aaf2f76e53e..ea79913fbb87f 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c

[ ... ]

> @@ -808,10 +827,7 @@ struct phy *phy_get(struct device *dev, const char *string)
>  
>  	get_device(&phy->dev);
>  

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for the phy
pointer to be freed before we access it in these paths?

Functions like _of_phy_get() and phy_find() locate a struct phy and return a
pointer to it without holding any reference counts or locks.

The caller then directly dereferences this pointer. For example, right before
this diff context in phy_get():

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

If the PHY provider module is concurrently unloaded between the lookup and the
dereference, phy_destroy() could be invoked, freeing the PHY and leading to a
use-after-free.

This same pattern appears in of_phy_get() and devm_of_phy_get_by_index() when
accessing phy->ops->owner. Could a reference be taken on the device or module
while still holding the lookup locks?

> -	link = device_link_add(dev, &phy->dev, DL_FLAG_STATELESS);
> -	if (!link)
> -		dev_dbg(dev, "failed to create device link to %s\n",
> -			dev_name(phy->dev.parent));
> +	phy_add_device_link(dev, phy);
>  
>  	return phy;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904083709.425893-1-inochiama@gmail.com?part=1

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