Re: [RFC net] netdev-genl: skip DEL_NTF on NETDEV_UNREGISTER rebroadcast

[Stanislav Fomichev <sdf.kernel@gmail.com>]

On 10/26, Xiaoguang Wang wrote:
> While netdev_run_todo() waits for the last references to an
> unregistered device, netdev_wait_allrefs_any() resends NETDEV_UNREGISTER
> about once per second. By then the device is no longer on its netns
> device list. Since commit 8a4fc54b07d7 ("net: get rid of
> rtnl_lock_unregistering()"), netns teardown does not wait for such
> devices. So cleanup_net() can run all pernet exit handlers for the
> device's netns while the device still points to it.
> 
> netdev_genl_netdevice_event() handles every NETDEV_UNREGISTER the same
> way. On a rebroadcast it calls genl_has_listeners() on dev_net(dev). If
> genl_pernet_exit() already ran for that netns, net->genl_sock is NULL:
> 
>   CPU0                                 CPU1
>   unregister_netdevice_many_notify()
>     unlist_netdevice(dev)
>     NETDEV_UNREGISTER -> DEL_NTF sent
>     net_set_todo(dev)
>   netdev_run_todo()
>     netdev_wait_allrefs_any()
>                                        cleanup_net(netA)
>                                          default_device_exit_batch()
>                                            (dev not on netA's list)
>                                          genl_pernet_exit(netA)
>                                            netA->genl_sock = NULL
>       NETDEV_UNREGISTER rebroadcast
>         netdev_genl_dev_notify(dev)
>           genl_has_listeners(netA)
>             netlink_has_listeners(NULL)
> 
> If a later cleanup_net() run has already freed the struct net,
> dev_net(dev) is a dangling pointer.
> 
> The splat below came from a 6.6.119-based kernel. The test kept creating
> and destroying SR-IOV VFs on two PFs. It also kept binding and unbinding
> the PCI devices and moving the VFs through short-lived namespaces. The
> VF driver is out of tree:
> 
>   BUG: kernel NULL pointer dereference, address: 0000000000000388
>   RIP: 0010:netlink_has_listeners+0x5/0x60
>   Call Trace:
>    netdev_genl_dev_notify+0x42/0x120
>    netdev_genl_netdevice_event+0x2e/0x60
>    notifier_call_chain+0x56/0xc0
>    netdev_wait_allrefs_any+0x1ac/0x240
>    netdev_run_todo+0x101/0x330
>    jmnd_vnic_remove+0xb1/0x220 [xline_nic]
>    auxiliary_bus_remove+0x18/0x30
>    device_release_driver_internal+0x19b/0x200
>    ...
>    unbind_store+0x9d/0xb0
> 
> Current mainline has the same code paths. This workload has not been run
> on mainline.
> 
> The first NETDEV_UNREGISTER is sent while reg_state is
> NETREG_UNREGISTERING, under RTNL and before netns teardown can pass
> default_device_exit_batch(), so the device-delete notification has
> already gone out by then. netdev_run_todo() sets NETREG_UNREGISTERED
> before it starts waiting, which is how the rebroadcast can be told
> apart. Skip the rebroadcast, as caif_usb and mlxsw already do. This
> also stops a duplicate NETDEV_CMD_DEV_DEL_NTF going to user space every
> second while the device is stuck.
> 
> Fixes: d3d854fd6a1d ("netdev-genl: create a simple family for netdev stuff")
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Xiaoguang Wang <lege.wang@jaguarmicro.com>
> ---
> I'm not a networking developer. I hit this crash in our SR-IOV stress
> testing and am mainly reporting the bug. The change below is the
> simplest fix I could find; if it should be fixed elsewhere, e.g. in
> netdev_wait_allrefs_any(), I'd appreciate guidance or am happy for
> someone more familiar with this code to take it over.

Note, same patch has been reposed here:

https://lore.kernel.org/netdev/20261026221306.3151025-1-namangulati@google.com/