linux-ppp.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/18] exit_net checks for objects initialized in net_init hook
       [not found] <9dfa013e-9098-e155-9c47-a73753338288@virtuozzo.com>
@ 2017-11-12  8:39 ` Vasily Averin
  2017-11-12 19:26   ` [PATCH v5 00/13] " Vasily Averin
  2017-11-12 19:33   ` [PATCH v5 12/13] ppp: exit_net cleanup checks added Vasily Averin
  2017-11-12  8:50 ` [PATCH v4 17/18] " Vasily Averin
  1 sibling, 2 replies; 5+ messages in thread
From: Vasily Averin @ 2017-11-12  8:39 UTC (permalink / raw)
  To: netdev
  Cc: Steffen Klassert, Herbert Xu, Hideaki YOSHIFUJI,
	Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	Trond Myklebust, Anna Schumaker, Remi Denis-Courmont,
	Paul Mackerras, linux-ppp, netfilter-devel, coreteam, linux-nfs

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v4 17/18] ppp: exit_net cleanup checks added
       [not found] <9dfa013e-9098-e155-9c47-a73753338288@virtuozzo.com>
  2017-11-12  8:39 ` [PATCH v4 00/18] exit_net checks for objects initialized in net_init hook Vasily Averin
@ 2017-11-12  8:50 ` Vasily Averin
  1 sibling, 0 replies; 5+ messages in thread
From: Vasily Averin @ 2017-11-12  8:50 UTC (permalink / raw)
  To: netdev; +Cc: linux-ppp, Paul Mackerras

Be sure that lists initialized in net_init hook were return
to initial state.

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 drivers/net/ppp/ppp_generic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index e365866..30744ce 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -960,6 +960,8 @@ static __net_exit void ppp_exit_net(struct net *net)
 	rtnl_unlock();
 
 	idr_destroy(&pn->units_idr);
+	WARN_ON_ONCE(!list_empty(&pn->all_channels));
+	WARN_ON_ONCE(!list_empty(&pn->new_channels));
 }
 
 static struct pernet_operations ppp_net_ops = {
-- 
2.7.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v5 00/13] exit_net checks for objects initialized in net_init hook
  2017-11-12  8:39 ` [PATCH v4 00/18] exit_net checks for objects initialized in net_init hook Vasily Averin
@ 2017-11-12 19:26   ` Vasily Averin
  2017-11-14  6:47     ` David Miller
  2017-11-12 19:33   ` [PATCH v5 12/13] ppp: exit_net cleanup checks added Vasily Averin
  1 sibling, 1 reply; 5+ messages in thread
From: Vasily Averin @ 2017-11-12 19:26 UTC (permalink / raw)
  To: netdev
  Cc: Steffen Klassert, linux-nfs, Trond Myklebust, Anna Schumaker,
	Remi Denis-Courmont, linux-ppp, Paul Mackerras, Herbert Xu,
	Hideaki YOSHIFUJI

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v5 12/13] ppp: exit_net cleanup checks added
  2017-11-12  8:39 ` [PATCH v4 00/18] exit_net checks for objects initialized in net_init hook Vasily Averin
  2017-11-12 19:26   ` [PATCH v5 00/13] " Vasily Averin
@ 2017-11-12 19:33   ` Vasily Averin
  1 sibling, 0 replies; 5+ messages in thread
From: Vasily Averin @ 2017-11-12 19:33 UTC (permalink / raw)
  To: netdev; +Cc: linux-ppp, Paul Mackerras

Be sure that lists initialized in net_init hook were return
to initial state.

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 drivers/net/ppp/ppp_generic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index e365866..30744ce 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -960,6 +960,8 @@ static __net_exit void ppp_exit_net(struct net *net)
 	rtnl_unlock();
 
 	idr_destroy(&pn->units_idr);
+	WARN_ON_ONCE(!list_empty(&pn->all_channels));
+	WARN_ON_ONCE(!list_empty(&pn->new_channels));
 }
 
 static struct pernet_operations ppp_net_ops = {
-- 
2.7.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5 00/13] exit_net checks for objects initialized in net_init hook
  2017-11-12 19:26   ` [PATCH v5 00/13] " Vasily Averin
@ 2017-11-14  6:47     ` David Miller
  0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2017-11-14  6:47 UTC (permalink / raw)
  To: vvs
  Cc: netdev, steffen.klassert, linux-nfs, trond.myklebust,
	anna.schumaker, courmisch, linux-ppp, paulus, herbert, yoshfuji

From: Vasily Averin <vvs@virtuozzo.com>
Date: Sun, 12 Nov 2017 22:26:44 +0300

> 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.

All applied to net-next except patch #9 and #10 which need to go via the
NFS maintainer.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-11-14  6:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <9dfa013e-9098-e155-9c47-a73753338288@virtuozzo.com>
2017-11-12  8:39 ` [PATCH v4 00/18] exit_net checks for objects initialized in net_init hook Vasily Averin
2017-11-12 19:26   ` [PATCH v5 00/13] " Vasily Averin
2017-11-14  6:47     ` David Miller
2017-11-12 19:33   ` [PATCH v5 12/13] ppp: exit_net cleanup checks added Vasily Averin
2017-11-12  8:50 ` [PATCH v4 17/18] " Vasily Averin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).