Skip to contentSkip to conversation

user namespaces: introduction

PATCH In review Serge E. Hallyn 4 participants 20 messages Jul 26, 2008 – Aug 3, 2008 13 messages not in the archive

Subsystems (6)
VFS MM core Scheduler Security (LSM) +2 more subsystems
lore
Share & tools
lore
#0 lore raw

cover user namespaces: introduction

Following is a set of user namespace patches I've been playing with this week.

The first two patches are I believe fixes which should go in regardless of which direction user namespaces take.

The rest of the patches are one approach to providing default cross-userns isolation for files. Any filesystem can provide its own intelligent cross-userns userid equivalence checks by defining its own permission function, which is what Eric and I have been talking about doing.

The next step is probably to handle some of the task-to-task cross-userns checks.

Comments appreciated.

thanks,
-serge

A message that is not in the archive (sent off-list or not yet received): 20080726002700.GA29686-r/[email protected]

#2 lore raw

PATCH 1/6 user namespaces: introduce user_struct->user_namespace relationship

From 9382f22a6c751e90baa4e7f3ba24c509e50a47a8 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serue-r/[email protected]>
Date: Tue, 22 Jul 2008 13:31:37 -0500
Subject: [PATCH 1/6] user namespaces: introduce user_struct->user_namespace relationship

When a task does clone(CLONE_NEWNS), the task's user is the 'creator' of the new user_namespace, and the user_namespace is tacked onto a list of those created by this user.

---
 include/linux/sched.h          |    1 +
 include/linux/user_namespace.h |    1 +
 kernel/user.c                  |    7 +++++++
 kernel/user_namespace.c        |   20 +++++++++++---------
 4 files changed, 20 insertions(+), 9 deletions(-)
include/linux/sched.h +1 -0
index dc7e592..cf36e14 100644
@@ -600,6 +600,7 @@ struct user_struct {
 	/* Hash table maintenance information */
 	struct hlist_node uidhash_node;
 	uid_t uid;
+	struct user_namespace *user_namespace;
 
 #ifdef CONFIG_USER_SCHED
 	struct task_group *tg;
include/linux/user_namespace.h +1 -0
index b5f41d4..f9477c3 100644
@@ -13,6 +13,7 @@ struct user_namespace {
 	struct kref		kref;
 	struct hlist_head	uidhash_table[UIDHASH_SZ];
 	struct user_struct	*root_user;
+	struct user_struct	*creator;
 };
 
 extern struct user_namespace init_user_ns;
kernel/user.c +7 -0
index 865ecf5..cfe8309 100644
@@ -22,6 +22,7 @@ struct user_namespace init_user_ns = {
 		.refcount	= ATOMIC_INIT(2),
 	},
 	.root_user = &root_user,
+	.creator = &root_user,
 };
 EXPORT_SYMBOL_GPL(init_user_ns);
 
@@ -53,6 +54,7 @@ struct user_struct root_user = {
 	.files		= ATOMIC_INIT(0),
 	.sigpending	= ATOMIC_INIT(0),
 	.locked_shm     = 0,
+	.user_namespace	= &init_user_ns,
 #ifdef CONFIG_USER_SCHED
 	.tg		= &init_task_group,
 #endif
@@ -321,6 +323,7 @@ done:
  */
 static inline void free_user(struct user_struct *up, unsigned long flags)
 {
+	put_user_ns(up->user_namespace);
 	/* restore back the count */
 	atomic_inc(&up->__count);
 	spin_unlock_irqrestore(&uidhash_lock, flags);
@@ -347,6 +350,7 @@ static inline void free_user(struct user_struct *up, unsigned long flags)
 	sched_destroy_user(up);
 	key_put(up->uid_keyring);
 	key_put(up->session_keyring);
+	put_user_ns(up->user_namespace);
 	kmem_cache_free(uid_cachep, up);
 }
 
@@ -409,6 +413,8 @@ struct user_struct *alloc_uid(struct user_namespace *ns, uid_t uid)
 		if (sched_create_user(new) < 0)
 			goto out_free_user;
 
+		new->user_namespace = get_user_ns(ns);
+
 		if (uids_user_create(new))
 			goto out_destoy_sched;
 
@@ -441,6 +447,7 @@ struct user_struct *alloc_uid(struct user_namespace *ns, uid_t uid)
 
 out_destoy_sched:
 	sched_destroy_user(new);
+	put_user_ns(new->user_namespace);
 out_free_user:
 	kmem_cache_free(uid_cachep, new);
 out_unlock:
kernel/user_namespace.c +11 -9
index a9ab059..e8db443 100644
@@ -19,7 +19,6 @@
 static struct user_namespace *clone_user_ns(struct user_namespace *old_ns)
 {
 	struct user_namespace *ns;
-	struct user_struct *new_user;
 	int n;
 
 	ns = kmalloc(sizeof(struct user_namespace), GFP_KERNEL);
@@ -38,15 +37,17 @@ static struct user_namespace *clone_user_ns(struct user_namespace *old_ns)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	/* Reset current->user with a new one */
-	new_user = alloc_uid(ns, current->uid);
-	if (!new_user) {
-		free_uid(ns->root_user);
-		kfree(ns);
-		return ERR_PTR(-ENOMEM);
-	}
+	/* pin the creating user */
+	ns->creator = current->user;
+	atomic_inc(&ns->creator->__count);
+
+	/*
+	 * The alloc_uid() incremented the userns refcount,
+	 * so drop it again 
+	 */
+	put_user_ns(ns);
 
-	switch_uid(new_user);
+	switch_uid(ns->root_user);
 	return ns;
 }
 
@@ -72,6 +73,7 @@ void free_user_ns(struct kref *kref)
 
 	ns = container_of(kref, struct user_namespace, kref);
 	release_uids(ns);
+	free_uid(ns->creator);
 	kfree(ns);
 }
 EXPORT_SYMBOL(free_user_ns);
Signature · 1 line
1.5.4.3

A message that is not in the archive (sent off-list or not yet received): 20080726002725.GA29874-r/[email protected]

#4 lore raw

Re: [Devel] [PATCH 1/6] user namespaces: introduce user_struct->user_namespace relationship

include/linux/sched.h struct user_struct { · line 603

Quoted 7 lines · Serge E. Hallyn
include/linux/sched.h +1 -0
@@ -600,6 +600,7 @@ struct user_struct {
 	/* Hash table maintenance information */
 	struct hlist_node uidhash_node;
 	uid_t uid;
+	struct user_namespace *user_namespace;

Call it "user_ns" please, and file "user_ns.c".

A message that is not in the archive (sent off-list or not yet received): 20080726020731.GA5115-QDJVlCTZ4KWTKS93B3g+7KFoa47nwP16@public.gmane.org

#6 lore raw

Re: [Devel] [PATCH 1/6] user namespaces: introduce user_struct->user_namespace relationship

Quoted 10 lines · Alexey Dobriyan

include/linux/sched.h struct user_struct { · line 603

Quoted 7 lines · Serge E. Hallyn
include/linux/sched.h +1 -0
@@ -600,6 +600,7 @@ struct user_struct {
 	/* Hash table maintenance information */
 	struct hlist_node uidhash_node;
 	uid_t uid;
+	struct user_namespace *user_namespace;

Call it "user_ns" please,

Odd, I had the same thought, but seem to have applied at the wrong patch (please see patch 2).

Thanks.

Quoted 1 line

and file "user_ns.c".

I'm ok with that, but that file has existed for quite some time by now, so I'm not introducing it.

If noone objects, I'll rename the file (as well as include/linux/user_namespace.h).

thanks,
-serge

#7 lore raw

PATCH 2/6 user namespaces: move user_ns from nsproxy into user struct

In reply to a message not in the archive

From ec5f54faf5afd16cb6cef40ebaaf3da25989d185 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serue-r/[email protected]>
Date: Thu, 24 Jul 2008 17:52:41 -0500
Subject: [PATCH 2/6] user namespaces: move user_ns from nsproxy into user struct

When we get the sysfs support needed to support fair user scheduling along with user namespaces, then we will need to be able to get the user namespace from the user struct.

So we need the user_ns to be a part of struct user. Once we can access it from tsk->user, we no longer have a use for tsk->nsproxy->user_ns.

When a user_namespace is created, the user which created it is marked as its 'creator'. The user_namespace pins the creator. Each userid in a user_ns pins the user_ns. This keeps refcounting nice and simple.

At the same time, this patch simplifies the refcounting. The current user and userns locking works as follows:

The task pins the user struct.

The task pins the nsproxy, the nsproxy pins the user_ns.

When a new user_ns is created, it creates a root user for it, and pins it. When the nsproxy releases the user_ns, the userns tries to release all its user structs.

So you see that the refcounting "works" for now, but only because the nsproxy (and therefore usr_ns) and user structs will be freed at the same time - when the last task using them is released.

Now we need to put the user_ns in the struct user. You can see that will mess up the refcounting.

Fortunately, once the user_ns is available from tsk->user, we don't need it in nsproxy.

So here is how the refcounting *should* be done:

The task pins the user struct.

The user struct pins its user namespace.

The user namespace pins the struct user which created it.

A user namespace now doesn't need to release its userids, because it is only released when its last user disappears.

This patch makes those changes.

---
 include/linux/init_task.h      |    1 -
 include/linux/key.h            |    3 ++
 include/linux/nsproxy.h        |    1 -
 include/linux/sched.h          |    3 +-
 include/linux/user_namespace.h |   10 +++----
 kernel/fork.c                  |    3 +-
 kernel/nsproxy.c               |   10 +------
 kernel/sys.c                   |    4 +-
 kernel/user.c                  |   52 +++++++++++----------------------------
 kernel/user_namespace.c        |   36 +++++++--------------------
 security/keys/process_keys.c   |    7 ++++-
 11 files changed, 45 insertions(+), 85 deletions(-)
include/linux/init_task.h +0 -1
index 93c45ac..ce87b8a 100644
@@ -57,7 +57,6 @@ extern struct nsproxy init_nsproxy;
 	.mnt_ns		= NULL,						\
 	INIT_NET_NS(net_ns)                                             \
 	INIT_IPC_NS(ipc_ns)						\
-	.user_ns	= &init_user_ns,				\
 }
 
 #define INIT_SIGHAND(sighand) {						\
include/linux/key.h +3 -0
index c45c962..ba53aef 100644
@@ -277,6 +277,8 @@ extern ctl_table key_sysctls[];
  * the userspace interface
  */
 extern void switch_uid_keyring(struct user_struct *new_user);
+extern void switch_uid_keyring_task(struct task_struct *tsk,
+				struct user_struct *new_user);
 extern int copy_keys(unsigned long clone_flags, struct task_struct *tsk);
 extern int copy_thread_group_keys(struct task_struct *tsk);
 extern void exit_keys(struct task_struct *tsk);
@@ -305,6 +307,7 @@ extern void key_init(void);
 #define key_ref_to_ptr(k)		({ NULL; })
 #define is_key_possessed(k)		0
 #define switch_uid_keyring(u)		do { } while(0)
+#define switch_uid_keyring_task(t,u)	do { } while(0)
 #define __install_session_keyring(t, k)	({ NULL; })
 #define copy_keys(f,t)			0
 #define copy_thread_group_keys(t)	0
include/linux/nsproxy.h +0 -1
index 0e66b57..7c7fb20 100644
@@ -27,7 +27,6 @@ struct nsproxy {
 	struct ipc_namespace *ipc_ns;
 	struct mnt_namespace *mnt_ns;
 	struct pid_namespace *pid_ns;
-	struct user_namespace *user_ns;
 	struct net 	     *net_ns;
 };
 extern struct nsproxy init_nsproxy;
include/linux/sched.h +2 -1
index cf36e14..2c2f036 100644
@@ -600,7 +600,7 @@ struct user_struct {
 	/* Hash table maintenance information */
 	struct hlist_node uidhash_node;
 	uid_t uid;
-	struct user_namespace *user_namespace;
+	struct user_namespace *user_ns;
 
 #ifdef CONFIG_USER_SCHED
 	struct task_group *tg;
@@ -1744,6 +1744,7 @@ static inline struct user_struct *get_uid(struct user_struct *u)
 }
 extern void free_uid(struct user_struct *);
 extern void switch_uid(struct user_struct *);
+extern void task_switch_uid(struct task_struct *tsk, struct user_struct *);
 extern void release_uids(struct user_namespace *ns);
 
 #include <asm/current.h>
include/linux/user_namespace.h +4 -6
index f9477c3..1b4959d 100644
@@ -27,8 +27,7 @@ static inline struct user_namespace *get_user_ns(struct user_namespace *ns)
 	return ns;
 }
 
-extern struct user_namespace *copy_user_ns(int flags,
-					   struct user_namespace *old_ns);
+extern int create_new_userns(int flags, struct task_struct *tsk);
 extern void free_user_ns(struct kref *kref);
 
 static inline void put_user_ns(struct user_namespace *ns)
@@ -44,13 +43,12 @@ static inline struct user_namespace *get_user_ns(struct user_namespace *ns)
 	return &init_user_ns;
 }
 
-static inline struct user_namespace *copy_user_ns(int flags,
-						  struct user_namespace *old_ns)
+static inline int create_new_userns(int flags, struct task_struct *tsk)
 {
 	if (flags & CLONE_NEWUSER)
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 
-	return old_ns;
+	return 0;
 }
 
 static inline void put_user_ns(struct user_namespace *ns)
kernel/fork.c +1 -2
index adefc11..42cc45f 100644
@@ -917,8 +917,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	retval = -EAGAIN;
 	if (atomic_read(&p->user->processes) >=
 			p->signal->rlim[RLIMIT_NPROC].rlim_cur) {
-		if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RESOURCE) &&
-		    p->user != current->nsproxy->user_ns->root_user)
+		if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RESOURCE))
 			goto bad_fork_free;
 	}
 
kernel/nsproxy.c +2 -8
index adc7851..861d6f4 100644
@@ -81,11 +81,9 @@ static struct nsproxy *create_new_namespaces(unsigned long flags,
 		goto out_pid;
 	}
 
-	new_nsp->user_ns = copy_user_ns(flags, tsk->nsproxy->user_ns);
-	if (IS_ERR(new_nsp->user_ns)) {
-		err = PTR_ERR(new_nsp->user_ns);
+	err = create_new_userns(flags, tsk);
+	if (err)
 		goto out_user;
-	}
 
 	new_nsp->net_ns = copy_net_ns(flags, tsk->nsproxy->net_ns);
 	if (IS_ERR(new_nsp->net_ns)) {
@@ -96,8 +94,6 @@ static struct nsproxy *create_new_namespaces(unsigned long flags,
 	return new_nsp;
 
 out_net:
-	if (new_nsp->user_ns)
-		put_user_ns(new_nsp->user_ns);
 out_user:
 	if (new_nsp->pid_ns)
 		put_pid_ns(new_nsp->pid_ns);
@@ -180,8 +176,6 @@ void free_nsproxy(struct nsproxy *ns)
 		put_ipc_ns(ns->ipc_ns);
 	if (ns->pid_ns)
 		put_pid_ns(ns->pid_ns);
-	if (ns->user_ns)
-		put_user_ns(ns->user_ns);
 	put_net(ns->net_ns);
 	kmem_cache_free(nsproxy_cachep, ns);
 }
kernel/sys.c +2 -2
index 14e9728..b8623be 100644
@@ -568,13 +568,13 @@ static int set_user(uid_t new_ruid, int dumpclear)
 {
 	struct user_struct *new_user;
 
-	new_user = alloc_uid(current->nsproxy->user_ns, new_ruid);
+	new_user = alloc_uid(current->user->user_ns, new_ruid);
 	if (!new_user)
 		return -EAGAIN;
 
 	if (atomic_read(&new_user->processes) >=
 				current->signal->rlim[RLIMIT_NPROC].rlim_cur &&
-			new_user != current->nsproxy->user_ns->root_user) {
+			!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) {
 		free_uid(new_user);
 		return -EAGAIN;
 	}
kernel/user.c +15 -37

114 lines not on this page · raw message

kernel/user_namespace.c +10 -26
index e8db443..0045dd0 100644
@@ -16,14 +16,17 @@
  * @old_ns: namespace to clone
  * Return NULL on error (failure to kmalloc), new ns otherwise
  */
-static struct user_namespace *clone_user_ns(struct user_namespace *old_ns)
+int create_new_userns(int flags, struct task_struct *tsk)
 {
 	struct user_namespace *ns;
 	int n;
 
+	if (!(flags & CLONE_NEWUSER))
+		return 0;
+
 	ns = kmalloc(sizeof(struct user_namespace), GFP_KERNEL);
 	if (!ns)
-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;
 
 	kref_init(&ns->kref);
 
@@ -34,37 +37,19 @@ static struct user_namespace *clone_user_ns(struct user_namespace *old_ns)
 	ns->root_user = alloc_uid(ns, 0);
 	if (!ns->root_user) {
 		kfree(ns);
-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;
 	}
 
 	/* pin the creating user */
-	ns->creator = current->user;
+	ns->creator = tsk->user;
 	atomic_inc(&ns->creator->__count);
 
-	/*
-	 * The alloc_uid() incremented the userns refcount,
-	 * so drop it again 
-	 */
+	/* alloc_uid() incremented the userns refcount.  drop it again */
 	put_user_ns(ns);
 
-	switch_uid(ns->root_user);
-	return ns;
-}
-
-struct user_namespace * copy_user_ns(int flags, struct user_namespace *old_ns)
-{
-	struct user_namespace *new_ns;
-
-	BUG_ON(!old_ns);
-	get_user_ns(old_ns);
-
-	if (!(flags & CLONE_NEWUSER))
-		return old_ns;
-
-	new_ns = clone_user_ns(old_ns);
+	task_switch_uid(tsk, ns->root_user);
 
-	put_user_ns(old_ns);
-	return new_ns;
+	return 0;
 }
 
 void free_user_ns(struct kref *kref)
@@ -72,7 +57,6 @@ void free_user_ns(struct kref *kref)
 	struct user_namespace *ns;
 
 	ns = container_of(kref, struct user_namespace, kref);
-	release_uids(ns);
 	free_uid(ns->creator);
 	kfree(ns);
 }
security/keys/process_keys.c +6 -1
index 5be6d01..8d1cfb2 100644
@@ -119,7 +119,8 @@ error:
 /*
  * deal with the UID changing
  */
-void switch_uid_keyring(struct user_struct *new_user)
+void switch_uid_keyring_task(struct task_struct *task,
+				struct user_struct *new_user)
 {
 #if 0 /* do nothing for now */
 	struct key *old;
@@ -142,6 +143,10 @@ void switch_uid_keyring(struct user_struct *new_user)
 
 } /* end switch_uid_keyring() */
 
+void switch_uid_keyring(struct user_struct *new_user)
+{
+	switch_uid_keyring_task(current, new_user);
+}
 /*****************************************************************************/
 /*
  * install a fresh thread keyring, discarding the old one
Signature · 1 line
1.5.4.3

A message that is not in the archive (sent off-list or not yet received): 20080726002735.GB29874-r/[email protected]

#9 lore raw

Re: [PATCH 2/6] user namespaces: move user_ns from nsproxy into user struct

Quoted 5 lines · Serge E. Hallyn
Quoted 1 line

From ec5f54faf5afd16cb6cef40ebaaf3da25989d185 Mon Sep 17 00:00:00 2001

From: Serge Hallyn <serue-r/[email protected]>
Date: Thu, 24 Jul 2008 17:52:41 -0500
Subject: [PATCH 2/6] user namespaces: move user_ns from nsproxy into user struct

Quoted 7 lines

When we get the sysfs support needed to support fair user scheduling along with user namespaces, then we will need to be able to get the user namespace from the user struct.

So we need the user_ns to be a part of struct user. Once we can access it from tsk->user, we no longer have a use for tsk->nsproxy->user_ns.

Is this true? Even in the general case of supporting setuid and setgid and everything else that potentially is in the user namespace?

I certainly support the cleanups you have made for the reasons you describe. I think however that there is there are no technical reasons not to have
nsproxy->user_ns after the changes have been made. I also agree that
there are no technical reasons for keeping nsproxy->user_ns at the moment.

Quoted 4 lines

When a user_namespace is created, the user which created it is marked as its 'creator'. The user_namespace pins the creator. Each userid in a user_ns pins the user_ns. This keeps refcounting nice and simple.

Eric

A message that is not in the archive (sent off-list or not yet received): [email protected]

#11 lore raw

Re: [PATCH 2/6] user namespaces: move user_ns from nsproxy into user struct

Quoted 11 lines · Eric W. Biederman
Quoted 9 lines · Serge E. Hallyn
Quoted 1 line

From ec5f54faf5afd16cb6cef40ebaaf3da25989d185 Mon Sep 17 00:00:00 2001

From: Serge Hallyn <serue-r/[email protected]>
Date: Thu, 24 Jul 2008 17:52:41 -0500
Subject: [PATCH 2/6] user namespaces: move user_ns from nsproxy into user struct

When we get the sysfs support needed to support fair user scheduling along with user namespaces, then we will need to be able to get the user namespace from the user struct.

Quoted 6 lines
Quoted 3 lines

So we need the user_ns to be a part of struct user. Once we can access it from tsk->user, we no longer have a use for tsk->nsproxy->user_ns.

Is this true? Even in the general case of supporting setuid and setgid and everything else that potentially is in the user namespace?

Sure. At any time we can get tsk->user->user_ns, and from that we can get tsk->user->user_ns->creator.

Quoted 3 lines

I certainly support the cleanups you have made for the reasons you describe. I think however that there is there are no technical reasons not to have
nsproxy->user_ns after the changes have been made.

Well I ended up tossing it to clarify my thinking about the refcounting. With that done, I think we could safely not have nsproxy pin the user_ns, trusting the tsk->user to pin it instead.

But keeping it in two places just seems needlessly complicated.

Quoted 2 lines

I also agree that
there are no technical reasons for keeping nsproxy->user_ns at the moment.

What is your preference?

Quoted 6 lines
Quoted 4 lines

When a user_namespace is created, the user which created it is marked as its 'creator'. The user_namespace pins the creator. Each userid in a user_ns pins the user_ns. This keeps refcounting nice and simple.

Eric

#12 lore raw

PATCH 3/6 user namespaces: rig generic_permission for simple userns check

In reply to a message not in the archive

From f6d09b06a1106936010bffd420267f5b7ee66238 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serue-r/[email protected]>
Date: Wed, 23 Jul 2008 17:01:09 -0500
Subject: [PATCH 3/6] user namespaces: rig generic_permission for simple userns check

Filesystems can provide their own permission() functions to do advanced inter-user_namespace userid equivalence checks.

For those filesystems which do not support that, we will do a simple check that current's user namespace is equivalent to the user_namespace which mounted the filesystem. If it is not equivalent, then the task can only have user nobody (that is, the 'other') permissions to a file.

For now, we actually just compare the user's user_ns to the init_user_ns. Next we will set the sb->user_ns to that of the task mounting a filesystem, and use inode->i_sb->user_ns instead of init_user_ns. By punting even on that, the implications, and therefore (in)correctness of this patch should be all the easier to verify.

---
 fs/namei.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)
fs/namei.c +13 -1
index 01e67dd..d5336fd 100644
@@ -31,6 +31,7 @@
 #include <linux/file.h>
 #include <linux/fcntl.h>
 #include <linux/device_cgroup.h>
+#include <linux/nsproxy.h>
 #include <asm/namei.h>
 #include <asm/uaccess.h>
 
@@ -168,7 +169,7 @@ void putname(const char *name)
 EXPORT_SYMBOL(putname);
 #endif
 
-
+extern struct user_namespace init_user_ns;
 /**
  * generic_permission  -  check for access rights on a Posix-like filesystem
  * @inode:	inode to check access rights for
@@ -184,7 +185,15 @@ int generic_permission(struct inode *inode, int mask,
 		int (*check_acl)(struct inode *inode, int mask))
 {
 	umode_t			mode = inode->i_mode;
+	int same_userns = (current->user->user_ns == &init_user_ns);
 
+	/*
+	 * If we're not in the inode's user namespace, we get
+	 * user nobody permissions, and we ignore acls
+	 * (bc serge doesn't know how to handle acls in this case)
+	 */
+	if (!same_userns)
+		goto check;
 	if (current->fsuid == inode->i_uid)
 		mode >>= 6;
 	else {
@@ -200,11 +209,14 @@ int generic_permission(struct inode *inode, int mask,
 			mode >>= 3;
 	}
 
+check:
 	/*
 	 * If the DACs are ok we don't need any capability check.
 	 */
 	if (((mode & mask & (MAY_READ|MAY_WRITE|MAY_EXEC)) == mask))
 		return 0;
+	if (!same_userns)
+		return -EACCES;
 
  check_capabilities:
 	/*
Signature · 1 line
1.5.4.3
#13 lore raw

PATCH 4/6 user namespaces: add user_ns to super block

In reply to a message not in the archive

From 420d6e81ce29d7a6fe3ab7b43c1171e105f8b697 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serue-r/[email protected]>
Date: Thu, 24 Jul 2008 18:00:54 -0500
Subject: [PATCH 4/6] user namespaces: add user_ns to super block

Add a user_ns to the super_block, and set it to the user_ns of the process which mounted the fs.

In generic_permission() compare the current user_ns to that of the user_ns which mounted the inode's filesystem.

---
 fs/namei.c         |    6 ++++--
 fs/super.c         |    3 +++
 include/linux/fs.h |    5 +++++
 3 files changed, 12 insertions(+), 2 deletions(-)
fs/namei.c +4 -2
index d5336fd..adf5f1b 100644
@@ -169,7 +169,6 @@ void putname(const char *name)
 EXPORT_SYMBOL(putname);
 #endif
 
-extern struct user_namespace init_user_ns;
 /**
  * generic_permission  -  check for access rights on a Posix-like filesystem
  * @inode:	inode to check access rights for
@@ -185,7 +184,10 @@ int generic_permission(struct inode *inode, int mask,
 		int (*check_acl)(struct inode *inode, int mask))
 {
 	umode_t			mode = inode->i_mode;
-	int same_userns = (current->user->user_ns == &init_user_ns);
+	int same_userns = 1;
+
+	if (current->user->user_ns != inode->i_sb->user_ns)
+		same_userns = 0;
 
 	/*
 	 * If we're not in the inode's user namespace, we get
fs/super.c +3 -0
index 453877c..12865ac 100644
@@ -39,6 +39,7 @@
 #include <linux/mutex.h>
 #include <linux/file.h>
 #include <asm/uaccess.h>
+#include <linux/user_namespace.h>
 #include "internal.h"
 
 
@@ -92,6 +93,7 @@ static struct super_block *alloc_super(struct file_system_type *type)
 		s->s_qcop = sb_quotactl_ops;
 		s->s_op = &default_op;
 		s->s_time_gran = 1000000000;
+		s->user_ns = get_user_ns(current->user->user_ns);
 	}
 out:
 	return s;
@@ -108,6 +110,7 @@ static inline void destroy_super(struct super_block *s)
 	security_sb_free(s);
 	kfree(s->s_subtype);
 	kfree(s->s_options);
+	put_user_ns(s->user_ns);
 	kfree(s);
 }
 
include/linux/fs.h +5 -0
index 9c2ac5c..68066a4 100644
@@ -1092,6 +1092,11 @@ struct super_block {
 	 * generic_show_options()
 	 */
 	char *s_options;
+
+	/*
+	 * namespace of the user which mounted the sb
+	 */
+	struct user_namespace *user_ns;
 };
 
 extern struct timespec current_fs_time(struct super_block *sb);
Signature · 1 line
1.5.4.3

A message that is not in the archive (sent off-list or not yet received): 20080726002754.GD29874-r/[email protected]

#15 lore raw

Re: [PATCH 4/6] user namespaces: add user_ns to super block

Quoted 10 lines · Serge E. Hallyn
Quoted 1 line

From 420d6e81ce29d7a6fe3ab7b43c1171e105f8b697 Mon Sep 17 00:00:00 2001

From: Serge Hallyn <serue-r/[email protected]>
Date: Thu, 24 Jul 2008 18:00:54 -0500
Subject: [PATCH 4/6] user namespaces: add user_ns to super block

Add a user_ns to the super_block, and set it to the user_ns of the process which mounted the fs.

In generic_permission() compare the current user_ns to that of the user_ns which mounted the inode's filesystem.

I don't think this is the right approach.

When we had the conversation earlier this was conceptually rejected as it prevents nfs superblock unification.

We really want to store this in the vfsmount and pass the user namespace down from there to where we are going to use it if at all possible.

The vfsmount also appears necessary if we are ever going to support multiple user namespaces per filesystem as the filesystem still need to know which user namespace to interpret it's data in.

Eric

A message that is not in the archive (sent off-list or not yet received): [email protected]

#17 lore raw

Re: [PATCH 4/6] user namespaces: add user_ns to super block

Quoted 18 lines · Eric W. Biederman
Quoted 10 lines · Serge E. Hallyn
Quoted 1 line

From 420d6e81ce29d7a6fe3ab7b43c1171e105f8b697 Mon Sep 17 00:00:00 2001

From: Serge Hallyn <serue-r/[email protected]>
Date: Thu, 24 Jul 2008 18:00:54 -0500
Subject: [PATCH 4/6] user namespaces: add user_ns to super block

Add a user_ns to the super_block, and set it to the user_ns of the process which mounted the fs.

In generic_permission() compare the current user_ns to that of the user_ns which mounted the inode's filesystem.

I don't think this is the right approach.

When we had the conversation earlier this was conceptually rejected as it prevents nfs superblock unification.

Quoted 6 lines

We really want to store this in the vfsmount and pass the user namespace down from there to where we are going to use it if at all possible.

The vfsmount also appears necessary if we are ever going to support multiple user namespaces per filesystem as the filesystem still need to know which user namespace to interpret it's data in.

Would this require passing the vfsmount to the filesystems themselves,
or would they be within the VFS code only? If not wholly within the VFS I wonder if Al Viro would object to this. He's resisted past attempts to pass the vfsmount structs into more filesystem code paths and I'm guessing that could affect whether or not this approach can be implemented.

Cheers,
-Matt Helsley

A message that is not in the archive (sent off-list or not yet received): 1217285230.25300.19.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/[email protected]

#19 lore raw

Re: [PATCH 4/6] user namespaces: add user_ns to super block

Quoted 2 lines · Matt Helsley

Would this require passing the vfsmount to the filesystems themselves,
or would they be within the VFS code only?

The interesting bit is the user_namespace contained in the vfsmount. We can pass that down. I think semantically it makes sense for a filesystem mount to only operate in a single mount namespace.

Quoted 5 lines

If not wholly within the VFS
I wonder if Al Viro would object to this. He's resisted past attempts to pass the vfsmount structs into more filesystem code paths and I'm guessing that could affect whether or not this approach can be implemented.

Dave Hansen raised that concern when we were talking about it earlier. Since we just care about a property of the mount it isn't a big deal.

Actually thinking about this a little farther it may be simplest to have the mnt_namespace capture the user_namespace, although that doesn't seem to map semantically very well with cloning of the filesystem.

This is very much a question of how do we map the uid/gids store in the filesystem into the uids/gids in the kernel. Which user namespace do they belong in.

Especially in the case of read only mounts we can safely share a filesystem between
user_namespaces with no changes to the filesystem. Which I suspect is the
first case we want to allow as that is a tremendous savings in space if you have lots of instances of the same distro, and people have been doing it with /usr for years.

Eric

A message that is not in the archive (sent off-list or not yet received): [email protected]

#21 lore raw

Re: [PATCH 4/6] user namespaces: add user_ns to super block

Quoted 15 lines · Eric W. Biederman
Quoted 2 lines · Matt Helsley

Would this require passing the vfsmount to the filesystems themselves,
or would they be within the VFS code only?

The interesting bit is the user_namespace contained in the vfsmount. We can pass that down. I think semantically it makes sense for a filesystem mount to only operate in a single mount namespace.

Quoted 5 lines

If not wholly within the VFS
I wonder if Al Viro would object to this. He's resisted past attempts to pass the vfsmount structs into more filesystem code paths and I'm guessing that could affect whether or not this approach can be implemented.

Quoted 6 lines

Dave Hansen raised that concern when we were talking about it earlier. Since we just care about a property of the mount it isn't a big deal.

Actually thinking about this a little farther it may be simplest to have the mnt_namespace capture the user_namespace, although that doesn't seem to map semantically very well with cloning of the filesystem.

Interesting idea. I'm going to pursue that.

So at a do_new_mount(), mnt->user_ns = current->user_ns. At do_loopback(), we ask the fs whether the new_mnt->user_ns can be set to current->user_ns. If not, it keeps the original, meaning that current will always receive user nobody access to the fs. Otherwise, the fs is saying that it knows how to properly convert userids from current->user->user_ns to ones which make sense in the original_mnt->user_ns.

Quoted 10 lines

This is very much a question of how do we map the uid/gids store in the filesystem into the uids/gids in the kernel. Which user namespace do they belong in.

Especially in the case of read only mounts we can safely share a filesystem between
user_namespaces with no changes to the filesystem. Which I suspect is the
first case we want to allow as that is a tremendous savings in space if you have lots of instances of the same distro, and people have been doing it with /usr for years.

Eric

#22 lore raw

Re: [PATCH 4/6] user namespaces: add user_ns to super block

In reply to a message not in the archive
Quoted 20 lines · Matt Helsley
Quoted 18 lines · Eric W. Biederman
Quoted 10 lines · Serge E. Hallyn
Quoted 1 line

From 420d6e81ce29d7a6fe3ab7b43c1171e105f8b697 Mon Sep 17 00:00:00 2001

From: Serge Hallyn <serue-r/[email protected]>
Date: Thu, 24 Jul 2008 18:00:54 -0500
Subject: [PATCH 4/6] user namespaces: add user_ns to super block

Add a user_ns to the super_block, and set it to the user_ns of the process which mounted the fs.

In generic_permission() compare the current user_ns to that of the user_ns which mounted the inode's filesystem.

I don't think this is the right approach.

When we had the conversation earlier this was conceptually rejected as it prevents nfs superblock unification.

Quoted 6 lines
Quoted 6 lines

We really want to store this in the vfsmount and pass the user namespace down from there to where we are going to use it if at all possible.

The vfsmount also appears necessary if we are ever going to support multiple user namespaces per filesystem as the filesystem still need to know which user namespace to interpret it's data in.

The filesystem can figure that out based on current's context, no?

With the per-sb user_ns, the default behavior is indeed very limited, but since you want to move all the user_ns functionality into the filesystem, the fs can tag vfsmounts based on the "new remount" you had talked about before.

Quoted 6 lines

Would this require passing the vfsmount to the filesystems themselves,
or would they be within the VFS code only? If not wholly within the VFS I wonder if Al Viro would object to this. He's resisted past attempts to pass the vfsmount structs into more filesystem code paths and I'm guessing that could affect whether or not this approach can be implemented.

Right, that's the main reason we might want to pursue the per-sb approach. Otherwise I would prefer the per-vfsmount approach.

Eric, if you think the per-vfsmount fight is worth fighting, then by all means let's do it and see what happens. So in that case ignore patches 3-5 from this set :)

-serge

A message that is not in the archive (sent off-list or not yet received): 20080729180515.GB365-r/[email protected]

#24 lore raw

Re: [PATCH 4/6] user namespaces: add user_ns to super block

Quoted 7 lines · Serge E. Hallyn

The filesystem can figure that out based on current's context, no?

With the per-sb user_ns, the default behavior is indeed very limited, but since you want to move all the user_ns functionality into the filesystem, the fs can tag vfsmounts based on the "new remount" you had talked about before.

I guess I want the filesystem to coordinate.

Quoted 7 lines
Quoted 6 lines

Would this require passing the vfsmount to the filesystems themselves,
or would they be within the VFS code only? If not wholly within the VFS I wonder if Al Viro would object to this. He's resisted past attempts to pass the vfsmount structs into more filesystem code paths and I'm guessing that could affect whether or not this approach can be implemented.

Quoted 6 lines

Right, that's the main reason we might want to pursue the per-sb approach. Otherwise I would prefer the per-vfsmount approach.

Eric, if you think the per-vfsmount fight is worth fighting, then by all means let's do it and see what happens. So in that case ignore patches 3-5 from this set :)

My intuitive sense is that the treating the handling of different user namespaces in the same filesystem is a trivial case of the superblock merging that nfs performs. And that we will preserve existing semantics much better if the user namespace is stored in the vfsmount. This allows mount propagation and friends to work without surprises.

The practical limitation I see of storing things outside of the vfsmount is when do you setup the mapping to handle a new user namespace?

So yes. I think it is worth the discussion. Let's not move the vfsmount down, and just move the user namespace pointer down as that is fundamentally what we care about.

Eric

A message that is not in the archive (sent off-list or not yet received): [email protected]

#26 lore raw

Re: [PATCH 4/6] user namespaces: add user_ns to super block

Quoted 37 lines · Eric W. Biederman
Quoted 7 lines · Serge E. Hallyn

The filesystem can figure that out based on current's context, no?

With the per-sb user_ns, the default behavior is indeed very limited, but since you want to move all the user_ns functionality into the filesystem, the fs can tag vfsmounts based on the "new remount" you had talked about before.

I guess I want the filesystem to coordinate.

Quoted 7 lines
Quoted 6 lines

Would this require passing the vfsmount to the filesystems themselves,
or would they be within the VFS code only? If not wholly within the VFS I wonder if Al Viro would object to this. He's resisted past attempts to pass the vfsmount structs into more filesystem code paths and I'm guessing that could affect whether or not this approach can be implemented.

Quoted 6 lines

Right, that's the main reason we might want to pursue the per-sb approach. Otherwise I would prefer the per-vfsmount approach.

Eric, if you think the per-vfsmount fight is worth fighting, then by all means let's do it and see what happens. So in that case ignore patches 3-5 from this set :)

My intuitive sense is that the treating the handling of different user namespaces in the same filesystem is a trivial case of the superblock merging that nfs performs. And that we will preserve existing semantics much better if the user namespace is stored in the vfsmount. This allows mount propagation and friends to work without surprises.

The practical limitation I see of storing things outside of the vfsmount is when do you setup the mapping to handle a new user namespace?

Quoted 5 lines

So yes. I think it is worth the discussion. Let's not move the vfsmount down, and just move the user namespace pointer down as that is fundamentally what we care about.

Eric

Ok I wasn't thinking right. We still can't get to a user_ns from an inode *.

So playing with this a bit tonight, it seems like the best way to pass the user_namespace up to the fs is just to define new super_operations which handle the conversions. Something like the following.

(This sits on top of the two patches I'm about to send out as replacements for patches 1 and 2 from my previous posting.)

-serge

From 0156ae52302a70302b1725450fae7f565195410a Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serue-r/[email protected]>
Date: Fri, 1 Aug 2008 18:55:59 -0500
Subject: [PATCH 3/3] user namespaces: enforce user namespaces for file permission

Add a user_ns to the sb. It is always set to the user_ns of the task which mounted the sb.

Define 3 new super_operations. convert_uid() and convert_gid() take a uid or gid from an inode on the sb's fs, and attempt to convert them into ids meaningful in the user namespace passed in, which presumably is current's. is_capable() checks whether current has the requested capability with respect to the sb's fs, which is dependent upon the relationship between current's user_ns and those which the sb knows about.

If the sb isn't user ns aware - which none are right now - then the new super_operations won't be defined. If in that case current and sb have different user namespaces, then the userids can't be compared.

If current's and sb's userids can't be compared, then current will get 'user other' (we usually say 'nobody') permissions to the inode.

---
 fs/namei.c         |   33 ++++++++++++++++++++++---
 fs/super.c         |    3 ++
 include/linux/fs.h |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+), 4 deletions(-)
fs/namei.c +29 -4
index 4ea63ed..e17bc96 100644
@@ -183,10 +183,32 @@ int generic_permission(struct inode *inode, int mask,
 		int (*check_acl)(struct inode *inode, int mask))
 {
 	umode_t			mode = inode->i_mode;
+	uid_t			iuid;
+	gid_t			igid;
+	int			ret;
+	int			good_userns = 1;
+	struct			super_block *sb = inode->i_sb;
+
+	ret = s_convert_uid(sb, current->user->user_ns,
+						inode->i_uid, &iuid);
+	if (!ret)
+		good_userns = 0;
+	ret = s_convert_gid(sb, current->user->user_ns,
+						inode->i_gid, &igid);
+	if (!ret)
+		good_userns = 0;
 
 	mask &= MAY_READ | MAY_WRITE | MAY_EXEC;
 
-	if (current->fsuid == inode->i_uid)
+	/*
+	 * If we're not in the inode's user namespace, we get
+	 * user nobody permissions, and we ignore acls
+	 * (bc serge doesn't know how to handle acls in this case)
+	 */
+	if (!good_userns)
+		goto check;
+
+	if (current->fsuid == iuid)
 		mode >>= 6;
 	else {
 		if (IS_POSIXACL(inode) && (mode & S_IRWXG) && check_acl) {
@@ -197,15 +219,18 @@ int generic_permission(struct inode *inode, int mask,
 				return error;
 		}
 
-		if (in_group_p(inode->i_gid))
+		if (in_group_p(igid))
 			mode >>= 3;
 	}
 
+check:
 	/*
 	 * If the DACs are ok we don't need any capability check.
 	 */
 	if ((mask & ~mode) == 0)
 		return 0;
+	if (!good_userns)
+		return -EACCES;
 
  check_capabilities:
 	/*
@@ -214,14 +239,14 @@ int generic_permission(struct inode *inode, int mask,
 	 */
 	if (!(mask & MAY_EXEC) ||
 	    (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode))
-		if (capable(CAP_DAC_OVERRIDE))
+		if (s_is_capable(sb, current->user->user_ns, CAP_DAC_OVERRIDE))
 			return 0;
 
 	/*
 	 * Searching includes executable on directories, else just read.
 	 */
 	if (mask == MAY_READ || (S_ISDIR(inode->i_mode) && !(mask & MAY_WRITE)))
-		if (capable(CAP_DAC_READ_SEARCH))
+		if (s_is_capable(sb, current->user->user_ns, CAP_DAC_READ_SEARCH))
 			return 0;
 
 	return -EACCES;
fs/super.c +3 -0
index e931ae9..3559637 100644
@@ -38,6 +38,7 @@
 #include <linux/kobject.h>
 #include <linux/mutex.h>
 #include <linux/file.h>
+#include <linux/user_namespace.h>
 #include <asm/uaccess.h>
 #include "internal.h"
 
@@ -93,6 +94,7 @@ static struct super_block *alloc_super(struct file_system_type *type)
 		s->s_qcop = sb_quotactl_ops;
 		s->s_op = &default_op;
 		s->s_time_gran = 1000000000;
+		s->user_ns = get_user_ns(current->user->user_ns);
 	}
 out:
 	return s;
@@ -109,6 +111,7 @@ static inline void destroy_super(struct super_block *s)
 	security_sb_free(s);
 	kfree(s->s_subtype);
 	kfree(s->s_options);
+	put_user_ns(s->user_ns);
 	kfree(s);
 }
 
include/linux/fs.h +68 -0

86 lines not on this page · raw message

Signature · 1 line
1.5.4.3

A message that is not in the archive (sent off-list or not yet received): 20080802000609.GA10211-r/[email protected]

#28 lore raw

Re: [PATCH 4/6] user namespaces: add user_ns to super block

Quoted 7 lines · Serge E. Hallyn

Ok I wasn't thinking right. We still can't get to a user_ns from an inode *.

So playing with this a bit tonight, it seems like the best way to pass the user_namespace up to the fs is just to define new super_operations which handle the conversions. Something like the following.

I'm pretty certain you want to pass in the entire inode instead of just inode->i_uid. That would allow you to pass one less argument and give the filesystems more information to work with.

Eric

A message that is not in the archive (sent off-list or not yet received): [email protected]

#30 lore raw

Re: [PATCH 4/6] user namespaces: add user_ns to super block

Quoted 10 lines · Eric W. Biederman
Quoted 7 lines · Serge E. Hallyn

Ok I wasn't thinking right. We still can't get to a user_ns from an inode *.

So playing with this a bit tonight, it seems like the best way to pass the user_namespace up to the fs is just to define new super_operations which handle the conversions. Something like the following.

Quoted 5 lines

I'm pretty certain you want to pass in the entire inode instead of just inode->i_uid. That would allow you to pass one less argument and give the filesystems more information to work with.

Eric

Oh yes of course, the fs will want the whole inode. Will change that.

And I suppose in that case I may as well have just one function to convert both uid and gid.

Thanks.

-serge

#31 lore raw

PATCH 5/6 user namespaces: refuse create in other user_ns

In reply to a message not in the archive

From 4d2c23452a67e25856893ab16fefd0f6e5aa58df Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serue-r/[email protected]>
Date: Thu, 24 Jul 2008 06:37:43 -0500
Subject: [PATCH 5/6] user namespaces: refuse create in other user_ns

Refuse writing to a directory in another user_ns. We can't support file creation because we wouldn't know who should own the file. This refuses file deletion as well - which I think is the sensible thing to do.

File writing is still allowed if the 'user other' permissions include write. That again probably makes sense for logging and such.

---
 fs/namei.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
fs/namei.c +6 -0
index adf5f1b..b39a990 100644
@@ -213,6 +213,12 @@ int generic_permission(struct inode *inode, int mask,
 
 check:
 	/*
+	 * Can't write to a directory in another user_ns
+	 * We wouldn't know who to make the owner!
+	 */
+	if (!same_userns && S_ISDIR(inode->i_mode) && (mask&MAY_WRITE))
+		return -EACCES;
+	/*
 	 * If the DACs are ok we don't need any capability check.
 	 */
 	if (((mode & mask & (MAY_READ|MAY_WRITE|MAY_EXEC)) == mask))
Signature · 1 line
1.5.4.3
#32 lore raw

PATCH 6/6 user_namespace: move put_user_ns outside lock

In reply to a message not in the archive

Oops, this one was supposed to get folded into the first patch, obviously. Sorry.

-serge

From daa76939c72adf146ded8a39e0211886e2bc943a Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serue-r/[email protected]>
Date: Fri, 25 Jul 2008 14:24:32 -0500
Subject: [PATCH 6/6] user_namespace: move put_user_ns outside lock

Since put_user_ns calls free_user on the creator, we shouldn't call it from inside the lock in free_user.

---
 kernel/user.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/user.c +1 -1
index 034dae5..d3dd353 100644
@@ -323,11 +323,11 @@ done:
  */
 static inline void free_user(struct user_struct *up, unsigned long flags)
 {
-	put_user_ns(up->user_ns);
 	/* restore back the count */
 	atomic_inc(&up->__count);
 	spin_unlock_irqrestore(&uidhash_lock, flags);
 
+	put_user_ns(up->user_ns);
 	INIT_WORK(&up->work, remove_user_sysfs_dir);
 	schedule_work(&up->work);
 }
Signature · 1 line
1.5.4.3