Skip to contentSkip to conversation

introduce user_ns inheritance in user-sched

PATCH In review Serge E. Hallyn 5 participants 8 messages Mar 19, 2009 – Mar 22, 2009

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 introduce user_ns inheritance in user-sched

In a kernel compiled with CONFIG_USER_SCHED=y, cpu shares are allocated according to uid. Shares are specifiable under /sys/kernel/uids/<uid>/

In a kernel compiled with CONFIG_USER_NS=y, clone(2) with the CLONE_NEWUSER flag creates a new user namespace, and the newly cloned task will belong to uid 0 in the new user namespace.

Without this patch, if uid 500 calls clone(CLONE_NEWUSER) (which is possible using a program with the cap_sys_admin,cap_setuid,cap_setgid=pe file capabilities), then the new task will get the cpu shares of uid 0.

After this patch, if uid 500 calls clone(CLONE_NEWUSER), then even though it is uid 0 in the new user namespace, it will be restricted to the cpu shares of uid 500.

Currently there is no way to set shares for uids in user namespaces other than the initial one. That will be trivial to add when sysfs tagging (or its functional equivalent, also needed to expose network devices in network namespaces other than init) becomes available.

Until cross-user-namespace file accesses are enforced, nothing stops uid 0 in a child namespace from simply writing new values into /sys/kernel/uids/500.

Here are results of some testing with and without the patch.

Cpu shares are initialized as follows::

	user root:   2048
	user hallyn: 1024
	user serge:  512

Results are the 'real' part of time make -j4 > o 2>&1, each time after a make clean.

=================================================================
UNPATCHED
User 1: user serge creates a child user_ns and runs as user root User 2: hallyn runs as user hallyn
=================================================================
User 1 User 2

run 1:   2m58.834s        3m0.609s
run 2:   2m59.248s        2m59.457s

=============================================================
PATCHED
User 1: user serge
User 2: user hallyn
=============================================================

User 1 User 2

run 1:   3m6.337s        2m22.681s
run 2:   3m6.323s        2m21.855s

=============================================================
PATCHED
User 1: user serge setuid to user root
User 2: hallyn
=============================================================

User 1 User 2

run 1:   2m17.782s       3m3.947s
run 2:   2m18.497s       3m7.961s

==========================================================
PATCHED
User 1: user root inside userns created by userid serge User 2: hallyn
==========================================================

User 1 User 2

run 1:   3m9.876s        2m8.428s
run 2:   3m8.539s        2m6.356s
---
 kernel/user.c           |   12 +++++++++---
 kernel/user_namespace.c |    2 +-
 2 files changed, 10 insertions(+), 4 deletions(-)
kernel/user.c +9 -3
index 850e0ba..53aeea2 100644
@@ -101,7 +101,12 @@ static int sched_create_user(struct user_struct *up)
 {
 	int rc = 0;
 
-	up->tg = sched_create_group(&root_task_group);
+	struct task_group *parent = &root_task_group;
+
+	if (up->user_ns != &init_user_ns)
+		parent = up->user_ns->creator->tg;
+
+	up->tg = sched_create_group(parent);
 	if (IS_ERR(up->tg))
 		rc = -ENOMEM;
 
@@ -434,11 +439,11 @@ struct user_struct *alloc_uid(struct user_namespace *ns, uid_t uid)
 		new->uid = uid;
 		atomic_set(&new->__count, 1);
 
+		new->user_ns = get_user_ns(ns);
+
 		if (sched_create_user(new) < 0)
 			goto out_free_user;
 
-		new->user_ns = get_user_ns(ns);
-
 		if (uids_user_create(new))
 			goto out_destoy_sched;
 
@@ -472,6 +477,7 @@ out_destoy_sched:
 	sched_destroy_user(new);
 	put_user_ns(new->user_ns);
 out_free_user:
+	put_user_ns(new->user_ns);
 	kmem_cache_free(uid_cachep, new);
 out_unlock:
 	uids_mutex_unlock();
kernel/user_namespace.c +1 -1
index 076c7c8..a99d3c7 100644
@@ -35,6 +35,7 @@ int create_user_ns(struct cred *new)
 		INIT_HLIST_HEAD(ns->uidhash_table + n);
 
 	/* Alloc new root user.  */
+	ns->creator = new->user;
 	root_user = alloc_uid(ns, 0);
 	if (!root_user) {
 		kfree(ns);
@@ -42,7 +43,6 @@ int create_user_ns(struct cred *new)
 	}
 
 	/* set the new root user in the credentials under preparation */
-	ns->creator = new->user;
 	new->user = root_user;
 	new->uid = new->euid = new->suid = new->fsuid = 0;
 	new->gid = new->egid = new->sgid = new->fsgid = 0;
Signature · 1 line
1.5.4.3
#1 lore raw
Quoted 119 lines · Serge E. Hallyn

In a kernel compiled with CONFIG_USER_SCHED=y, cpu shares are allocated according to uid. Shares are specifiable under /sys/kernel/uids/<uid>/

In a kernel compiled with CONFIG_USER_NS=y, clone(2) with the CLONE_NEWUSER flag creates a new user namespace, and the newly cloned task will belong to uid 0 in the new user namespace.

Without this patch, if uid 500 calls clone(CLONE_NEWUSER) (which is possible using a program with the cap_sys_admin,cap_setuid,cap_setgid=pe file capabilities), then the new task will get the cpu shares of uid 0.

After this patch, if uid 500 calls clone(CLONE_NEWUSER), then even though it is uid 0 in the new user namespace, it will be restricted to the cpu shares of uid 500.

Currently there is no way to set shares for uids in user namespaces other than the initial one. That will be trivial to add when sysfs tagging (or its functional equivalent, also needed to expose network devices in network namespaces other than init) becomes available.

Until cross-user-namespace file accesses are enforced, nothing stops uid 0 in a child namespace from simply writing new values into /sys/kernel/uids/500.

Here are results of some testing with and without the patch.

Cpu shares are initialized as follows::

	user root:   2048
	user hallyn: 1024
	user serge:  512

Results are the 'real' part of time make -j4 > o 2>&1, each time after a make clean.

=================================================================
UNPATCHED
User 1: user serge creates a child user_ns and runs as user root User 2: hallyn runs as user hallyn
=================================================================
User 1 User 2

run 1:   2m58.834s        3m0.609s
run 2:   2m59.248s        2m59.457s

=============================================================
PATCHED
User 1: user serge
User 2: user hallyn
=============================================================

User 1 User 2

run 1:   3m6.337s        2m22.681s
run 2:   3m6.323s        2m21.855s

=============================================================
PATCHED
User 1: user serge setuid to user root
User 2: hallyn
=============================================================

User 1 User 2

run 1:   2m17.782s       3m3.947s
run 2:   2m18.497s       3m7.961s

==========================================================
PATCHED
User 1: user root inside userns created by userid serge User 2: hallyn
==========================================================

User 1 User 2

run 1:   3m9.876s        2m8.428s
run 2:   3m8.539s        2m6.356s
---
 kernel/user.c           |   12 +++++++++---
 kernel/user_namespace.c |    2 +-
 2 files changed, 10 insertions(+), 4 deletions(-)
kernel/user.c +8 -3
index 850e0ba..53aeea2 100644
@@ -101,7 +101,12 @@ static int sched_create_user(struct user_struct *up)
 {
 	int rc = 0;
 
-	up->tg = sched_create_group(&root_task_group);
+	struct task_group *parent = &root_task_group;
+
+	if (up->user_ns != &init_user_ns)
+		parent = up->user_ns->creator->tg;
+
+	up->tg = sched_create_group(parent);
 	if (IS_ERR(up->tg))
 		rc = -ENOMEM;
 
@@ -434,11 +439,11 @@ struct user_struct *alloc_uid(struct user_namespace *ns, uid_t uid)
 		new->uid = uid;
 		atomic_set(&new->__count, 1);
 
+		new->user_ns = get_user_ns(ns);
+
 		if (sched_create_user(new) < 0)
 			goto out_free_user;
 
-		new->user_ns = get_user_ns(ns);
-
 		if (uids_user_create(new))
 			goto out_destoy_sched;
 

kernel/user.c out_destoy_sched: · line 478

Quoted 2 lines
kernel/user.c +0 -0
@@ -472,6 +477,7 @@ out_destoy_sched:
 	sched_destroy_user(new);
 	put_user_ns(new->user_ns);

Shouldn't this put_user_ns(new->user_ns) be removed? It looks like two references to new->user_ns are being dropped if anything fails after sched_create_user(new) succeeds yet as far as I can tell the patch does not introduce any new references to new->user_ns.

Otherwise looks good to me.

Cheers,
-Matt Helsley

#2 lore raw
Quoted 119 lines · Matt Helsley
Quoted 118 lines · Serge E. Hallyn

In a kernel compiled with CONFIG_USER_SCHED=y, cpu shares are allocated according to uid. Shares are specifiable under /sys/kernel/uids/<uid>/

In a kernel compiled with CONFIG_USER_NS=y, clone(2) with the CLONE_NEWUSER flag creates a new user namespace, and the newly cloned task will belong to uid 0 in the new user namespace.

Without this patch, if uid 500 calls clone(CLONE_NEWUSER) (which is possible using a program with the cap_sys_admin,cap_setuid,cap_setgid=pe file capabilities), then the new task will get the cpu shares of uid 0.

After this patch, if uid 500 calls clone(CLONE_NEWUSER), then even though it is uid 0 in the new user namespace, it will be restricted to the cpu shares of uid 500.

Currently there is no way to set shares for uids in user namespaces other than the initial one. That will be trivial to add when sysfs tagging (or its functional equivalent, also needed to expose network devices in network namespaces other than init) becomes available.

Until cross-user-namespace file accesses are enforced, nothing stops uid 0 in a child namespace from simply writing new values into /sys/kernel/uids/500.

Here are results of some testing with and without the patch.

Cpu shares are initialized as follows::

	user root:   2048
	user hallyn: 1024
	user serge:  512

Results are the 'real' part of time make -j4 > o 2>&1, each time after a make clean.

=================================================================
UNPATCHED
User 1: user serge creates a child user_ns and runs as user root User 2: hallyn runs as user hallyn
=================================================================
User 1 User 2

run 1:   2m58.834s        3m0.609s
run 2:   2m59.248s        2m59.457s

=============================================================
PATCHED
User 1: user serge
User 2: user hallyn
=============================================================

User 1 User 2

run 1:   3m6.337s        2m22.681s
run 2:   3m6.323s        2m21.855s

=============================================================
PATCHED
User 1: user serge setuid to user root
User 2: hallyn
=============================================================

User 1 User 2

run 1:   2m17.782s       3m3.947s
run 2:   2m18.497s       3m7.961s

==========================================================
PATCHED
User 1: user root inside userns created by userid serge User 2: hallyn
==========================================================

User 1 User 2

run 1:   3m9.876s        2m8.428s
run 2:   3m8.539s        2m6.356s
---
 kernel/user.c           |   12 +++++++++---
 kernel/user_namespace.c |    2 +-
 2 files changed, 10 insertions(+), 4 deletions(-)
kernel/user.c +8 -3
index 850e0ba..53aeea2 100644
@@ -101,7 +101,12 @@ static int sched_create_user(struct user_struct *up)
 {
 	int rc = 0;
 
-	up->tg = sched_create_group(&root_task_group);
+	struct task_group *parent = &root_task_group;
+
+	if (up->user_ns != &init_user_ns)
+		parent = up->user_ns->creator->tg;
+
+	up->tg = sched_create_group(parent);
 	if (IS_ERR(up->tg))
 		rc = -ENOMEM;
 
@@ -434,11 +439,11 @@ struct user_struct *alloc_uid(struct user_namespace *ns, uid_t uid)
 		new->uid = uid;
 		atomic_set(&new->__count, 1);
 
+		new->user_ns = get_user_ns(ns);
+
 		if (sched_create_user(new) < 0)
 			goto out_free_user;
 
-		new->user_ns = get_user_ns(ns);
-
 		if (uids_user_create(new))
 			goto out_destoy_sched;
 
Quoted 8 lines

kernel/user.c out_destoy_sched: · line 478

Quoted 3 lines
kernel/user.c +0 -0
@@ -472,6 +477,7 @@ out_destoy_sched:
 	sched_destroy_user(new);
 	put_user_ns(new->user_ns);

Shouldn't this put_user_ns(new->user_ns) be removed? It looks like two references to new->user_ns are being dropped if anything fails after sched_create_user(new) succeeds yet as far as I can tell the patch does not introduce any new references to new->user_ns.

Ouch, yeah, thought I'd done that...

Thanks for catching that! Will resend.

Quoted 4 lines

Otherwise looks good to me.

Cheers,
-Matt Helsley

thanks,
-serge

#3 lore raw
In reply to Matt Helsley
Quoted 6 lines · Matt Helsley

Shouldn't this put_user_ns(new->user_ns) be removed? It looks like two references to new->user_ns are being dropped if anything fails after sched_create_user(new) succeeds yet as far as I can tell the patch does not introduce any new references to new->user_ns.

Otherwise looks good to me.

Here is the new version. Thanks again.

From 55c264b27cb1f6f91007ae2aeda2d4f6067bb2eb Mon Sep 17 00:00:00 2001
From: Serge E. Hallyn <[email protected]>
Date: Wed, 18 Mar 2009 13:29:32 -0700
Subject: [PATCH] introduce user_ns inheritance in user-sched (v2)

In a kernel compiled with CONFIG_USER_SCHED=y, cpu shares are allocated according to uid. Shares are specifiable under /sys/kernel/uids/

In a kernel compiled with CONFIG_USER_NS=y, clone(2) with the CLONE_NEWUSER flag creates a new user namespace, and the newly cloned task will belong to uid 0 in the new user namespace.

Without this patch, if uid 500 calls clone(CLONE_NEWUSER) (which is possible using a program with the cap_sys_admin,cap_setuid,cap_setgid=pe file capabilities), then the new task will get the cpu shares of uid 0.

After this patch, if uid 500 calls clone(CLONE_NEWUSER), then even though it is uid 0 in the new user namespace, it will be restricted to the cpu shares of uid 500.

Currently there is no way to set shares for uids in user namespaces other than the initial one. That will be trivial to add when sysfs tagging (or its functional equivalent, also needed to expose network devices in network namespaces other than init) becomes available.

Until cross-user-namespace file accesses are enforced, nothing stops uid 0 in a child namespace from simply writing new values into /sys/kernel/uids/500.

Here are results of some testing with and without the patch.

Cpu shares are initialized as follows::

	user root:   2048
	user hallyn: 1024
	user serge:  512

Results are the 'real' part of time make -j4 > o 2>&1, each time after a make clean.

=================================================================
UNPATCHED
User 1: user serge creates a child user_ns and runs as user root User 2: hallyn runs as user hallyn
=================================================================
User 1 User 2

run 1:   2m58.834s        3m0.609s
run 2:   2m59.248s        2m59.457s

=============================================================
PATCHED
User 1: user serge
User 2: user hallyn
=============================================================

User 1 User 2

run 1:   3m6.337s        2m22.681s
run 2:   3m6.323s        2m21.855s

=============================================================
PATCHED
User 1: user serge setuid to user root
User 2: hallyn
=============================================================

User 1 User 2

run 1:   2m17.782s       3m3.947s
run 2:   2m18.497s       3m7.961s

==========================================================
PATCHED
User 1: user root inside userns created by userid serge User 2: hallyn
==========================================================

User 1 User 2

run 1:   3m9.876s        2m8.428s
run 2:   3m8.539s        2m6.356s

Changelog:
Mar 19: Matt Helsley pointed out there were
two calls to put_user_ns() in alloc_uid() error path.

---
 kernel/user.c           |   13 +++++++++----
 kernel/user_namespace.c |    2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)
kernel/user.c +9 -4
index 850e0ba..8ae4bf8 100644
@@ -101,7 +101,12 @@ static int sched_create_user(struct user_struct *up)
 {
 	int rc = 0;
 
-	up->tg = sched_create_group(&root_task_group);
+	struct task_group *parent = &root_task_group;
+
+	if (up->user_ns != &init_user_ns)
+		parent = up->user_ns->creator->tg;
+
+	up->tg = sched_create_group(parent);
 	if (IS_ERR(up->tg))
 		rc = -ENOMEM;
 
@@ -434,11 +439,11 @@ struct user_struct *alloc_uid(struct user_namespace *ns, uid_t uid)
 		new->uid = uid;
 		atomic_set(&new->__count, 1);
 
+		new->user_ns = get_user_ns(ns);
+
 		if (sched_create_user(new) < 0)
 			goto out_free_user;
 
-		new->user_ns = get_user_ns(ns);
-
 		if (uids_user_create(new))
 			goto out_destoy_sched;
 
@@ -470,8 +475,8 @@ struct user_struct *alloc_uid(struct user_namespace *ns, uid_t uid)
 
 out_destoy_sched:
 	sched_destroy_user(new);
-	put_user_ns(new->user_ns);
 out_free_user:
+	put_user_ns(new->user_ns);
 	kmem_cache_free(uid_cachep, new);
 out_unlock:
 	uids_mutex_unlock();
kernel/user_namespace.c +1 -1
index 076c7c8..a99d3c7 100644
@@ -35,6 +35,7 @@ int create_user_ns(struct cred *new)
 		INIT_HLIST_HEAD(ns->uidhash_table + n);
 
 	/* Alloc new root user.  */
+	ns->creator = new->user;
 	root_user = alloc_uid(ns, 0);
 	if (!root_user) {
 		kfree(ns);
@@ -42,7 +43,6 @@ int create_user_ns(struct cred *new)
 	}
 
 	/* set the new root user in the credentials under preparation */
-	ns->creator = new->user;
 	new->user = root_user;
 	new->uid = new->euid = new->suid = new->fsuid = 0;
 	new->gid = new->egid = new->sgid = new->fsgid = 0;
Signature · 1 line
1.5.4.3
#4 lore raw
In reply to Serge E. Hallyn
Quoted 7 lines · Serge E. Hallyn

In a kernel compiled with CONFIG_USER_SCHED=y, cpu shares are allocated according to uid. Shares are specifiable under /sys/kernel/uids/<uid>/

In a kernel compiled with CONFIG_USER_NS=y, clone(2) with the CLONE_NEWUSER flag creates a new user namespace, and the newly cloned task will belong to uid 0 in the new user namespace.

We seem to be adding more and more stuff for USER_SCHED, is anybody actually using that cruft?

How far along with cgroups are we to fully simulate that behaviour?

I think if we have a capable cgroup based replacement for USER_SCHED we should axe it from the kernel, would save lots of code...

#5 lore raw
Quoted 9 lines · Peter Zijlstra
Quoted 7 lines · Serge E. Hallyn

In a kernel compiled with CONFIG_USER_SCHED=y, cpu shares are allocated according to uid. Shares are specifiable under /sys/kernel/uids/<uid>/

In a kernel compiled with CONFIG_USER_NS=y, clone(2) with the CLONE_NEWUSER flag creates a new user namespace, and the newly cloned task will belong to uid 0 in the new user namespace.

Quoted 7 lines

We seem to be adding more and more stuff for USER_SCHED, is anybody actually using that cruft?

How far along with cgroups are we to fully simulate that behaviour?

I think if we have a capable cgroup based replacement for USER_SCHED we should axe it from the kernel, would save lots of code...

I didn't realize that was the plan. Using PAM to move users around cgroups? If so, then yeah that would simplify quite a bit
of code. Won't catch all setuid()s of course - I don't know
who uses USER_SCHED and if that would matter.

-serge

#6 lore raw
Quoted 13 lines · Serge E. Hallyn
Quoted 12 lines · Peter Zijlstra
Quoted 7 lines · Serge E. Hallyn

In a kernel compiled with CONFIG_USER_SCHED=y, cpu shares are allocated according to uid. Shares are specifiable under /sys/kernel/uids/<uid>/

In a kernel compiled with CONFIG_USER_NS=y, clone(2) with the CLONE_NEWUSER flag creates a new user namespace, and the newly cloned task will belong to uid 0 in the new user namespace.

We seem to be adding more and more stuff for USER_SCHED, is anybody actually using that cruft?

Quoted 7 lines
Quoted 4 lines

How far along with cgroups are we to fully simulate that behaviour?

I think if we have a capable cgroup based replacement for USER_SCHED we should axe it from the kernel, would save lots of code...

I didn't realize that was the plan. Using PAM to move users around cgroups?

Right, thing is, distro's will all want cgroup enabled, since that's the latest fad :-), so this user sched thing will only be for people who build their own kernels -- but I suspect most of those simply disable all this group scheduling.

Quoted 2 lines

If so, then yeah that would simplify quite a bit
of code. Won't catch all setuid()s of course

Right, so if we could somehow get a setuid notification hooked into cgroups,.. not sure that would be worth the trouble though.

Quoted 1 line

- I don't know who uses USER_SCHED and if that would matter.

Right, me neither... I would just love to be able to cut all that code out :-)

#7 lore raw
Quoted 19 lines · Peter Zijlstra
Quoted 18 lines · Serge E. Hallyn
Quoted 16 lines · Peter Zijlstra
Quoted 7 lines · Serge E. Hallyn

In a kernel compiled with CONFIG_USER_SCHED=y, cpu shares are allocated according to uid. Shares are specifiable under /sys/kernel/uids/<uid>/

In a kernel compiled with CONFIG_USER_NS=y, clone(2) with the CLONE_NEWUSER flag creates a new user namespace, and the newly cloned task will belong to uid 0 in the new user namespace.

We seem to be adding more and more stuff for USER_SCHED, is anybody actually using that cruft?

How far along with cgroups are we to fully simulate that behaviour?

I think if we have a capable cgroup based replacement for USER_SCHED we should axe it from the kernel, would save lots of code...

Quoted 8 lines
Quoted 2 lines

I didn't realize that was the plan. Using PAM to move users around cgroups?

Right, thing is, distro's will all want cgroup enabled, since that's the latest fad :-), so this user sched thing will only be for people who build their own kernels -- but I suspect most of those simply disable all this group scheduling.

But if they do not, then the behavior is wrong now, and I would prefer it to be fixed, which is why this patch.

Quoted 6 lines
Quoted 2 lines

If so, then yeah that would simplify quite a bit
of code. Won't catch all setuid()s of course

Right, so if we could somehow get a setuid notification hooked into cgroups,.. not sure that would be worth the trouble though.

Does anyone really care about uid based grouping? (I do realize the userspace daemon classifies on the basis of uids as of now, but still, how many use cases really want uid based grouping as opposed to process type (as in browser, compiler..) type of grouping)

Quoted 5 lines
Quoted 1 line

- I don't know who uses USER_SCHED and if that would matter.

Right, me neither... I would just love to be able to cut all that code out :-)

me too :)

Signature · 2 lines
regards,
Dhaval