Commit 7866cc57 authored by Florian Westphal's avatar Florian Westphal Committed by Pablo Neira Ayuso
Browse files

netns: add and use net_ns_barrier 



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: default avatarJoe Stringer <joe@ovn.org>
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Acked-by: default avatarDavid S. Miller <davem@davemloft.net>
Acked-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 2c41f33c

include/net/net_namespace.h

+3 −0
Original line number Diff line number Diff line
@@ -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 */

net/core/net_namespace.c

+17 −0
Original line number Diff line number Diff line
@@ -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)

net/netfilter/nf_conntrack_core.c

+9 −0
Original line number Diff line number Diff line
@@ -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.

CRA Git | Maintained and supported by SUSTech CRA and CCSE