| Message ID | 20250217203705.40342-1-kuniyu@amazon.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | gtp/geneve: Suppress list_del() splat during ->exit_batch_rtnl(). | expand |
Hello: This series was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 17 Feb 2025 12:37:03 -0800 you wrote: > The common pattern in tunnel device's ->exit_batch_rtnl() is iterating > two netdev lists for each netns: (i) for_each_netdev() to clean up > devices in the netns, and (ii) the device type specific list to clean > up devices in other netns. > > list_for_each_entry(net, net_list, exit_list) { > for_each_netdev_safe(net, dev, next) { > /* (i) call unregister_netdevice_queue(dev, list) */ > } > > [...] Here is the summary with links: - [v1,net,1/2] gtp: Suppress list corruption splat in gtp_net_exit_batch_rtnl(). https://git.kernel.org/netdev/net/c/4ccacf86491d - [v1,net,2/2] geneve: Suppress list corruption splat in geneve_destroy_tunnels(). https://git.kernel.org/netdev/net/c/62fab6eef61f You are awesome, thank you!
The common pattern in tunnel device's ->exit_batch_rtnl() is iterating two netdev lists for each netns: (i) for_each_netdev() to clean up devices in the netns, and (ii) the device type specific list to clean up devices in other netns. list_for_each_entry(net, net_list, exit_list) { for_each_netdev_safe(net, dev, next) { /* (i) call unregister_netdevice_queue(dev, list) */ } list_for_each_entry_safe(xxx, xxx_next, &net->yyy, zzz) { /* (ii) call unregister_netdevice_queue(xxx->dev, list) */ } } Then, ->exit_batch_rtnl() could touch the same device twice. Say we have two netns A & B and device B that is created in netns A and moved to netns B. 1. cleanup_net() processes netns A and then B. 2. ->exit_batch_rtnl() finds the device B while iterating netns A's (ii) [ device B is not yet unlinked from netns B as unregister_netdevice_many() has not been called. ] 3. ->exit_batch_rtnl() finds the device B while iterating netns B's (i) gtp and geneve calls ->dellink() at 2. and 3. that calls list_del() for (ii) and unregister_netdevice_queue(). Calling unregister_netdevice_queue() twice is fine because it uses list_move_tail(), but the 2nd list_del() triggers a splat when CONFIG_DEBUG_LIST is enabled. Possible solution is either of (a) Use list_del_init() in ->dellink() (b) Iterate dev with empty ->unreg_list for (i) like #define for_each_netdev_alive(net, d) \ list_for_each_entry(d, &(net)->dev_base_head, dev_list) \ if (list_empty(&d->unreg_list)) (c) Remove (i) and delegate it to default_device_exit_batch(). This series avoids the 2nd ->dellink() by (c) to suppress the splat for gtp and geneve. Note that IPv4/IPv6 tunnels calls just unregister_netdevice() during ->exit_batch_rtnl() and dev is unlinked from (ii) later in ->ndo_uninit(), so they are safe. Also, pfcp has the same pattern but is safe because unregister_netdevice_many() is called for each netns. Kuniyuki Iwashima (2): gtp: Suppress list corruption splat in gtp_net_exit_batch_rtnl(). geneve: Suppress list corruption splat in geneve_destroy_tunnels(). drivers/net/geneve.c | 7 ------- drivers/net/gtp.c | 5 ----- 2 files changed, 12 deletions(-)