This patch set checks that lists initialized in net_init hooks were
return to initial state at end of net_exit hooks.
I hope such checks allows to detect leaked per-netns objects.
Also I hope that all new pernet_operations will inherit such checks too.
I assume that elements added into per-net lists should not live longer than net namespace,
and should be deleted from the list. I think exit_net hook is good place for such check.
Recently I've found lost list_entry and enabled timer on stop of net namespace.
Then I've reviewed all existing pernet_operations and found that many drivers
have such checks already. So I decided to complete this task and add such checks
into all affected subsystems.
v2:
- net pointer removed from output
- fixed compilation for phonet driver
Vasily Averin (21):
exit_net cleanup: geneve sock_list check
ppp: exit_net cleanup checks added
vxlan: exit_net cleanup checks added
netdev: exit_net cleanup check added
nfs4blocklayout: exit_net cleanup check added
nfs client: exit_net cleanup check added
fib_notifier: exit_net cleanup check added
fib_rules: exit_net cleanup check added
clusterip: exit_net cleanup check added
xfrm6_tunnel: exit_net cleanup check added
af_key: replace BUG_ON on WARN_ON in net_exit hook
l2tp: exit_net cleanup check added
nf_tables: exit_net cleanup check added
nfnetlink_log: exit_net cleanup check added
nfnetlink_gueue: exit_net cleanup check added
x_tables: exit_net cleanup check added
hashlimit: exit_net cleanup check added
recent: exit_net cleanup check added
packet: exit_net cleanup check added
phonet: exit_net cleanup check added
sunrpc: exit_net cleanup check added
drivers/net/geneve.c | 2 ++
drivers/net/ppp/ppp_generic.c | 4 ++++
drivers/net/vxlan.c | 6 ++++++
fs/nfs/blocklayout/rpc_pipefs.c | 2 ++
fs/nfs/client.c | 4 ++++
net/core/dev.c | 3 +++
net/core/fib_notifier.c | 7 +++++++
net/core/fib_rules.c | 7 +++++++
net/ipv4/netfilter/ipt_CLUSTERIP.c | 2 ++
net/ipv6/xfrm6_tunnel.c | 12 ++++++++++++
net/key/af_key.c | 2 +-
net/l2tp/l2tp_core.c | 6 ++++++
net/netfilter/nf_tables_api.c | 9 +++++++++
net/netfilter/nfnetlink_log.c | 6 ++++++
net/netfilter/nfnetlink_queue.c | 7 +++++++
net/netfilter/x_tables.c | 10 ++++++++++
net/netfilter/xt_hashlimit.c | 4 ++++
net/netfilter/xt_recent.c | 4 ++++
net/packet/af_packet.c | 2 ++
net/phonet/pn_dev.c | 4 ++++
net/sunrpc/sunrpc_syms.c | 4 ++++
21 files changed, 106 insertions(+), 1 deletion(-)
--
2.7.4
@@ -1673,6 +1673,8 @@ static void __net_exit geneve_exit_net(struct net *net)/* unregister the devices gathered above */unregister_netdevice_many(&list);rtnl_unlock();+WARN(!list_empty(&gn->sock_list),+"&s: sock_list is not empty\n",__func__);}staticstructpernet_operationsgeneve_net_ops={
@@ -960,6 +960,10 @@ static __net_exit void ppp_exit_net(struct net *net)rtnl_unlock();idr_destroy(&pn->units_idr);+WARN(!list_empty(&pn->all_channels),+"%s: all_channels list is not empty\n",__func__);+WARN(!list_empty(&pn->new_channels),+"%s: new_channels list is not empty\n",__func__);}staticstructpernet_operationsppp_net_ops={
Be sure that sock_list array initialized in net_init hook was return
to initial state
Signed-off-by: Vasily Averin <redacted>
---
drivers/net/vxlan.c | 6 ++++++
1 file changed, 6 insertions(+)
@@ -3704,6 +3704,7 @@ static void __net_exit vxlan_exit_net(struct net *net)structvxlan_net*vn=net_generic(net,vxlan_net_id);structvxlan_dev*vxlan,*next;structnet_device*dev,*aux;+unsignedinth;LIST_HEAD(list);rtnl_lock();
@@ -3723,6 +3724,11 @@ static void __net_exit vxlan_exit_net(struct net *net)unregister_netdevice_many(&list);rtnl_unlock();++for(h=0;h<PORT_HASH_SIZE;++h)+if(WARN(!hlist_empty(&vn->sock_list[h]),+"%s: sock_list is not empty\n",__func__))+break;}staticstructpernet_operationsvxlan_net_ops={
Be sure that dev_base_head list initialized in net_init hook was return
to initial state
Signed-off-by: Vasily Averin <redacted>
---
net/core/dev.c | 3 +++
1 file changed, 3 insertions(+)
@@ -8562,6 +8562,9 @@ static void __net_exit netdev_exit(struct net *net){kfree(net->dev_name_head);kfree(net->dev_index_head);+if(net!=&init_net)+WARN(!list_empty(&net->dev_base_head),+"%s: dev_base_head is not empty\n",__func__);}staticstructpernet_operations__net_initdatanetdev_net_ops={
Be sure that bl_wq wait queue initialized in net_init hook
is not used anymore.
Signed-off-by: Vasily Averin <redacted>
---
fs/nfs/blocklayout/rpc_pipefs.c | 2 ++
1 file changed, 2 insertions(+)
@@ -256,6 +256,8 @@ static void nfs4blocklayout_net_exit(struct net *net)nfs4blocklayout_unregister_net(net,nn->bl_device_pipe);rpc_destroy_pipe_data(nn->bl_device_pipe);nn->bl_device_pipe=NULL;+WARN(!list_empty(&nn->bl_wq.head),+"%s: bl_wq head is not empty\n",__func__);}staticstructpernet_operationsnfs4blocklayout_net_ops={
Be sure that fib_notifier_ops list initilized in net_init hook was return
to initial state.
Signed-off-by: Vasily Averin <redacted>
---
net/core/fib_notifier.c | 7 +++++++
1 file changed, 7 insertions(+)
@@ -161,8 +161,15 @@ static int __net_init fib_notifier_net_init(struct net *net)return0;}+staticvoid__net_exitfib_notifier_net_exit(structnet*net)+{+WARN(!list_empty(&net->fib_notifier_ops),+"%s: fib_notifier_ops list is not empty\n",__func__);+}+staticstructpernet_operationsfib_notifier_net_ops={.init=fib_notifier_net_init,+.exit=fib_notifier_net_exit,};staticint__initfib_notifier_init(void)
Be sure that rules_ops list initialized in net_init hook was return
to initial state.
Signed-off-by: Vasily Averin <redacted>
---
net/core/fib_rules.c | 7 +++++++
1 file changed, 7 insertions(+)
@@ -1019,8 +1019,15 @@ static int __net_init fib_rules_net_init(struct net *net)return0;}+staticvoid__net_exitfib_rules_net_exit(structnet*net)+{+WARN(!list_empty(&net->rules_ops),+"%s: rules_ops list is not empty\n",__func__);+}+staticstructpernet_operationsfib_rules_net_ops={.init=fib_rules_net_init,+.exit=fib_rules_net_exit,};staticint__initfib_rules_init(void)
Be sure that configs list initialized in net_init hook was return
to initial state.
Signed-off-by: Vasily Averin <redacted>
---
net/ipv4/netfilter/ipt_CLUSTERIP.c | 2 ++
1 file changed, 2 insertions(+)
@@ -819,6 +819,8 @@ static void clusterip_net_exit(struct net *net)cn->procdir=NULL;#endifnf_unregister_net_hook(net,&cip_arp_ops);+WARN(!list_empty(&cn->configs),+"%s: configs list is not empty\n"i,__func__);}staticstructpernet_operationsclusterip_net_ops={
Be sure that spi_byaddr and spi_byspi arrays initialized in net_init hook
were return to initial state
Signed-off-by: Vasily Averin <redacted>
---
net/ipv6/xfrm6_tunnel.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
@@ -338,6 +338,18 @@ static int __net_init xfrm6_tunnel_net_init(struct net *net)staticvoid__net_exitxfrm6_tunnel_net_exit(structnet*net){+structxfrm6_tunnel_net*xfrm6_tn=xfrm6_tunnel_pernet(net);+unsignedinti;++for(i=0;i<XFRM6_TUNNEL_SPI_BYADDR_HSIZE;i++)+if(WARN(!hlist_empty(&xfrm6_tn->spi_byaddr[i]),+"%s: spi_byaddr is not empty\n",__func__))+break;++for(i=0;i<XFRM6_TUNNEL_SPI_BYSPI_HSIZE;i++)+if(WARN(!hlist_empty(&xfrm6_tn->spi_byspi[i]),+"%s: spi_byspi is not empty\n",__func__))+break;}staticstructpernet_operationsxfrm6_tunnel_net_ops={
@@ -1888,6 +1888,7 @@ static __net_exit void l2tp_exit_net(struct net *net){structl2tp_net*pn=l2tp_pernet(net);structl2tp_tunnel*tunnel=NULL;+inthash;rcu_read_lock_bh();list_for_each_entry_rcu(tunnel,&pn->l2tp_tunnel_list,list){
@@ -1897,6 +1898,11 @@ static __net_exit void l2tp_exit_net(struct net *net)flush_workqueue(l2tp_wq);rcu_barrier();++for(hash=0;hash<L2TP_HASH_SIZE_2;hash++)+if(WARN(!hlist_empty(&pn->l2tp_session_hlist[hash]),+"%s: session_hlist is not empty\n",__func__))+break;}staticstructpernet_operationsl2tp_net_ops={
@@ -1093,10 +1093,16 @@ static int __net_init nfnl_log_net_init(struct net *net)staticvoid__net_exitnfnl_log_net_exit(structnet*net){+unsignedinti;+structnfnl_log_net*log=nfnl_log_pernet(net);#ifdef CONFIG_PROC_FSremove_proc_entry("nfnetlink_log",net->nf.proc_netfilter);#endifnf_log_unset(net,&nfulnl_logger);+for(i=0;i<INSTANCE_BUCKETS;i++)+if(WARN(!hlist_empty(&log->instance_table[i]),+"%s: instance_table is not empty\n",__func__))+break;}staticstructpernet_operationsnfnl_log_net_ops={
@@ -5778,6 +5778,14 @@ static int __net_init nf_tables_init_net(struct net *net)return0;}+staticvoid__net_exitnf_tables_exit_net(structnet*net)+{+WARN(!list_empty(&net->nft.af_info),+"%s: af_info list is not empty\n",__func__);+WARN(!list_empty(&net->nft.commit_list),+"%s: commit_list is not empty\n",__func__);+}+int__nft_release_basechain(structnft_ctx*ctx){structnft_rule*rule,*nr;
@@ -1714,8 +1714,18 @@ static int __net_init xt_net_init(struct net *net)return0;}+staticvoid__net_exitxt_net_exit(structnet*net)+{+inti;++for(i=0;i<NFPROTO_NUMPROTO;i++)+WARN(!list_empty(&net->xt.tables[i]),+"%s: tables list is not empty\n",__func__);+}+staticstructpernet_operationsxt_net_ops={.init=xt_net_init,+.exit=xt_net_exit,};staticint__initxt_init(void)
@@ -1338,7 +1338,11 @@ static int __net_init hashlimit_net_init(struct net *net)staticvoid__net_exithashlimit_net_exit(structnet*net){+structhashlimit_net*hashlimit_net=hashlimit_pernet(net);+hashlimit_proc_net_exit(net);+WARN(!hlist_empty(&hashlimit_net->htables),+"%s: htables hlist is not empty\n",__func__);}staticstructpernet_operationshashlimit_net_ops={
Be sure that tables list initialized in net_init hook was return
to initial state.
Signed-off-by: Vasily Averin <redacted>
---
net/netfilter/xt_recent.c | 4 ++++
1 file changed, 4 insertions(+)
@@ -681,7 +681,11 @@ static int __net_init recent_net_init(struct net *net)staticvoid__net_exitrecent_net_exit(structnet*net){+structrecent_net*recent_net=recent_pernet(net);+recent_proc_net_exit(net);+WARN(!list_empty(&recent_net->tables),+"%s: tables list is not empty\n",__func__);}staticstructpernet_operationsrecent_net_ops={
@@ -4562,6 +4562,8 @@ static int __net_init packet_net_init(struct net *net)staticvoid__net_exitpacket_net_exit(structnet*net){remove_proc_entry("packet",net->proc_net);+WARN(!hlist_empty(&net->packet.sklist),+"%s: sklist is not empty\n",__func__);}staticstructpernet_operationspacket_net_ops={
@@ -331,7 +331,11 @@ static int __net_init phonet_init_net(struct net *net)staticvoid__net_exitphonet_exit_net(structnet*net){+structphonet_net*pnn=phonet_pernet(net);+remove_proc_entry("phonet",net->proc_net);+WARN(!list_empty(&pnn->pndevs.list),+"%s: pndevs.list is not empty\n",__func__);}staticstructpernet_operationsphonet_net_ops={
This patch set checks that lists initialized in net_init hooks were
return to initial state at end of net_exit hooks.
I hope such checks allows to detect leaked per-netns objects.
Also I hope that all new pernet_operations will inherit such checks too.
I assume that elements added into per-net lists should not live longer than net namespace,
and should be deleted from the list. I think exit_net hook is good place for such check.
Recently I've found lost list_entry and enabled timer on stop of net namespace.
Then I've reviewed all existing pernet_operations and found that many drivers
have such checks already. So I decided to complete this task and add such checks
into all affected subsystems.
v3:
- use net->ns.inum as net Id
- removed patches for hashlimit and recent,
they handle tables list in exit_net hook.
- added patches for grace and lockd
v2:
- net pointer removed from output
- fixed compilation for phonet driver
Vasily Averin (21):
grace: replace BUG_ON by WARN_ONCE in exit_net hook
lockd: added cleanup checks in exit_net hook
exit_net cleanup: geneve sock_list check
ppp: exit_net cleanup checks added
vxlan: exit_net cleanup checks added
netdev: exit_net cleanup check added
nfs4blocklayout: exit_net cleanup check added
nfs client: exit_net cleanup check added
fib_notifier: exit_net cleanup check added
fib_rules: exit_net cleanup check added
clusterip: exit_net cleanup check added
xfrm6_tunnel: exit_net cleanup check added
af_key: replace BUG_ON on WARN_ONCE in net_exit hook
l2tp: exit_net cleanup check added
nf_tables: exit_net cleanup check added
nfnetlink_log: exit_net cleanup check added
nfnetlink_gueue: exit_net cleanup check added
x_tables: exit_net cleanup check added
packet: exit_net cleanup check added
phonet: exit_net cleanup check added
sunrpc: exit_net cleanup check added
drivers/net/geneve.c | 3 +++
drivers/net/ppp/ppp_generic.c | 6 ++++++
drivers/net/vxlan.c | 7 +++++++
fs/lockd/svc.c | 11 +++++++++++
fs/nfs/blocklayout/rpc_pipefs.c | 3 +++
fs/nfs/client.c | 4 ++++
fs/nfs_common/grace.c | 4 +++-
net/core/dev.c | 4 ++++
net/core/fib_notifier.c | 8 ++++++++
net/core/fib_rules.c | 8 ++++++++
net/ipv4/netfilter/ipt_CLUSTERIP.c | 3 +++
net/ipv6/xfrm6_tunnel.c | 14 ++++++++++++++
net/key/af_key.c | 4 +++-
net/l2tp/l2tp_core.c | 7 +++++++
net/netfilter/nf_tables_api.c | 11 +++++++++++
net/netfilter/nfnetlink_log.c | 7 +++++++
net/netfilter/nfnetlink_queue.c | 8 ++++++++
net/netfilter/x_tables.c | 12 ++++++++++++
net/packet/af_packet.c | 2 ++
net/phonet/pn_dev.c | 5 +++++
net/sunrpc/sunrpc_syms.c | 5 +++++
21 files changed, 134 insertions(+), 2 deletions(-)
--
2.7.4
@@ -690,6 +690,17 @@ static int lockd_init_net(struct net *net)staticvoidlockd_exit_net(structnet*net){+structlockd_net*ln=net_generic(net,lockd_net_id);++WARN_ONCE(!list_empty(&ln->lockd_manager.list),+"net %x %s: lockd_manager.list is not empty\n",+net->ns.inum,__func__);+WARN_ONCE(!list_empty(&ln->nsm_handles),+"net %x %s: nsm_handles list is not empty\n",+net->ns.inum,__func__);+WARN_ONCE(delayed_work_pending(&ln->grace_period_end),+"net %x %s: grace_period_end was not cancelled\n",+net->ns.inum,__func__);}staticstructpernet_operationslockd_net_ops={
@@ -960,6 +960,12 @@ static __net_exit void ppp_exit_net(struct net *net)rtnl_unlock();idr_destroy(&pn->units_idr);+WARN_ONCE(!list_empty(&pn->all_channels),+"net %x %s: all_channels list is not empty\n",+net->ns.inum,__func__);+WARN_ONCE(!list_empty(&pn->new_channels),+"net %x %s: new_channels list is not empty\n",+net->ns.inum,__func__);}staticstructpernet_operationsppp_net_ops={
Be sure that sock_list array initialized in net_init hook was return
to initial state
Signed-off-by: Vasily Averin <redacted>
---
drivers/net/vxlan.c | 7 +++++++
1 file changed, 7 insertions(+)
Be sure that dev_base_head list initialized in net_init hook was return
to initial state
Signed-off-by: Vasily Averin <redacted>
---
net/core/dev.c | 4 ++++
1 file changed, 4 insertions(+)
Be sure that nfs_client_list and nfs_volume_list lists initialized
in net_init hook were return to initial state in net_exit hook.
Signed-off-by: Vasily Averin <redacted>
---
fs/nfs/client.c | 4 ++++
1 file changed, 4 insertions(+)
@@ -204,6 +204,10 @@ void nfs_cleanup_cb_ident_idr(struct net *net)structnfs_net*nn=net_generic(net,nfs_net_id);idr_destroy(&nn->cb_ident_idr);+WARN_ONCE(!list_empty(&nn->nfs_client_list),+"net %x exit: nfs_client_list is not empty\n",net->ns.inum);+WARN_ONCE(!list_empty(&nn->nfs_volume_list),+"net %x exit: nfs_volume_list is not empty\n",net->ns.inum);}/* nfs_client_lock held */
Be sure that fib_notifier_ops list initilized in net_init hook was return
to initial state.
Signed-off-by: Vasily Averin <redacted>
---
net/core/fib_notifier.c | 8 ++++++++
1 file changed, 8 insertions(+)
@@ -161,8 +161,16 @@ static int __net_init fib_notifier_net_init(struct net *net)return0;}+staticvoid__net_exitfib_notifier_net_exit(structnet*net)+{+WARN_ONCE(!list_empty(&net->fib_notifier_ops),+"net %x %s: fib_notifier_ops list is not empty\n",+net->ns.inum,__func__);+}+staticstructpernet_operationsfib_notifier_net_ops={.init=fib_notifier_net_init,+.exit=fib_notifier_net_exit,};staticint__initfib_notifier_init(void)
Be sure that rules_ops list initialized in net_init hook was return
to initial state.
Signed-off-by: Vasily Averin <redacted>
---
net/core/fib_rules.c | 8 ++++++++
1 file changed, 8 insertions(+)
@@ -1019,8 +1019,16 @@ static int __net_init fib_rules_net_init(struct net *net)return0;}+staticvoid__net_exitfib_rules_net_exit(structnet*net)+{+WARN_ONCE(!list_empty(&net->rules_ops),+"net %x %s: rules_ops list is not empty\n",+net->ns.inum,__func__);+}+staticstructpernet_operationsfib_rules_net_ops={.init=fib_rules_net_init,+.exit=fib_rules_net_exit,};staticint__initfib_rules_init(void)
Be sure that configs list initialized in net_init hook was return
to initial state.
Signed-off-by: Vasily Averin <redacted>
---
net/ipv4/netfilter/ipt_CLUSTERIP.c | 3 +++
1 file changed, 3 insertions(+)
@@ -819,6 +819,9 @@ static void clusterip_net_exit(struct net *net)cn->procdir=NULL;#endifnf_unregister_net_hook(net,&cip_arp_ops);+WARN_ONCE(!list_empty(&cn->configs),+"net %x %s: configs list is not empty\n",+net->ns.inum,__func__);}staticstructpernet_operationsclusterip_net_ops={
Be sure that spi_byaddr and spi_byspi arrays initialized in net_init hook
were return to initial state
Signed-off-by: Vasily Averin <redacted>
---
net/ipv6/xfrm6_tunnel.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
@@ -338,6 +338,20 @@ static int __net_init xfrm6_tunnel_net_init(struct net *net)staticvoid__net_exitxfrm6_tunnel_net_exit(structnet*net){+structxfrm6_tunnel_net*xfrm6_tn=xfrm6_tunnel_pernet(net);+unsignedinti;++for(i=0;i<XFRM6_TUNNEL_SPI_BYADDR_HSIZE;i++)+if(WARN_ONCE(!hlist_empty(&xfrm6_tn->spi_byaddr[i]),+"net %x %s: spi_byaddr is not empty\n",+net->ns.inum,__func__))+break;++for(i=0;i<XFRM6_TUNNEL_SPI_BYSPI_HSIZE;i++)+if(WARN_ONCE(!hlist_empty(&xfrm6_tn->spi_byspi[i]),+"net %x %s: spi_byspi is not empty\n",+net->ns.inum,__func__))+break;}staticstructpernet_operationsxfrm6_tunnel_net_ops={
@@ -5778,6 +5778,16 @@ static int __net_init nf_tables_init_net(struct net *net)return0;}+staticvoid__net_exitnf_tables_exit_net(structnet*net)+{+WARN_ONCE(!list_empty(&net->nft.af_info),+"net %x %s: af_info list is not empty\n",+net->ns.inum,__func__);+WARN_ONCE(!list_empty(&net->nft.commit_list),+"net %x %s: commit_list is not empty\n",+net->ns.inum,__func__);+}+int__nft_release_basechain(structnft_ctx*ctx){structnft_rule*rule,*nr;
@@ -1093,10 +1093,17 @@ static int __net_init nfnl_log_net_init(struct net *net)staticvoid__net_exitnfnl_log_net_exit(structnet*net){+unsignedinti;+structnfnl_log_net*log=nfnl_log_pernet(net);#ifdef CONFIG_PROC_FSremove_proc_entry("nfnetlink_log",net->nf.proc_netfilter);#endifnf_log_unset(net,&nfulnl_logger);+for(i=0;i<INSTANCE_BUCKETS;i++)+if(WARN_ONCE(!hlist_empty(&log->instance_table[i]),+"net %x %s: instance_table is not empty\n",+net->ns.inum,__func__))+break;}staticstructpernet_operationsnfnl_log_net_ops={
@@ -1714,8 +1714,20 @@ static int __net_init xt_net_init(struct net *net)return0;}+staticvoid__net_exitxt_net_exit(structnet*net)+{+inti;++for(i=0;i<NFPROTO_NUMPROTO;i++)+if(WARN_ONCE(!list_empty(&net->xt.tables[i]),+"net %x %s: tables list is not empty\n",+net->ns.inum.__func__)+break;+}+staticstructpernet_operationsxt_net_ops={.init=xt_net_init,+.exit=xt_net_exit,};staticint__initxt_init(void)
@@ -4562,6 +4562,8 @@ static int __net_init packet_net_init(struct net *net)staticvoid__net_exitpacket_net_exit(structnet*net){remove_proc_entry("packet",net->proc_net);+WARN_ONCE(!hlist_empty(&net->packet.sklist),+"net %x %s: sklist is not empty\n",net->ns.inum,__func__);}staticstructpernet_operationspacket_net_ops={
@@ -331,7 +331,12 @@ static int __net_init phonet_init_net(struct net *net)staticvoid__net_exitphonet_exit_net(structnet*net){+structphonet_net*pnn=phonet_pernet(net);+remove_proc_entry("phonet",net->proc_net);+WARN_ONCE(!list_empty(&pnn->pndevs.list),+"net %x %s: pndevs.list is not empty\n",+net->ns.inum,__func__);}staticstructpernet_operationsphonet_net_ops={
Be sure that all_clients list initialized in net_init hook was return
to initial state.
Signed-off-by: Vasily Averin <redacted>
---
net/sunrpc/sunrpc_syms.c | 5 +++++
1 file changed, 5 insertions(+)
@@ -65,10 +65,15 @@ static __net_init int sunrpc_init_net(struct net *net)static__net_exitvoidsunrpc_exit_net(structnet*net){+structsunrpc_net*sn=net_generic(net,sunrpc_net_id);+rpc_pipefs_exit_net(net);unix_gid_cache_destroy(net);ip_map_cache_destroy(net);rpc_proc_exit(net);+WARN_ONCE(!list_empty(&sn->all_clients),+"net %x %s: all_clients list is not empty\n",+net->ns.inum,__func__);}staticstructpernet_operationssunrpc_net_ops={
From: walter harms <hidden> Date: 2017-11-06 13:34:36
Hello Vasily Averin,
just a general hint:
when you send new versions of a patch please document also
what you have changed. Here an example from an other ML:
The problematic code looks like this:
res_seq = res_hdr->xd_hdr.length_sn & TB_XDOMAIN_SN_MASK;
res_seq >>= TB_XDOMAIN_SN_SHIFT;
TB_XDOMAIN_SN_SHIFT is 27, and right shifting a u8 27 bits is always
going to result in zero. The fix is to declare these variables as u32.
Fixes: d1ff70241a27 ("thunderbolt: Add support for XDomain discovery protocol")
Signed-off-by: Dan Carpenter <redacted>
---
v2: I accidentally sent this through the wrong list, so I'm resending to
netdev. Also Mika asked me to split it up because the Fixes tags
are different for these patches.
please notice the V2. that tell the reader what has changes against
the V1.
re,
wh
Am 06.11.2017 14:23, schrieb Vasily Averin:
quoted hunk
Be sure that lists initialized in net_init hook were return
to initial state.
Signed-off-by: Vasily Averin <redacted>
---
drivers/net/ppp/ppp_generic.c | 6 ++++++
1 file changed, 6 insertions(+)
@@ -960,6 +960,12 @@ static __net_exit void ppp_exit_net(struct net *net)rtnl_unlock();idr_destroy(&pn->units_idr);+WARN_ONCE(!list_empty(&pn->all_channels),+"net %x %s: all_channels list is not empty\n",+net->ns.inum,__func__);+WARN_ONCE(!list_empty(&pn->new_channels),+"net %x %s: new_channels list is not empty\n",+net->ns.inum,__func__);}staticstructpernet_operationsppp_net_ops={
Hello Vasily,
On Mon, 6 Nov 2017, Vasily Averin wrote:
Be sure that configs list initialized in net_init hook was return
to initial state.
What is the goal of the patch series you sent in the third version in a
row?
- If the deinitializations are missing from the files, the patches
do not fix them, just emit warnings.
- If the deinitializations are not missing, the patches are totally
unnecessary.
It looks like debugging... but not expressed that way, neither in the
subject lines nor in the patch descriptions.
Best regards,
Jozsef
@@ -819,6 +819,9 @@ static void clusterip_net_exit(struct net *net)cn->procdir=NULL;#endifnf_unregister_net_hook(net,&cip_arp_ops);+WARN_ONCE(!list_empty(&cn->configs),+"net %x %s: configs list is not empty\n",+net->ns.inum,__func__);}staticstructpernet_operationsclusterip_net_ops={
--
2.7.4
-
E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
H-1525 Budapest 114, POB. 49, Hungary
@@ -331,7 +331,12 @@ static int __net_init phonet_init_net(struct net *net)staticvoid__net_exitphonet_exit_net(structnet*net){+structphonet_net*pnn=phonet_pernet(net);+remove_proc_entry("phonet",net->proc_net);+WARN_ONCE(!list_empty(&pnn->pndevs.list),+"net %x %s: pndevs.list is not empty\n",+net->ns.inum,__func__);}staticstructpernet_operationsphonet_net_ops={
In my opinon, tis is still utterly pointless. Really, what bug did this
specific patch help to fix?
If you want to debug network namespaces, I have a feeling that the network
namespace code is a better place to do so than individual protocol stacks.
--
雷米‧德尼-库尔蒙
https://www.remlab.net/
From: kbuild test robot <hidden> Date: 2017-11-07 02:38:30
Hi Vasily,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net/master]
[also build test WARNING on v4.14-rc8 next-20171106]
[cannot apply to net-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Vasily-Averin/exit_net-checks-for-objects-initialized-in-net_init-hook/20171107-094512
config: i386-randconfig-x001-201745 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from arch/x86/include/asm/bug.h:82:0,
from include/linux/bug.h:5,
from include/linux/thread_info.h:12,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:81,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/stat.h:19,
from include/linux/module.h:10,
from drivers/net/geneve.c:14:
drivers/net/geneve.c: In function 'geneve_exit_net':
quoted
drivers/net/geneve.c:1671:7: warning: too many arguments for format [-Wformat-extra-args]
"&s: sock_list is not empty\n", __func__);
^
include/asm-generic/bug.h:94:43: note: in definition of macro '__WARN_printf'
#define __WARN_printf(arg...) do { printk(arg); __WARN(); } while (0)
^~~
quoted
drivers/net/geneve.c:1670:2: note: in expansion of macro 'WARN'
WARN(!list_empty(&gn->sock_list),
^~~~
vim +1671 drivers/net/geneve.c
1643
1644 static void __net_exit geneve_exit_net(struct net *net)
1645 {
1646 struct geneve_net *gn = net_generic(net, geneve_net_id);
1647 struct geneve_dev *geneve, *next;
1648 struct net_device *dev, *aux;
1649 LIST_HEAD(list);
1650
1651 rtnl_lock();
1652
1653 /* gather any geneve devices that were moved into this ns */
1654 for_each_netdev_safe(net, dev, aux)
1655 if (dev->rtnl_link_ops == &geneve_link_ops)
1656 unregister_netdevice_queue(dev, &list);
1657
1658 /* now gather any other geneve devices that were created in this ns */
1659 list_for_each_entry_safe(geneve, next, &gn->geneve_list, next) {
1660 /* If geneve->dev is in the same netns, it was already added
1661 * to the list by the previous loop.
1662 */
1663 if (!net_eq(dev_net(geneve->dev), net))
1664 unregister_netdevice_queue(geneve->dev, &list);
1665 }
1666
1667 /* unregister the devices gathered above */
1668 unregister_netdevice_many(&list);
1669 rtnl_unlock();
1670 WARN(!list_empty(&gn->sock_list),
1671 "&s: sock_list is not empty\n", __func__);
From: kbuild test robot <hidden> Date: 2017-11-07 02:42:32
Hi Vasily,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
[also build test ERROR on v4.14-rc8 next-20171106]
[cannot apply to net-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Vasily-Averin/exit_net-checks-for-objects-initialized-in-net_init-hook/20171107-094512
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All error/warnings (new ones prefixed by >>):
In file included from ./arch/xtensa/include/generated/asm/bug.h:1:0,
from include/linux/bug.h:5,
from include/linux/thread_info.h:12,
from include/asm-generic/preempt.h:5,
from ./arch/xtensa/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:81,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/stat.h:19,
from include/linux/module.h:10,
from net/ipv4//netfilter/ipt_CLUSTERIP.c:13:
net/ipv4//netfilter/ipt_CLUSTERIP.c: In function 'clusterip_net_exit':
quoted
net/ipv4//netfilter/ipt_CLUSTERIP.c:823:40: error: expected ')' before 'i'
"%s: configs list is not empty\n"i, __func__);
^
include/asm-generic/bug.h:89:69: note: in definition of macro '__WARN_printf'
#define __WARN_printf(arg...) warn_slowpath_fmt(__FILE__, __LINE__, arg)
^
quoted
net/ipv4//netfilter/ipt_CLUSTERIP.c:822:2: note: in expansion of macro 'WARN'
WARN(!list_empty(&cn->configs),
^
quoted
net/ipv4//netfilter/ipt_CLUSTERIP.c:823:40: warning: format '%s' expects a matching 'char *' argument [-Wformat=]
"%s: configs list is not empty\n"i, __func__);
^
include/asm-generic/bug.h:89:69: note: in definition of macro '__WARN_printf'
#define __WARN_printf(arg...) warn_slowpath_fmt(__FILE__, __LINE__, arg)
^
quoted
net/ipv4//netfilter/ipt_CLUSTERIP.c:822:2: note: in expansion of macro 'WARN'
WARN(!list_empty(&cn->configs),
^
vim +823 net/ipv4//netfilter/ipt_CLUSTERIP.c
813
814 static void clusterip_net_exit(struct net *net)
815 {
816 #ifdef CONFIG_PROC_FS
817 struct clusterip_net *cn = net_generic(net, clusterip_net_id);
818 proc_remove(cn->procdir);
819 cn->procdir = NULL;
820 #endif
821 nf_unregister_net_hook(net, &cip_arp_ops);
> 822 WARN(!list_empty(&cn->configs),
> 823 "%s: configs list is not empty\n"i, __func__);
824 }
825
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
@@ -690,6 +690,17 @@ static int lockd_init_net(struct net *net)staticvoidlockd_exit_net(structnet*net){+structlockd_net*ln=net_generic(net,lockd_net_id);++WARN_ONCE(!list_empty(&ln->lockd_manager.list),+"net %x %s: lockd_manager.list is not empty\n",+net->ns.inum,__func__);+WARN_ONCE(!list_empty(&ln->nsm_handles),+"net %x %s: nsm_handles list is not empty\n",+net->ns.inum,__func__);+WARN_ONCE(delayed_work_pending(&ln->grace_period_end),+"net %x %s: grace_period_end was not cancelled\n",+net->ns.inum,__func__);}staticstructpernet_operationslockd_net_ops={
OpenVz kernel team have a long history of fighting against namespace-related bugs,
some of them could be excluded by using simple checks described below.
One of typical errors is related to live cycle of namespaces:
usually objects created for some namespace should not live longer than namespace itself.
Such kind of issues can be invisible on usual systems where additional namespaces
are not used, because initial namespaces usually lives forever and never destroyed.
However in systems with namespaces it can lead to memory leaks or to use-after-free.
Both of them are critical for systems with running containers.
As you knows it's quite hard to find the reason of such issues,
especially in rarely-triggered scenarios on production nodes on default kernels
without specially enabled debug settings. Any additional hints can be useful here.
This patch set should help to detect some of these issues.
It is based on assumption that objects initialized in init hook of pernet_operations
should return to initial state until end of exit hook.
Many drivers and subsystems already have such checks, however I've found number
of places where list_empty check would be useful at least as smoke test.
These checks are useful for long-term stable kernels,
they allows to detect problems related to incomplete or incorrectly
backported patches.
Also this patch set replaces BUG_ON in existing checks:
memory leaks and possible memory corruptions are bad of course,
however in many cases they are not fatal
and should not crash production hosts unconditionally.
Changes:
v4:
- excluded grace and lockd patches taken by Bruce Fields
- let's use WARN_ON_ONCE without any extra messages
adobriyan@ is right, output of net Id gives nothing to host admin,
and developers in any case will extract information from core dump
- updated description in cover letter
- dropped nfs4blocklayout patch: waitqueue check does not look useful
- patches was reordered to be per-subsystem grouped
- cover letter should be sent to all people included into cc: of any patches
- minor cosmetic changes in some patches
v3:
- use net->ns.inum as net Id
- removed patches for hashlimit and recent,
they handle tables list in exit_net hook.
- added patches for grace and lockd
v2:
- net pointer removed from output
- fixed compilation for phonet driver
Vasily Averin (18):
af_key: replace BUG_ON on WARN_ON in net_exit hook
geneve: exit_net cleanup check added
packet: exit_net cleanup check added
vxlan: exit_net cleanup checks added
netdev: exit_net cleanup check added
fib_notifier: exit_net cleanup check added
fib_rules: exit_net cleanup check added
l2tp: exit_net cleanup check added
clusterip: exit_net cleanup check added
nf_tables: exit_net cleanup check added
nfnetlink_log: exit_net cleanup check added
nfnetlink_gueue: exit_net cleanup check added
x_tables: exit_net cleanup check added
nfs client: exit_net cleanup check added
sunrpc: exit_net cleanup check added
phonet: exit_net cleanup check added
ppp: exit_net cleanup checks added
xfrm6_tunnel: exit_net cleanup check added
drivers/net/geneve.c | 1 +
drivers/net/ppp/ppp_generic.c | 2 ++
drivers/net/vxlan.c | 5 +++++
fs/nfs/inode.c | 4 ++++
net/core/dev.c | 2 ++
net/core/fib_notifier.c | 6 ++++++
net/core/fib_rules.c | 6 ++++++
net/ipv4/netfilter/ipt_CLUSTERIP.c | 1 +
net/ipv6/xfrm6_tunnel.c | 10 ++++++++++
net/key/af_key.c | 2 +-
net/l2tp/l2tp_core.c | 5 +++++
net/netfilter/nf_tables_api.c | 7 +++++++
net/netfilter/nfnetlink_log.c | 5 +++++
net/netfilter/nfnetlink_queue.c | 6 ++++++
net/netfilter/x_tables.c | 10 ++++++++++
net/packet/af_packet.c | 1 +
net/phonet/pn_dev.c | 3 +++
net/sunrpc/sunrpc_syms.c | 3 +++
18 files changed, 78 insertions(+), 1 deletion(-)
--
2.7.4
@@ -4562,6 +4562,7 @@ static int __net_init packet_net_init(struct net *net)staticvoid__net_exitpacket_net_exit(structnet*net){remove_proc_entry("packet",net->proc_net);+WARN_ON_ONCE(!hlist_empty(&net->packet.sklist));}staticstructpernet_operationspacket_net_ops={
Be sure that sock_list array initialized in net_init hook was return
to initial state
Signed-off-by: Vasily Averin <redacted>
---
drivers/net/vxlan.c | 5 +++++
1 file changed, 5 insertions(+)
Be sure that dev_base_head list initialized in net_init hook was return
to initial state
Signed-off-by: Vasily Averin <redacted>
---
net/core/dev.c | 2 ++
1 file changed, 2 insertions(+)
Be sure that fib_notifier_ops list initilized in net_init hook was return
to initial state.
Signed-off-by: Vasily Averin <redacted>
---
net/core/fib_notifier.c | 6 ++++++
1 file changed, 6 insertions(+)
@@ -161,8 +161,14 @@ static int __net_init fib_notifier_net_init(struct net *net)return0;}+staticvoid__net_exitfib_notifier_net_exit(structnet*net)+{+WARN_ON_ONCE(!list_empty(&net->fib_notifier_ops));+}+staticstructpernet_operationsfib_notifier_net_ops={.init=fib_notifier_net_init,+.exit=fib_notifier_net_exit,};staticint__initfib_notifier_init(void)
Be sure that rules_ops list initialized in net_init hook was return
to initial state.
Signed-off-by: Vasily Averin <redacted>
---
net/core/fib_rules.c | 6 ++++++
1 file changed, 6 insertions(+)
@@ -1019,8 +1019,14 @@ static int __net_init fib_rules_net_init(struct net *net)return0;}+staticvoid__net_exitfib_rules_net_exit(structnet*net)+{+WARN_ON_ONCE(!list_empty(&net->rules_ops));+}+staticstructpernet_operationsfib_rules_net_ops={.init=fib_rules_net_init,+.exit=fib_rules_net_exit,};staticint__initfib_rules_init(void)
Be sure that configs list initialized in net_init hook was return
to initial state.
Signed-off-by: Vasily Averin <redacted>
---
net/ipv4/netfilter/ipt_CLUSTERIP.c | 1 +
1 file changed, 1 insertion(+)
@@ -5778,6 +5778,12 @@ static int __net_init nf_tables_init_net(struct net *net)return0;}+staticvoid__net_exitnf_tables_exit_net(structnet*net)+{+WARN_ON_ONCE(!list_empty(&net->nft.af_info));+WARN_ON_ONCE(!list_empty(&net->nft.commit_list));+}+int__nft_release_basechain(structnft_ctx*ctx){structnft_rule*rule,*nr;
@@ -1714,8 +1714,18 @@ static int __net_init xt_net_init(struct net *net)return0;}+staticvoid__net_exitxt_net_exit(structnet*net)+{+inti;++for(i=0;i<NFPROTO_NUMPROTO;i++)+if(WARN_ON_ONCE(!list_empty(&net->xt.tables[i])))+break;+}+staticstructpernet_operationsxt_net_ops={.init=xt_net_init,+.exit=xt_net_exit,};staticint__initxt_init(void)
@@ -331,7 +331,10 @@ static int __net_init phonet_init_net(struct net *net)staticvoid__net_exitphonet_exit_net(structnet*net){+structphonet_net*pnn=phonet_pernet(net);+remove_proc_entry("phonet",net->proc_net);+WARN_ON_ONCE(!list_empty(&pnn->pndevs.list));}staticstructpernet_operationsphonet_net_ops={
Be sure that spi_byaddr and spi_byspi arrays initialized in net_init hook
were return to initial state
Signed-off-by: Vasily Averin <redacted>
---
net/ipv6/xfrm6_tunnel.c | 10 ++++++++++
1 file changed, 10 insertions(+)
@@ -338,6 +338,16 @@ static int __net_init xfrm6_tunnel_net_init(struct net *net)staticvoid__net_exitxfrm6_tunnel_net_exit(structnet*net){+structxfrm6_tunnel_net*xfrm6_tn=xfrm6_tunnel_pernet(net);+unsignedinti;++for(i=0;i<XFRM6_TUNNEL_SPI_BYADDR_HSIZE;i++)+if(WARN_ON_ONCE(!hlist_empty(&xfrm6_tn->spi_byaddr[i])))+break;++for(i=0;i<XFRM6_TUNNEL_SPI_BYSPI_HSIZE;i++)+if(WARN_ON_ONCE(!hlist_empty(&xfrm6_tn->spi_byspi[i])))+break;}staticstructpernet_operationsxfrm6_tunnel_net_ops={
In my opinon, tis is still utterly pointless. Really, what bug did this
specific patch help to fix?
I'm maintainer of legacy OpenVz kernels, we release containers-ready kernels 15+ years,
they are widely used by hosting providers, usually they use 30-300 containers per node.
In this scenario if any of container causes the problem it affects many other.
For many years we got lot of strange memory corruptions and found lot of memory leaks
in namespace-fied subsytems.
These bugs are invisible on usual kernls, because init_net lives forever and never destroyed.
However they are quite important for systems running lott of independent namespaces,
that can be restarted many times without host admin assistance.
Each memory leak can be repeated many times and finally it enables OOM-killer
that disables whole node. And nobody understand what's happen.
Such kind of checks allows to be sure that net namespace exit was clear and leaked nothing.
If you want to debug network namespaces, I have a feeling that the network
namespace code is a better place to do so than individual protocol stacks.
Common network namespace code knows nothing about specific of each individual driver/subsystem.
I do not understand how it's possible to do it in common netns code.
Thank you,
Vasily Averin
@@ -1093,10 +1093,15 @@ static int __net_init nfnl_log_net_init(struct net *net)staticvoid__net_exitnfnl_log_net_exit(structnet*net){+unsignedinti;+structnfnl_log_net*log=nfnl_log_pernet(net);
Need empty line after the declarations.
#ifdef CONFIG_PROC_FS
remove_proc_entry("nfnetlink_log", net->nf.proc_netfilter);
#endif
nf_log_unset(net, &nfulnl_logger);
+ for (i = 0; i < INSTANCE_BUCKETS; i++)
+ if (WARN_ON_ONCE(!hlist_empty(&log->instance_table[i])))
+ break;
}
OpenVz kernel team have a long history of fighting against namespace-related bugs,
some of them could be prevented by using simple checks described below.
One of typical errors is related to live cycle of namespaces:
usually objects created for some namespace should not live longer than namespace itself.
Such kind of issues can be invisible on usual systems where additional namespaces
are not used, because initial namespaces usually lives forever and never destroyed.
However in systems with namespaces it can lead to memory leaks or to use-after-free.
Both of them are critical for systems with running containers.
As you knows it's quite hard to find the reason of such issues,
especially in rarely-triggered scenarios on production nodes on default kernels
without specially enabled debug settings. Any additional hints can be useful here.
This patch set should help to detect some of these issues.
It is based on assumption that objects initialized in init hook of pernet_operations
should return to initial state until end of exit hook.
Many drivers and subsystems already have such checks, however I've found number
of places where list_empty check would be useful at least as smoke test.
These checks are useful for long-term stable kernels,
they allows to detect problems related to incomplete or incorrectly
backported patches.
Changes:
v5:
- fixed nit pointed by Florian Westphal
- netfilter patches are send separately to netfilter-devel@
v4:
- excluded grace and lockd patches taken by Bruce Fields
- let's use WARN_ON_ONCE without any extra messages
adobriyan@ is right, output of net Id gives nothing to host admin,
and developers in any case will extract information from core dump
- updated description in cover letter
- dropped nfs4blocklayout patch: waitqueue check does not look useful
- patches was reordered to be per-subsystem grouped
- cover letter should be sent to all people included into cc: of any patches
- minor cosmetic changes in some patches
v3:
- use net->ns.inum as net Id
- removed patches for hashlimit and recent,
they handle tables list in exit_net hook.
- added patches for grace and lockd
v2:
- net pointer removed from output
- fixed compilation for phonet driver
Vasily Averin (13):
af_key: replace BUG_ON on WARN_ON in net_exit hook
geneve: exit_net cleanup check added
packet: exit_net cleanup check added
vxlan: exit_net cleanup checks added
netdev: exit_net cleanup check added
fib_notifier: exit_net cleanup check added
fib_rules: exit_net cleanup check added
l2tp: exit_net cleanup check added
nfs client: exit_net cleanup check added
sunrpc: exit_net cleanup check added
phonet: exit_net cleanup check added
ppp: exit_net cleanup checks added
xfrm6_tunnel: exit_net cleanup check added
drivers/net/geneve.c | 1 +
drivers/net/ppp/ppp_generic.c | 2 ++
drivers/net/vxlan.c | 4 ++++
fs/nfs/inode.c | 4 ++++
net/core/dev.c | 2 ++
net/core/fib_notifier.c | 6 ++++++
net/core/fib_rules.c | 6 ++++++
net/ipv6/xfrm6_tunnel.c | 8 ++++++++
net/key/af_key.c | 2 +-
net/l2tp/l2tp_core.c | 4 ++++
net/packet/af_packet.c | 1 +
net/phonet/pn_dev.c | 3 +++
net/sunrpc/sunrpc_syms.c | 3 +++
13 files changed, 45 insertions(+), 1 deletion(-)
--
2.7.4
@@ -4562,6 +4562,7 @@ static int __net_init packet_net_init(struct net *net)staticvoid__net_exitpacket_net_exit(structnet*net){remove_proc_entry("packet",net->proc_net);+WARN_ON_ONCE(!hlist_empty(&net->packet.sklist));}staticstructpernet_operationspacket_net_ops={
Be sure that sock_list array initialized in net_init hook was return
to initial state
Signed-off-by: Vasily Averin <redacted>
---
drivers/net/vxlan.c | 4 ++++
1 file changed, 4 insertions(+)