Re: [PATCH net-next v18 02/10] net: phy: phy_link_topology: Track ports in phy_link_topology

[Maxime Chevallier]

On 9/29/26 02:38, netdev-bot+sashiko@xxxxxxxxxx wrote:

> A second path involves MAC drivers that connect the PHY in ndo_open and
> disconnect it in ndo_stop (for example mvpp2 or mvneta via phylink). With
> a module PHY present, taking the link down runs phy_detach_internal(P) ->
> phy_link_topo_del_phy(dev, P), which removes only P's own ports.
> 
> When the module is then unplugged, sfp_sm_phy_detach() -> sfp_remove_phy()
> -> phy_sfp_disconnect_phy() sees P->attached_dev == NULL and skips
> phy_link_topo_del_phy(dev, M). phy_device_remove(M) -> phy_remove() ->
> phy_cleanup_ports() then frees M's ports without erasing them.
> 
> xa_destroy() doesn't dereference the entries, so this commit alone won't
> crash. However, a later patch in this series, "net: ethtool: Introduce
> ethtool command to list ports", adds an ETHTOOL_MSG_PORT_GET dump with no
> admin permission flag. That dump walks xa_for_each(&dev->link_topo->ports).
> Its port_prepare_data() copies port->supported and port->interfaces and
> dereferences port->upstream_port.
> 
> Wouldn't that dump read freed memory? phy_del_port(),
> phy_sfp_disconnect_phy() and phy_link_topo_del_phy() are unchanged at the
> end of the series.
Hmm I just tested and no, the port is correctly cleared...

The phylink_stop() triggers the phy_stop() machinery that takes care
of clearing the ports up

Maxime