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

[Xuanqiang Luo]

Hi Andrew,

在 2026/8/25 21:03, Andrew Lunn 写道:

> 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?

I don't have a real use case for unbinding the PHY while keeping the
MAC bound. I found this during code review and used sysfs unbind to
reproduce the crash.

> 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@xxxxxxxxx/
> 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.

Thanks for clarifying. I agree.

> Does this problem happen if you keep to the symmetry?

No. I checked the normal teardown path, and the problem does not occur
when the symmetric teardown order is followed. This is expected.

> Does your fix to this problem make the symmetry worse?

The normal teardown path is unchanged, but the patch allows the
asymmetric state created by a PHY-first unbind to continue with the IRQ
still installed. From that point of view, the fix is wrong.

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

Yes. I agree that this is the right direction. I will stop here and
think more carefully about how to enforce the ordering before posting
another version.

Thanks,
Xuanqiang