Re: [PATCH net v1] net: phy: fix NULL deref in IRQ handler after unbind

[Andrew Lunn <andrew@lunn.ch>]

On Tue, Aug 25, 2026 at 11:34:04AM +0800, Xuanqiang Luo wrote:
> Hi Andrew,
> 
> 在 2026/8/24 20:54, Andrew Lunn 写道:
> 
> > > The IRQ is requested at attach time and released by phy_disconnect(),
> > > so phy_remove() cannot free it without a later double-free.
> > So this sounds wrong.
> > 
> > If you unbind the PHY, you need to also unbind the MAC, since a MAC
> > without a PHY is useless. When the MAC unloads, it will call
> > phy_remove() so everything unwinds in the correct order.
> > 
> >      Andrew
> > 
> > ---
> > pw-bot: cr
> > 	
> 
> I agree that the MAC should normally be unbound before the PHY.
> 
> Also, the paragraph about freeing the IRQ in phy_remove() was
> misleading. It was not relevant to the change being proposed,
> so I will drop it in v2.
> 
> Do you mean that unbinding the PHY first through sysfs is not a
> supported operation?

To me, unbind is an odd thing to do. What is your use case?

When doing development work, i tend to reboot the target. If not, i
would unload the MAC driver and the PHY driver, to ensure i have a
clean state.

> 
> The same sequence was used to reproduce the issue fixed by commit
> c2b727df7caa ("net: phy: Avoid NPD upon phy_detach() when driver is
> unbound"), which made me think that it should at least not crash:
> 
> https://lore.kernel.org/all/20200917034310.2360488-2-f.fainelli@gmail.com/

A crash is not good.

But we also need to look, is the fix the correct architecturally, or
we are actually making it worse. Drivers have function pairs. A
function which does setup, and a mirror function which does
tairdown. A function to bind to a MAC and a mirror which unbinds from
a MAC. That symmetry makes drivers simple, easy to reason about.

Does this problem happen if you keep to the symmetry?

Does your fix to this problem make the symmetry worse?

If unbind were to cause the MAC to unload, is the symmetry kept?

   Andrew