Re: [PATCH net v2] net: emac: move setting of netops to fix crash

[Nicolai Buchwitz <nb@tipi-net.de>]

Hi Christian

On 21.9.2026 18:28, Christian Lamparter wrote:
> fixes the following crash on driver initialization:
> 
> |BUG: Kernel NULL pointer dereference on read at 0x00000158
> |Faulting instruction address: 0xc0566b40
> |Oops: Kernel access of bad area, sig: 11 [#1]
> |BE PAGE_SIZE=4K  PowerPC 44x Platform
> |Modules linked in:
> |CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Tainted: GW 7.3.0-rc3+ #1
> |Tainted: [W]=WARN
> |Hardware name: MyBook Live APM821XX 0x12c41c83 PowerPC 44x Platform
> |NIP:  c0566b40 LR: c05648f8 CTR: c04c1e9c
> |REGS: c1053a20 TRAP: 0300   Tainted: GW  (7.3.0-rc3+)
> |MSR:  0002b000 <CE,EE,FP,ME>  CR: 24008808  XER: 00000000
> |DEAR: 00000158 ESR: 00000000
> |GPR00: c05648f8 c1053b10 c1063600 c1030000 c5ab3000 00000000 [...]
> |GPR08: 00000002 00000000 00000000 c1053b40 84002808 00000000 [...]
> |GPR16: cfffd210 00000002 c0beafcc cfffc960 00000000 c1030644 [...]
> |GPR24: c0beafbc c1037000 00000000 0000000a 00000000 c1030000 [...]
> |NIP [c0566b40] phy_link_topo_add_phy+0x2c/0x1d0
> |LR [c05648f8] phy_attach_direct+0x1a4/0x368
> |Call Trace:
> |[c1053b10] [c0811e04] klist_put+0x54/0xb4 (unreliable)
> |[c1053b40] [c05648f8] phy_attach_direct+0x1a4/0x368
> |[c1053b70] [c0564ae8] phy_connect_direct+0x2c/0x60
> |[c1053b90] [c056c7a8] of_phy_connect+0x50/0x74
> |[c1053bc0] [c0572a88] emac_probe+0xd50/0x119c
> |[c1053c90] [c04cb770] platform_probe+0x74/0xa4
> |[c1053cb0] [c04c8f08] really_probe+0x120/0x2b0
> |[c1053cd0] [c04c9254] __driver_probe_device+0x1bc/0x1fc
> |[c1053d00] [c04c9334] driver_probe_device+0x38/0xa8
> |[c1053d30] [c04c9560] __driver_attach+0xf4/0x10c
> |[c1053d50] [c04c6af8] bus_for_each_dev+0x68/0xd0
> |[c1053d90] [c04c7c34] bus_add_driver+0xcc/0x1ec
> |[c1053dc0] [c04c9f6c] driver_register+0xcc/0x110
> |[c1053de0] [c0aa3f40] emac_init+0x1c4/0x200
> 
> This bug showed up starting with v7.3-rc1. At the NIP in
> phy_link_topo_add_phy() is a netdev_need_ops_lock() check.
> This was added by the following
> commit ded86da4bbb7 ("net: ethtool: relax ethnl_req_get_phydev() 
> locking assertion")

ded86da4bbb7 was added in v7.2-rc1?

   $ git grep -c netdev_need_ops_lock v7.1 -- 
drivers/net/phy/phy_link_topology.c
   $ git grep -c netdev_need_ops_lock v7.2-rc1 -- 
drivers/net/phy/phy_link_topology.c
   v7.2-rc1:drivers/net/phy/phy_link_topology.c:1

> 
> The bug shows up because at the time of_phy_connect() was called, the
> netdev_ops were *not yet* determined. My fix is to move the code that
> sets netdev_ops+commac.ops+ethtool_ops further up as emac_init_config()
> derives that by looking at the device-tree and sets the required
> dev->phy_mode accordingly.
> 
> During review, the Sashiko bot's AI stated that the commac assignment
> became a dead store. Great catch! To keep the original behavior as-is,
> one mentioned option "set dev->commac.ops = &emac_commac_ops only in
> the non-gige case?" sounded like a great plan. So the dev->commac.ops
> assignment for the non-gige-case moves into the else block.
> 
> This patch was tested on a WD MyBook Live (RGMII). the device now works 
> again.

nit: [...] (RGMII). the device" -> The device

> 
> Fixes: ded86da4bbb7 ("net: ethtool: relax ethnl_req_get_phydev() 
> locking assertion")
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
> 
> v2:
>   - "set dev->commac.ops = &emac_commac_ops only in the non-gige case"
>   - reworded commit message to include commac_ops and Sashiko
> v1: 
> https://lore.kernel.org/all/178999551085.2160803.13630958553679103031@kernel.org/
> ---
> [...]

With the commit message changed:

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks
Nicolai