netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: netfilter-devel@vger.kernel.org
Cc: davem@davemloft.net, netdev@vger.kernel.org
Subject: [PATCH 23/29] netns: add and use net_ns_barrier
Date: Fri, 30 Jun 2017 00:53:21 +0200	[thread overview]
Message-ID: <1498776807-11124-24-git-send-email-pablo@netfilter.org> (raw)
In-Reply-To: <1498776807-11124-1-git-send-email-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Quoting Joe Stringer:
  If a user loads nf_conntrack_ftp, sends FTP traffic through a network
  namespace, destroys that namespace then unloads the FTP helper module,
  then the kernel will crash.

Events that lead to the crash:
1. conntrack is created with ftp helper in netns x
2. This netns is destroyed
3. netns destruction is scheduled
4. netns destruction wq starts, removes netns from global list
5. ftp helper is unloaded, which resets all helpers of the conntracks
via for_each_net()

but because netns is already gone from list the for_each_net() loop
doesn't include it, therefore all of these conntracks are unaffected.

6. helper module unload finishes
7. netns wq invokes destructor for rmmod'ed helper

CC: "Eric W. Biederman" <ebiederm@xmission.com>
Reported-by: Joe Stringer <joe@ovn.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: David S. Miller <davem@davemloft.net>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/net_namespace.h       |  3 +++
 net/core/net_namespace.c          | 17 +++++++++++++++++
 net/netfilter/nf_conntrack_core.c |  9 +++++++++
 3 files changed, 29 insertions(+)

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index fe80bb48ab1f..a24a57593202 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -158,6 +158,7 @@ extern struct net init_net;
 struct net *copy_net_ns(unsigned long flags, struct user_namespace *user_ns,
 			struct net *old_net);
 
+void net_ns_barrier(void);
 #else /* CONFIG_NET_NS */
 #include <linux/sched.h>
 #include <linux/nsproxy.h>
@@ -168,6 +169,8 @@ static inline struct net *copy_net_ns(unsigned long flags,
 		return ERR_PTR(-EINVAL);
 	return old_net;
 }
+
+static inline void net_ns_barrier(void) {}
 #endif /* CONFIG_NET_NS */
 
 
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 1934efd4a9d4..1f15abb1d733 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -482,6 +482,23 @@ static void cleanup_net(struct work_struct *work)
 		net_drop_ns(net);
 	}
 }
+
+/**
+ * net_ns_barrier - wait until concurrent net_cleanup_work is done
+ *
+ * cleanup_net runs from work queue and will first remove namespaces
+ * from the global list, then run net exit functions.
+ *
+ * Call this in module exit path to make sure that all netns
+ * ->exit ops have been invoked before the function is removed.
+ */
+void net_ns_barrier(void)
+{
+	mutex_lock(&net_mutex);
+	mutex_unlock(&net_mutex);
+}
+EXPORT_SYMBOL(net_ns_barrier);
+
 static DECLARE_WORK(net_cleanup_work, cleanup_net);
 
 void __put_net(struct net *net)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index c3bd9b086dcc..9979f46c81dc 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1720,6 +1720,8 @@ EXPORT_SYMBOL_GPL(nf_ct_iterate_cleanup_net);
  * Like nf_ct_iterate_cleanup, but first marks conntracks on the
  * unconfirmed list as dying (so they will not be inserted into
  * main table).
+ *
+ * Can only be called in module exit path.
  */
 void
 nf_ct_iterate_destroy(int (*iter)(struct nf_conn *i, void *data), void *data)
@@ -1734,6 +1736,13 @@ nf_ct_iterate_destroy(int (*iter)(struct nf_conn *i, void *data), void *data)
 	}
 	rtnl_unlock();
 
+	/* Need to wait for netns cleanup worker to finish, if its
+	 * running -- it might have deleted a net namespace from
+	 * the global list, so our __nf_ct_unconfirmed_destroy() might
+	 * not have affected all namespaces.
+	 */
+	net_ns_barrier();
+
 	/* a conntrack could have been unlinked from unconfirmed list
 	 * before we grabbed pcpu lock in __nf_ct_unconfirmed_destroy().
 	 * This makes sure its inserted into conntrack table.
-- 
2.1.4

  parent reply	other threads:[~2017-06-29 22:53 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-29 22:52 [PATCH 00/29] Netfilter updates for net-next Pablo Neira Ayuso
2017-06-29 22:52 ` [PATCH 01/29] netfilter: ctnetlink: delete extra spaces Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 02/29] netfilter: ipt_CLUSTERIP: switch to nf_register_net_hook Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 03/29] netfilter: dup: resolve warnings about missing prototypes Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 04/29] netfilter: nft_rt: make local functions static Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 05/29] netfilter: conntrack: rename nf_ct_iterate_cleanup Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 06/29] netfilter: conntrack: don't call iter for non-confirmed conntracks Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 07/29] netfilter: conntrack: add nf_ct_iterate_destroy Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 08/29] netfilter: conntrack: restart iteration on resize Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 09/29] netfilter: nat: destroy nat mappings on module exit path only Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 10/29] netfilter: nft_set_hash: unnecessary forward declaration Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 11/29] netfilter: nf_tables: no size estimation if number of set elements is unknown Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 12/29] netfilter: nft_set_hash: use nft_rhash prefix for resizable set backend Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 13/29] netfilter: nf_tables: select set backend flavour depending on description Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 14/29] netfilter: nf_tables: pass set description to ->privsize Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 15/29] netfilter: nft_set_hash: add nft_hash_buckets() Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 16/29] netfilter: nf_tables: allow large allocations for new sets Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 17/29] netfilter: nft_set_hash: add non-resizable hashtable implementation Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 18/29] netfilter: nft_set_hash: add lookup variant for fixed size hashtable Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 19/29] netfilter: nf_ct_helper: use nf_ct_iterate_destroy to unlink helper objs Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 20/29] netfilter: cttimeout: use nf_ct_iterate_cleanup_net to unlink timeout objs Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 21/29] netfilter: ipt_CLUSTERIP: do not hold dev Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 22/29] netfilter: move table iteration out of netns exit paths Pablo Neira Ayuso
2017-06-29 22:53 ` Pablo Neira Ayuso [this message]
2017-06-29 22:53 ` [PATCH 24/29] netfilter: ebt: Use new helper ebt_invalid_target to check target Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 25/29] netfilter, kbuild: use canonical method to specify objs Pablo Neira Ayuso
2017-06-30 11:17   ` David Laight
2017-06-29 22:53 ` [PATCH 26/29] netfilter: use nf_conntrack_helpers_register when possible Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 27/29] netfilter: conntrack: use NFPROTO_MAX to size array Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 28/29] netfilter: nf_tables: reduce chain type table size Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 29/29] netfilter: nfnetlink: extended ACK reporting Pablo Neira Ayuso
2017-06-30 16:09 ` [PATCH 00/29] Netfilter updates for net-next David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1498776807-11124-24-git-send-email-pablo@netfilter.org \
    --to=pablo@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).