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

[Bryan O'Donoghue]

On 11/09/2026 21:56, Frank Li wrote:
> suggested subject:
>
> phy: core: use phy_provider_mutex protect between _of_phy_get and try_module_get()

A pattern I try to encourage and role-model is making fixes very 
explicit and obvious - frequently I'll ask people to prefix their 
patches fixing things with Fix.

I'll split the difference with you since now that I read this submitted 
patch title you're right, it could read better.

"Fix race-condition between _of_phy_get and try_module_get()"
> > 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@xxxxxxxxxxxxxxx
> > Reviewed-by: Loic Poulain<loic.poulain@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Bryan O'Donoghue<bryan.odonoghue@xxxxxxxxxx>
> > ---
> ...
> > @@ -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;
> > +	}
> why no use auto cleanup guard() for mutex lock?

Technically this fix would apply to kernels predating scoped_guard() - 
kernel 6.5 v Fixes: @ ~ 3.13.

Not really sure how far back the cherry-pick will work but I'd like to 
give the possibility.

We can always sweep this file to use scoped_guard() on tip-of-tree later.

---
bod