On pią, gru 12, 2008 at 08:09:08 -0600, Serge E. Hallyn wrote: > Quoting Grzegorz Nosek ([email protected]): > > Hi all, > > > > Is there a good reason for ns_can_attach to restrict moving tasks only > > to direct descentants of the current cgroup? I.e. could the code: > > > > orig = task_cgroup(task, ns_subsys_id); > > if (orig && orig != new_cgroup->parent) > > return -EPERM; > > > > be replaced with: > > > > orig = task_cgroup(task, ns_subsys_id); > > if (orig && !cgroup_is_descendant_of(new_cgroup, orig)) > > return -EPERM; > > > > (for a suitable definition of cgroup_is_descendant_of). It would allow > > moving tasks down the cgroup hierarchy more than one level at a time and > > as far as I can see, would pose no additional problems. > > > > Please keep CC'd, I'm not subscribed. > > Well you can always move it down one level at a time, right? :) > > But I can't think of any reason why it would be a problem. So > pls feel free to send a patch.
OK, here's the patch. After some basic testing looks like it's working as advertised. Please have a look at the patch, esp. the subsys_id part, I'm not sure I got the code/comments all right. Best regards, Grzegorz Nosek -------------------------------------- >From 8750df0d341c3227a0ed33088602addec003b713 Mon Sep 17 00:00:00 2001 From: Grzegorz Nosek <[email protected]> Date: Fri, 12 Dec 2008 21:48:09 +0100 Subject: [PATCH] Relax ns_can_attach checks to allow attaching to grandchild cgroups The ns_proxy cgroup allows moving processes to child cgroups only one level deep at a time. This commit relaxes this restriction and makes it possible to attach tasks directly to grandchild cgroups, e.g.: ($pid is in the root cgroup) echo $pid > /cgroup/CG1/CG2/tasks Previously this operation would fail with -EPERM and would have to be performed as two steps: echo $pid > /cgroup/CG1/tasks echo $pid > /cgroup/CG1/CG2/tasks Signed-off-by: Grzegorz Nosek <[email protected]> --- include/linux/cgroup.h | 5 +++-- kernel/cgroup.c | 19 ++++++++++++------- kernel/ns_cgroup.c | 9 +++------ 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h index 1ba3ded..ab8a5cd 100644 --- a/include/linux/cgroup.h +++ b/include/linux/cgroup.h @@ -301,8 +301,9 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen); int cgroup_task_count(const struct cgroup *cgrp); -/* Return true if the cgroup is a descendant of the current cgroup */ -int cgroup_is_descendant(const struct cgroup *cgrp); +/* Return true if cgrp is a descendant of the task's cgroup subsystem subsys_id */ +int cgroup_is_descendant(const struct cgroup *cgrp, struct task_struct *task, + int subsys_id); /* Control Group subsystem type. See Documentation/cgroups.txt for details */ diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 2ae040d..0002fa5 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -3101,28 +3101,33 @@ int cgroup_clone(struct task_struct *tsk, struct cgroup_subsys *subsys, } /** - * cgroup_is_descendant - see if @cgrp is a descendant of current task's cgrp + * cgroup_is_descendant - see if @cgrp is a descendant of @task's cgrp * @cgrp: the cgroup in question + * @task: the task in question + * @subsys_id: cgroup subsystem id used to determine hierarchy; if negative, + * use get_first_subsys() * - * See if @cgrp is a descendant of the current task's cgroup in - * the appropriate hierarchy. + * See if @cgrp is a descendant of @task's cgroup in the appropriate + * hierarchy. * * If we are sending in dummytop, then presumably we are creating * the top cgroup in the subsystem. * * Called only by the ns (nsproxy) cgroup. */ -int cgroup_is_descendant(const struct cgroup *cgrp) +int cgroup_is_descendant(const struct cgroup *cgrp, struct task_struct *task, + int subsys_id) { int ret; struct cgroup *target; - int subsys_id; if (cgrp == dummytop) return 1; - get_first_subsys(cgrp, NULL, &subsys_id); - target = task_cgroup(current, subsys_id); + if (subsys_id < 0) + get_first_subsys(cgrp, NULL, &subsys_id); + + target = task_cgroup(task, subsys_id); while (cgrp != target && cgrp!= cgrp->top_cgroup) cgrp = cgrp->parent; ret = (cgrp == target); diff --git a/kernel/ns_cgroup.c b/kernel/ns_cgroup.c index 43c2111..5247d84 100644 --- a/kernel/ns_cgroup.c +++ b/kernel/ns_cgroup.c @@ -46,21 +46,18 @@ int ns_cgroup_clone(struct task_struct *task, struct pid *pid) static int ns_can_attach(struct cgroup_subsys *ss, struct cgroup *new_cgroup, struct task_struct *task) { - struct cgroup *orig; - if (current != task) { if (!capable(CAP_SYS_ADMIN)) return -EPERM; - if (!cgroup_is_descendant(new_cgroup)) + if (!cgroup_is_descendant(new_cgroup, current, -1)) return -EPERM; } if (atomic_read(&new_cgroup->count) != 0) return -EPERM; - orig = task_cgroup(task, ns_subsys_id); - if (orig && orig != new_cgroup->parent) + if (!cgroup_is_descendant(new_cgroup, task, ns_subsys_id)) return -EPERM; return 0; @@ -78,7 +75,7 @@ static struct cgroup_subsys_state *ns_create(struct cgroup_subsys *ss, if (!capable(CAP_SYS_ADMIN)) return ERR_PTR(-EPERM); - if (!cgroup_is_descendant(cgroup)) + if (!cgroup_is_descendant(cgroup, current, -1)) return ERR_PTR(-EPERM); ns_cgroup = kzalloc(sizeof(*ns_cgroup), GFP_KERNEL); -- 1.5.4.4 _______________________________________________ Containers mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/containers _______________________________________________ Devel mailing list [email protected] https://openvz.org/mailman/listinfo/devel
