netns: add and use net_ns_barrier
authorFlorian Westphal <fw@strlen.de>
Tue, 30 May 2017 09:38:12 +0000 (11:38 +0200)
committerPablo Neira Ayuso <pablo@netfilter.org>
Mon, 19 Jun 2017 17:09:19 +0000 (19:09 +0200)
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
net/core/net_namespace.c
net/netfilter/nf_conntrack_core.c

index fe80bb48ab1f0c7b1665a10a9bf2388b32eb5013..a24a57593202456dd3e25ce1891f70c33ccd7ffe 100644 (file)
@@ -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 */
 
 
index 1934efd4a9d4986f3497abc40968fd08c91896b3..1f15abb1d733c702d8dfaf62898db3712bdd90e5 100644 (file)
@@ -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)
index c3bd9b086dcc9d60e62e232c078bd915b133bb8c..9979f46c81dce32bc2288cfd4561c571f5bea4c5 100644 (file)
@@ -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.