netdev - Re: [PATCH 3/7] ns proc: Add support for the network namespace.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1305141705.1236.49.camel@orca.stoopid.dyndns.org>
Date:	Wed, 11 May 2011 14:21:34 -0500
From:	Nathan Lynch <ntl@...ox.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	jamal <hadi@...erus.ca>, Daniel Lezcano <daniel.lezcano@...e.fr>,
	Linux Containers <containers@...ts.osdl.org>,
	Renato Westphal <renatowestphal@...il.com>
Subject: Re: [PATCH 3/7] ns proc: Add support for the network namespace.

On Fri, 2011-05-06 at 19:24 -0700, Eric W. Biederman wrote:
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index 3f86026..bf7707e 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -573,3 +573,34 @@ void unregister_pernet_device(struct pernet_operations *ops)
>  	mutex_unlock(&net_mutex);
>  }
>  EXPORT_SYMBOL_GPL(unregister_pernet_device);
> +
> +#ifdef CONFIG_NET_NS
> +static void *netns_get(struct task_struct *task)
> +{
> +	struct net *net;
> +	rcu_read_lock();
> +	net = get_net(task->nsproxy->net_ns);

This should use task_nsproxy() and check the result before grabbing the
net_ns, but I think you fix that in a later patch.

Regardless, it looks as if all the proc_ns_ops->get() implementations
really just want the nsproxy, so maybe the get() methods should take
that instead of the task_struct, and proc_ns_instantiate() should do
something like:

struct nsproxy *nsproxy;
...

ei->ns_ops = ns_ops;
error = -ESRCH;
rcu_read_lock();
nsproxy = task_nsproxy(task);
rcu_read_unlock();
if (!nsproxy)
	got out;
ei->ns = ns_ops->get(nsproxy);


So then the zombie check is consolidated in one place instead of having
to do it in every get() method.



> +	rcu_read_unlock();
> +	return net;
> +}
> +
> +static void netns_put(void *ns)
> +{
> +	put_net(ns);
> +}
> +
> +static int netns_install(struct nsproxy *nsproxy, void *ns)
> +{
> +	put_net(nsproxy->net_ns);
> +	nsproxy->net_ns = get_net(ns);
> +	return 0;
> +}

This introduces a window where, potentially, nsproxy->net_ns is stale
before it is updated with the namespace which is being attached, no? 
(Same concern applies to other install methods in the patch set).  It
seems possible to oops the kernel in this window by looking up
/proc/$PID/ns/net while $PID is in the midst of setns().


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html