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

[Xuanqiang Luo <xuanqiang.luo@linux.dev>]

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

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