Re: [PATCH net] net: phy: Warn if phy is attached when removing

[Sean Anderson]

On 8/18/22 1:24 PM, Jakub Kicinski wrote:
> On Tue, 16 Aug 2022 12:37:01 -0400 Sean Anderson wrote:
>> netdevs using phylib can be oopsed from userspace in the following
>> manner:
>> 
>> $ ip link set $iface up
>> $ echo $(basename $(readlink /sys/class/net/$iface/phydev)) > \
>>       /sys/class/net/$iface/phydev/driver/unbind
>> $ ip link set $iface down
>> 
>> However, the traceback provided is a bit too late, since it does not
>> capture the root of the problem (unbinding the driver). It's also
>> possible that the memory has been reallocated if sufficient time passes
>> between when the phy is detached and when the netdev touches the phy
>> (which could result in silent memory corruption). Add a warning at the
>> source of the problem. A future patch could make this more robust by
>> calling dev_close.
> 
> Hm, so we're adding the warning to get more detailed reports "from the
> field"? Guess we've all done that, so fair.

My suspicion is that this case never occurs, since users don't expect to
be able to remove the phy while the interface is running (and so don't
attempt it). If we do end up getting reports of this bug, then we will
need to create a more robust fix. My intention is to take the same
strategy for PCS devices as whatever we do here, since the issue is
analogous.

--Sean