> On 31.05.2019 22:54, Andrew Lunn wrote: >>> It is possible that scheduled work started by the PHY driver is still >>> outstanding when phy_device_remove is called if the PHY was initially >>> started but never connected, and therefore phy_disconnect is never >>> called. phy_stop does not guarantee that the scheduled work is stopped >>> because it is called under rtnl_lock. This can cause an oops due to >>> use-after-free if the delayed work fires after freeing the PHY device. >>> > The patch itself at least shouldn't do any harm. However the justification > isn't fully convincing yet. > PHY drivers don't start any scheduled work. This queue is used by the > phylib state machine. phy_stop usually isn't called under rtnl_lock, > and it calls phy_stop_machine that cancels pending work. > Did you experience such an oops? Can you provide a call chain where > your described scenario could happen?
Upon further investigation, it appears that this change is no longer needed in the mainline. Previously (such as in 4.19 kernels as we are using), phy_stop did not call phy_stop_machine, only phy_disconnect did, so if the phy was started but never connected and disconnected before stopping it, the delayed work was not stopped. That sequence didn't occur often, but could happen in some failure cases which I believe was what I ran into during development when this change was originally made. It looks like this was fixed in commit cbfd12b3e8c3542e8142aa041714ed614d3f67b0 "net: phy: ensure phylib state machine is stopped after calling phy_stop". So my patch can be dropped - but maybe that patch should be added to stable?
