From: ebiederm@xmission.com (Eric W. Biederman)
To: Pavel Emelyanov <xemul@parallels.com>
Cc: Daniel Lezcano <dlezcano@fr.ibm.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "Serge E. Hallyn" <serge@hallyn.com>,
        Andrew Morton <akpm@linux-foundation.org>
In-Reply-To: <4FAD0555.4090906@parallels.com> (Pavel Emelyanov's message of
	"Fri, 11 May 2012 16:25:57 +0400")
References: <4FAD0524.3000307@parallels.com> <4FAD0555.4090906@parallels.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux)
Date: Fri, 11 May 2012 10:05:48 -0700
Message-ID: <87d36a7imr.fsf@xmission.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: Re: [PATCH 2/2] ns: Add proc_ns_operations for mount namespaces
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3677
Lines: 125

Pavel Emelyanov <xemul@parallels.com> writes:

> Currently LXC by default creates a container in a new mount
> namespace. Thus in order to explore it we have to
>
> a) find out, that a new mount namespace is in use
> b) enter this other namespace
>
> This patch solves both -- allows us to distinguish one mount
> namespace from another by comparing its inode numbers and lets
> us enter a mount namespace with the setns system call.

There are two significant bugs with your patch.

You do not set fs->root or fs->pwd to values in the new mount namespace,
I don't believe there is anywhere else in the vfs where this is possible
except possible fchdir.

It is easily possible to create a reference counting cycle by bind
mounting the current mount namespace into itself.

Not that I am opposed to the concept I have just been dusting my patch
for this same functionality off.

Eric


> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
> ---
>  fs/namespace.c          |   45 +++++++++++++++++++++++++++++++++++++++++++++
>  fs/proc/namespaces.c    |    1 +
>  include/linux/proc_fs.h |    2 ++
>  3 files changed, 48 insertions(+), 0 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index e608199..9467904 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -20,6 +20,7 @@
>  #include <linux/fs_struct.h>	/* get_fs_root et.al. */
>  #include <linux/fsnotify.h>	/* fsnotify_vfsmount_delete */
>  #include <linux/uaccess.h>
> +#include <linux/proc_fs.h>
>  #include "pnode.h"
>  #include "internal.h"
>  
> @@ -2633,3 +2634,47 @@ bool our_mnt(struct vfsmount *mnt)
>  {
>  	return check_mnt(real_mount(mnt));
>  }
> +
> +static void *mntns_get(struct task_struct *task)
> +{
> +	struct mnt_namespace *mn = NULL;
> +	struct nsproxy *nsproxy;
> +
> +	rcu_read_lock();
> +	nsproxy = task_nsproxy(task);
> +	if (nsproxy) {
> +		mn = nsproxy->mnt_ns;
> +		get_mnt_ns(mn);
> +	}
> +	rcu_read_unlock();
> +
> +	return mn;
> +}
> +
> +static void mntns_put(void *ns)
> +{
> +	put_mnt_ns(ns);
> +}
> +
> +static int mntns_install(struct nsproxy *nsproxy, void *ns)
> +{
> +	put_mnt_ns(nsproxy->mnt_ns);
> +	get_mnt_ns(ns);
> +	nsproxy->mnt_ns = ns;
> +	return 0;
> +}
> +
> +static u64 mntns_get_id(void *_ns)
> +{
> +	struct mnt_namespace *ns = _ns;
> +	return ns->root->mnt_id;
> +}
> +
> +const struct proc_ns_operations mntns_operations = {
> +	.name		= "mnt",
> +	.type		= CLONE_NEWNS,
> +	.get		= mntns_get,
> +	.put		= mntns_put,
> +	.install	= mntns_install,
> +	.get_id		= mntns_get_id,
> +};
> diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c
> index b6c7560..e0399dd 100644
> --- a/fs/proc/namespaces.c
> +++ b/fs/proc/namespaces.c
> @@ -24,6 +24,7 @@ static const struct proc_ns_operations *ns_entries[] = {
>  #ifdef CONFIG_IPC_NS
>  	&ipcns_operations,
>  #endif
> +	&mntns_operations,
>  };
>  
>  static const struct file_operations ns_file_operations = {
> diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
> index e5ee83a..f6311b4 100644
> --- a/include/linux/proc_fs.h
> +++ b/include/linux/proc_fs.h
> @@ -252,6 +252,8 @@ struct proc_ns_operations {
>  extern const struct proc_ns_operations netns_operations;
>  extern const struct proc_ns_operations utsns_operations;
>  extern const struct proc_ns_operations ipcns_operations;
> +extern const struct proc_ns_operations mntns_operations;
> +
>  
>  union proc_op {
>  	int (*proc_get_link)(struct dentry *, struct path *);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/