* ns_can_attach (nsproxy cgroup)
@ 2008-12-12 9:51 Grzegorz Nosek
[not found] ` <20081212095153.GA20956-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Grzegorz Nosek @ 2008-12-12 9:51 UTC (permalink / raw)
To: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
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.
Best regards,
Grzegorz Nosek
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ns_can_attach (nsproxy cgroup)
[not found] ` <20081212095153.GA20956-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
@ 2008-12-12 14:09 ` Serge E. Hallyn
[not found] ` <20081212140908.GA9571-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Serge E. Hallyn @ 2008-12-12 14:09 UTC (permalink / raw)
To: Grzegorz Nosek; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
Quoting Grzegorz Nosek (root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org):
> 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.
thanks,
-serge
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ns_can_attach (nsproxy cgroup)
[not found] ` <20081212140908.GA9571-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
@ 2008-12-12 14:23 ` Grzegorz Nosek
2008-12-12 21:30 ` Grzegorz Nosek
1 sibling, 0 replies; 10+ messages in thread
From: Grzegorz Nosek @ 2008-12-12 14:23 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Grzegorz Nosek
On Fri, Dec 12, 2008 at 08:09:08AM -0600, Serge E. Hallyn wrote:
> Quoting Grzegorz Nosek (root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org):
> > 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? :)
Yeah, but I found a patch by Andrea Righi which fits my use case almost
100% [1]. The tasks are moved between cgroups by the kernel, so it's
kernel hacking for me either way and I guess modifying ns_can_attach
will be cleaner.
> But I can't think of any reason why it would be a problem. So
> pls feel free to send a patch.
Will do.
Best regards,
Grzegorz Nosek
[1] http://lwn.net/Articles/294364/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ns_can_attach (nsproxy cgroup)
[not found] ` <20081212140908.GA9571-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-12-12 14:23 ` Grzegorz Nosek
@ 2008-12-12 21:30 ` Grzegorz Nosek
[not found] ` <20081212213042.GA23581-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
1 sibling, 1 reply; 10+ messages in thread
From: Grzegorz Nosek @ 2008-12-12 21:30 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Grzegorz Nosek
On pią, gru 12, 2008 at 08:09:08 -0600, Serge E. Hallyn wrote:
> Quoting Grzegorz Nosek (root@localdomain.pl):
> > 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 <root@localdomain.pl>
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 <root@localdomain.pl>
---
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
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: ns_can_attach (nsproxy cgroup)
[not found] ` <20081212213042.GA23581-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
@ 2008-12-13 7:03 ` Li Zefan
[not found] ` <49435E58.9050209-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Li Zefan @ 2008-12-13 7:03 UTC (permalink / raw)
To: Grzegorz Nosek; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
> /**
> - * 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()
The last argument is redundant.
[...]
> - orig = task_cgroup(task, ns_subsys_id);
> - if (orig && orig != new_cgroup->parent)
> + if (!cgroup_is_descendant(new_cgroup, task, ns_subsys_id))
It makes no difference with:
if (!cgroup_is_descendant(new_cgroup, task, -1))
That's why we don't need argument '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);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ns_can_attach (nsproxy cgroup)
[not found] ` <49435E58.9050209-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
@ 2008-12-14 14:06 ` Grzegorz Nosek
[not found] ` <20081214140604.GA22768-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Grzegorz Nosek @ 2008-12-14 14:06 UTC (permalink / raw)
To: Li Zefan
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Grzegorz Nosek
On sob, gru 13, 2008 at 03:03:52 +0800, Li Zefan wrote:
> > /**
> > - * 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()
>
> The last argument is redundant.
>
> [...]
>
> It makes no difference with:
> if (!cgroup_is_descendant(new_cgroup, task, -1))
>
> That's why we don't need argument 'subsys_id'.
Thanks, I thought so (the "first" subsys_id looked arbitrary to me) but not knowing
the cgroup internals I wanted to preserve the original code as much as possible.
Will update the patch.
Best regards,
Grzegorz Nosek
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ns_can_attach (nsproxy cgroup)
[not found] ` <20081214140604.GA22768-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
@ 2008-12-16 23:08 ` Grzegorz Nosek
[not found] ` <20081216230808.GA26026-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Grzegorz Nosek @ 2008-12-16 23:08 UTC (permalink / raw)
To: Grzegorz Nosek; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
On nie, gru 14, 2008 at 03:06:04 +0100, Grzegorz Nosek wrote:
> On sob, gru 13, 2008 at 03:03:52 +0800, Li Zefan wrote:
> > > /**
> > > - * 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()
> >
> > The last argument is redundant.
> >
> > [...]
> >
> > It makes no difference with:
> > if (!cgroup_is_descendant(new_cgroup, task, -1))
> >
> > That's why we don't need argument 'subsys_id'.
>
> Thanks, I thought so (the "first" subsys_id looked arbitrary to me) but not knowing
> the cgroup internals I wanted to preserve the original code as much as possible.
> Will update the patch.
As promised, here's the updated patch.
From 2abb608222f3dfd1c5da3022a75c824f2b00b3f5 Mon Sep 17 00:00:00 2001
From: Grzegorz Nosek <root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org>
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 <root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org>
---
include/linux/cgroup.h | 4 ++--
kernel/cgroup.c | 12 +++++++-----
kernel/ns_cgroup.c | 9 +++------
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 1ba3ded..53188f5 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -301,8 +301,8 @@ 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 */
+int cgroup_is_descendant(const struct cgroup *cgrp, struct task_struct *task);
/* Control Group subsystem type. See Documentation/cgroups.txt for details */
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 2ae040d..5512d78 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -3101,18 +3101,19 @@ 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
*
- * 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 ret;
struct cgroup *target;
@@ -3122,7 +3123,8 @@ int cgroup_is_descendant(const struct cgroup *cgrp)
return 1;
get_first_subsys(cgrp, NULL, &subsys_id);
- target = task_cgroup(current, 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..d99b4cc 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))
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))
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))
return ERR_PTR(-EPERM);
ns_cgroup = kzalloc(sizeof(*ns_cgroup), GFP_KERNEL);
--
1.5.4.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: ns_can_attach (nsproxy cgroup)
[not found] ` <20081216230808.GA26026-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
@ 2008-12-17 0:52 ` Li Zefan
[not found] ` <49484D33.5020308-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Li Zefan @ 2008-12-17 0:52 UTC (permalink / raw)
To: Grzegorz Nosek; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
>>From 2abb608222f3dfd1c5da3022a75c824f2b00b3f5 Mon Sep 17 00:00:00 2001
> From: Grzegorz Nosek <root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org>
> 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
>
Now looks good to me. :)
Reviewed-by: Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
> Signed-off-by: Grzegorz Nosek <root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org>
> ---
> include/linux/cgroup.h | 4 ++--
> kernel/cgroup.c | 12 +++++++-----
> kernel/ns_cgroup.c | 9 +++------
> 3 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index 1ba3ded..53188f5 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -301,8 +301,8 @@ 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 */
> +int cgroup_is_descendant(const struct cgroup *cgrp, struct task_struct *task);
>
> /* Control Group subsystem type. See Documentation/cgroups.txt for details */
>
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 2ae040d..5512d78 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -3101,18 +3101,19 @@ 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
> *
> - * 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 ret;
> struct cgroup *target;
> @@ -3122,7 +3123,8 @@ int cgroup_is_descendant(const struct cgroup *cgrp)
> return 1;
>
> get_first_subsys(cgrp, NULL, &subsys_id);
> - target = task_cgroup(current, 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..d99b4cc 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))
> 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))
> 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))
> return ERR_PTR(-EPERM);
>
> ns_cgroup = kzalloc(sizeof(*ns_cgroup), GFP_KERNEL);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ns_can_attach (nsproxy cgroup)
[not found] ` <49484D33.5020308-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
@ 2008-12-18 14:45 ` Grzegorz Nosek
[not found] ` <20081218144506.GA2185-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Grzegorz Nosek @ 2008-12-18 14:45 UTC (permalink / raw)
To: Li Zefan
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Grzegorz Nosek
On śro, gru 17, 2008 at 08:52:03 +0800, Li Zefan wrote:
> >>From 2abb608222f3dfd1c5da3022a75c824f2b00b3f5 Mon Sep 17 00:00:00 2001
> > From: Grzegorz Nosek <root@localdomain.pl>
> > 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
> >
>
> Now looks good to me. :)
>
> Reviewed-by: Li Zefan <lizf@cn.fujitsu.com>
Thanks, shall I send the patch somewhere else for some mainstream tree
inclusion or is somebody willing to pick it up from here?
Best regards,
Grzegorz Nosek
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ns_can_attach (nsproxy cgroup)
[not found] ` <20081218144506.GA2185-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
@ 2008-12-19 0:43 ` Li Zefan
0 siblings, 0 replies; 10+ messages in thread
From: Li Zefan @ 2008-12-19 0:43 UTC (permalink / raw)
To: Grzegorz Nosek; +Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
>> Now looks good to me. :)
>>
>> Reviewed-by: Li Zefan <lizf-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
>
> Thanks, shall I send the patch somewhere else for some mainstream tree
> inclusion or is somebody willing to pick it up from here?
>
You can resend this patch to Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
and CC Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> since the patch touches cgroup.c,
and feel free to add my reviewed tag. :)
> Best regards,
> Grzegorz Nosek
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2008-12-19 0:43 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-12 9:51 ns_can_attach (nsproxy cgroup) Grzegorz Nosek
[not found] ` <20081212095153.GA20956-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
2008-12-12 14:09 ` Serge E. Hallyn
[not found] ` <20081212140908.GA9571-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-12-12 14:23 ` Grzegorz Nosek
2008-12-12 21:30 ` Grzegorz Nosek
[not found] ` <20081212213042.GA23581-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
2008-12-13 7:03 ` Li Zefan
[not found] ` <49435E58.9050209-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2008-12-14 14:06 ` Grzegorz Nosek
[not found] ` <20081214140604.GA22768-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
2008-12-16 23:08 ` Grzegorz Nosek
[not found] ` <20081216230808.GA26026-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
2008-12-17 0:52 ` Li Zefan
[not found] ` <49484D33.5020308-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2008-12-18 14:45 ` Grzegorz Nosek
[not found] ` <20081218144506.GA2185-yp6mvK3Bdd2rDJvtcaxF/A@public.gmane.org>
2008-12-19 0:43 ` Li Zefan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.