Re: [PATCH net 01/10] net: hisilicon: hisi_femac: Move setting of netops to fix crash

[Jijie Shao]

on 2026/10/8 10:26, David Yang wrote:
> hisi_femac_drv_probe() connects the PHY before dev->netdev_ops is
> assigned. phy_attach_direct() -> phy_link_topo_add_phy() reads
> dev->netdev_ops through netdev_need_ops_lock() since the commit in
> question, so probing crashes with a NULL pointer dereference when
> CONFIG_NET_SHAPER is enabled.
>
>    phy_link_topo_add_phy from phy_attach_direct+0xec/0x37c
>    of_phy_get_and_connect from hisi_femac_drv_probe+0x1f4/0x558
>    Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>
> Assign netdev_ops before the PHY attach, as was done for emac in commit
> 7c9f391ec89c ("net: emac: move setting of netops to fix crash").
>
> Fixes: ded86da4bbb7 ("net: ethtool: relax ethnl_req_get_phydev() locking assertion")
> Signed-off-by: David Yang <mmyangfl@xxxxxxxxx>

Reviewed-by: Jijie Shao <shaojijie@xxxxxxxxxx>

If possible, modify this file as well. I have observed similar issues on the hibmcge driver recently:
drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c

There are similar issues:
The driver assigns netdev_ops after hbg_init(), but hbg_init() calls
hbg_mdio_init() -> hbg_phy_connect() -> phy_connect_direct() ->
phy_attach_direct() -> phy_link_topo_add_phy(), which invokes
netdev_need_ops_lock(). With CONFIG_NET_SHAPER enabled, this function
dereferences dev->netdev_ops->net_shaper_ops. Since netdev_ops is still
NULL at this point, a NULL pointer dereference occurs.

Move the netdev_ops assignment before hbg_init() so that
netdev_need_ops_lock() can safely access netdev_ops during PHY
attachment.

Thanks,
Jijie Shao

> ---
>   drivers/net/ethernet/hisilicon/hisi_femac.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/hisilicon/hisi_femac.c b/drivers/net/ethernet/hisilicon/hisi_femac.c
> index d244a40df430..016605e86f0b 100644
> --- a/drivers/net/ethernet/hisilicon/hisi_femac.c
> +++ b/drivers/net/ethernet/hisilicon/hisi_femac.c
> @@ -830,6 +830,8 @@ static int hisi_femac_drv_probe(struct platform_device *pdev)
>   		hisi_femac_phy_reset(priv);
>   	}
>   
> +	ndev->netdev_ops = &hisi_femac_netdev_ops;
> +
>   	phy = of_phy_get_and_connect(ndev, node, hisi_femac_adjust_link);
>   	if (!phy) {
>   		dev_err(dev, "connect to PHY failed!\n");
> @@ -850,7 +852,6 @@ static int hisi_femac_drv_probe(struct platform_device *pdev)
>   
>   	ndev->watchdog_timeo = 6 * HZ;
>   	ndev->priv_flags |= IFF_UNICAST_FLT;
> -	ndev->netdev_ops = &hisi_femac_netdev_ops;
>   	ndev->ethtool_ops = &hisi_femac_ethtools_ops;
>   	netif_napi_add_weight(ndev, &priv->napi, hisi_femac_poll,
>   			      FEMAC_POLL_WEIGHT);