* [PATCH 1/2] cgroups: Fix to return errno in a failure path
@ 2010-01-26 8:16 Li Zefan
2010-01-26 8:17 ` [PATCH 2/2] cgroups: Clean up cgroup_pidlist_find() a bit Li Zefan
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Li Zefan @ 2010-01-26 8:16 UTC (permalink / raw)
To: Andrew Morton
Cc: Paul Menage, KAMEZAWA Hiroyuki, LKML, containers@lists.osdl.org
In cgroup_create(), if alloc_css_id() returns failure, the errno
is not propagated to userspace, so mkdir will fail silently.
To trigger this bug, we mount blkio (or memory subsystem), and
create more then 65534 cgroups. (The number of cgroups is limited
to 65535 if a subsystem has use_id == 1)
# mount -t cgroup -o blkio xxx /mnt
# for ((i = 0; i < 65534; i++)); do mkdir /mnt/$i; done
# mkdir /mnt/65534
(should return ENOSPC)
#
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
cgroup.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
--- a/kernel/cgroup.c.orig 2010-01-19 16:37:37.000000000 +0800
+++ a/kernel/cgroup.c 2010-01-19 16:39:07.000000000 +0800
@@ -3279,14 +3279,17 @@ static long cgroup_create(struct cgroup
for_each_subsys(root, ss) {
struct cgroup_subsys_state *css = ss->create(ss, cgrp);
+
if (IS_ERR(css)) {
err = PTR_ERR(css);
goto err_destroy;
}
init_cgroup_css(css, ss, cgrp);
- if (ss->use_id)
- if (alloc_css_id(ss, parent, cgrp))
+ if (ss->use_id) {
+ err = alloc_css_id(ss, parent, cgrp);
+ if (err)
goto err_destroy;
+ }
/* At error, ->destroy() callback has to free assigned ID. */
}
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/2] cgroups: Clean up cgroup_pidlist_find() a bit
2010-01-26 8:16 [PATCH 1/2] cgroups: Fix to return errno in a failure path Li Zefan
@ 2010-01-26 8:17 ` Li Zefan
2010-01-26 23:08 ` Paul Menage
2010-01-26 14:50 ` [PATCH 1/2] cgroups: Fix to return errno in a failure path Serge E. Hallyn
2010-01-26 23:01 ` Paul Menage
2 siblings, 1 reply; 6+ messages in thread
From: Li Zefan @ 2010-01-26 8:17 UTC (permalink / raw)
To: Li Zefan
Cc: Andrew Morton, Paul Menage, Ben Blum, LKML,
containers@lists.osdl.org
Don't Call get_pid_ns() before we locate/alloc the ns.
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
cgroup.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
--- a/kernel/cgroup.c.orig 2010-01-26 14:24:29.000000000 +0800
+++ a/kernel/cgroup.c 2010-01-26 14:24:44.000000000 +0800
@@ -2643,7 +2643,8 @@ static struct cgroup_pidlist *cgroup_pid
{
struct cgroup_pidlist *l;
/* don't need task_nsproxy() if we're looking at ourself */
- struct pid_namespace *ns = get_pid_ns(current->nsproxy->pid_ns);
+ struct pid_namespace *ns = current->nsproxy->pid_ns;
+
/*
* We can't drop the pidlist_mutex before taking the l->mutex in case
* the last ref-holder is trying to remove l from the list at the same
@@ -2653,8 +2654,6 @@ static struct cgroup_pidlist *cgroup_pid
mutex_lock(&cgrp->pidlist_mutex);
list_for_each_entry(l, &cgrp->pidlists, links) {
if (l->key.type == type && l->key.ns == ns) {
- /* found a matching list - drop the extra refcount */
- put_pid_ns(ns);
/* make sure l doesn't vanish out from under us */
down_write(&l->mutex);
mutex_unlock(&cgrp->pidlist_mutex);
@@ -2665,13 +2664,12 @@ static struct cgroup_pidlist *cgroup_pid
l = kmalloc(sizeof(struct cgroup_pidlist), GFP_KERNEL);
if (!l) {
mutex_unlock(&cgrp->pidlist_mutex);
- put_pid_ns(ns);
return l;
}
init_rwsem(&l->mutex);
down_write(&l->mutex);
l->key.type = type;
- l->key.ns = ns;
+ l->key.ns = get_pid_ns(ns);
l->use_count = 0; /* don't increment here */
l->list = NULL;
l->owner = cgrp;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] cgroups: Clean up cgroup_pidlist_find() a bit
2010-01-26 8:17 ` [PATCH 2/2] cgroups: Clean up cgroup_pidlist_find() a bit Li Zefan
@ 2010-01-26 23:08 ` Paul Menage
0 siblings, 0 replies; 6+ messages in thread
From: Paul Menage @ 2010-01-26 23:08 UTC (permalink / raw)
To: Li Zefan; +Cc: Andrew Morton, Ben Blum, LKML, containers@lists.osdl.org
On Tue, Jan 26, 2010 at 12:17 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
> Don't Call get_pid_ns() before we locate/alloc the ns.
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Paul Menage <menage@google.com>
> ---
> cgroup.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> --- a/kernel/cgroup.c.orig 2010-01-26 14:24:29.000000000 +0800
> +++ a/kernel/cgroup.c 2010-01-26 14:24:44.000000000 +0800
> @@ -2643,7 +2643,8 @@ static struct cgroup_pidlist *cgroup_pid
> {
> struct cgroup_pidlist *l;
> /* don't need task_nsproxy() if we're looking at ourself */
> - struct pid_namespace *ns = get_pid_ns(current->nsproxy->pid_ns);
> + struct pid_namespace *ns = current->nsproxy->pid_ns;
> +
> /*
> * We can't drop the pidlist_mutex before taking the l->mutex in case
> * the last ref-holder is trying to remove l from the list at the same
> @@ -2653,8 +2654,6 @@ static struct cgroup_pidlist *cgroup_pid
> mutex_lock(&cgrp->pidlist_mutex);
> list_for_each_entry(l, &cgrp->pidlists, links) {
> if (l->key.type == type && l->key.ns == ns) {
> - /* found a matching list - drop the extra refcount */
> - put_pid_ns(ns);
> /* make sure l doesn't vanish out from under us */
> down_write(&l->mutex);
> mutex_unlock(&cgrp->pidlist_mutex);
> @@ -2665,13 +2664,12 @@ static struct cgroup_pidlist *cgroup_pid
> l = kmalloc(sizeof(struct cgroup_pidlist), GFP_KERNEL);
> if (!l) {
> mutex_unlock(&cgrp->pidlist_mutex);
> - put_pid_ns(ns);
> return l;
> }
> init_rwsem(&l->mutex);
> down_write(&l->mutex);
> l->key.type = type;
> - l->key.ns = ns;
> + l->key.ns = get_pid_ns(ns);
> l->use_count = 0; /* don't increment here */
> l->list = NULL;
> l->owner = cgrp;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] cgroups: Fix to return errno in a failure path
2010-01-26 8:16 [PATCH 1/2] cgroups: Fix to return errno in a failure path Li Zefan
2010-01-26 8:17 ` [PATCH 2/2] cgroups: Clean up cgroup_pidlist_find() a bit Li Zefan
@ 2010-01-26 14:50 ` Serge E. Hallyn
2010-01-26 23:01 ` Paul Menage
2 siblings, 0 replies; 6+ messages in thread
From: Serge E. Hallyn @ 2010-01-26 14:50 UTC (permalink / raw)
To: Li Zefan; +Cc: Andrew Morton, containers@lists.osdl.org, Paul Menage, LKML
Quoting Li Zefan (lizf@cn.fujitsu.com):
> In cgroup_create(), if alloc_css_id() returns failure, the errno
> is not propagated to userspace, so mkdir will fail silently.
>
> To trigger this bug, we mount blkio (or memory subsystem), and
> create more then 65534 cgroups. (The number of cgroups is limited
> to 65535 if a subsystem has use_id == 1)
>
> # mount -t cgroup -o blkio xxx /mnt
> # for ((i = 0; i < 65534; i++)); do mkdir /mnt/$i; done
> # mkdir /mnt/65534
> (should return ENOSPC)
> #
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Yup.
Acked-by: Serge Hallyn <serue@us.ibm.com>
> ---
> cgroup.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> --- a/kernel/cgroup.c.orig 2010-01-19 16:37:37.000000000 +0800
> +++ a/kernel/cgroup.c 2010-01-19 16:39:07.000000000 +0800
> @@ -3279,14 +3279,17 @@ static long cgroup_create(struct cgroup
>
> for_each_subsys(root, ss) {
> struct cgroup_subsys_state *css = ss->create(ss, cgrp);
> +
> if (IS_ERR(css)) {
> err = PTR_ERR(css);
> goto err_destroy;
> }
> init_cgroup_css(css, ss, cgrp);
> - if (ss->use_id)
> - if (alloc_css_id(ss, parent, cgrp))
> + if (ss->use_id) {
> + err = alloc_css_id(ss, parent, cgrp);
> + if (err)
> goto err_destroy;
> + }
> /* At error, ->destroy() callback has to free assigned ID. */
> }
>
> _______________________________________________
> Containers mailing list
> Containers@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] cgroups: Fix to return errno in a failure path
2010-01-26 8:16 [PATCH 1/2] cgroups: Fix to return errno in a failure path Li Zefan
2010-01-26 8:17 ` [PATCH 2/2] cgroups: Clean up cgroup_pidlist_find() a bit Li Zefan
2010-01-26 14:50 ` [PATCH 1/2] cgroups: Fix to return errno in a failure path Serge E. Hallyn
@ 2010-01-26 23:01 ` Paul Menage
2010-01-27 0:05 ` KAMEZAWA Hiroyuki
2 siblings, 1 reply; 6+ messages in thread
From: Paul Menage @ 2010-01-26 23:01 UTC (permalink / raw)
To: Li Zefan
Cc: Andrew Morton, KAMEZAWA Hiroyuki, LKML, containers@lists.osdl.org
On Tue, Jan 26, 2010 at 12:16 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Paul Menage <menage@google.com>
> ---
> cgroup.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> --- a/kernel/cgroup.c.orig 2010-01-19 16:37:37.000000000 +0800
> +++ a/kernel/cgroup.c 2010-01-19 16:39:07.000000000 +0800
> @@ -3279,14 +3279,17 @@ static long cgroup_create(struct cgroup
>
> for_each_subsys(root, ss) {
> struct cgroup_subsys_state *css = ss->create(ss, cgrp);
> +
> if (IS_ERR(css)) {
> err = PTR_ERR(css);
> goto err_destroy;
> }
> init_cgroup_css(css, ss, cgrp);
> - if (ss->use_id)
> - if (alloc_css_id(ss, parent, cgrp))
> + if (ss->use_id) {
> + err = alloc_css_id(ss, parent, cgrp);
> + if (err)
> goto err_destroy;
> + }
> /* At error, ->destroy() callback has to free assigned ID. */
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] cgroups: Fix to return errno in a failure path
2010-01-26 23:01 ` Paul Menage
@ 2010-01-27 0:05 ` KAMEZAWA Hiroyuki
0 siblings, 0 replies; 6+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-01-27 0:05 UTC (permalink / raw)
To: Paul Menage; +Cc: Li Zefan, Andrew Morton, LKML, containers@lists.osdl.org
On Tue, 26 Jan 2010 15:01:26 -0800
Paul Menage <menage@google.com> wrote:
> On Tue, Jan 26, 2010 at 12:16 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
> >
> > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
>
> Acked-by: Paul Menage <menage@google.com>
>
Thank you.
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > ---
> > cgroup.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > --- a/kernel/cgroup.c.orig 2010-01-19 16:37:37.000000000 +0800
> > +++ a/kernel/cgroup.c 2010-01-19 16:39:07.000000000 +0800
> > @@ -3279,14 +3279,17 @@ static long cgroup_create(struct cgroup
> >
> > for_each_subsys(root, ss) {
> > struct cgroup_subsys_state *css = ss->create(ss, cgrp);
> > +
> > if (IS_ERR(css)) {
> > err = PTR_ERR(css);
> > goto err_destroy;
> > }
> > init_cgroup_css(css, ss, cgrp);
> > - if (ss->use_id)
> > - if (alloc_css_id(ss, parent, cgrp))
> > + if (ss->use_id) {
> > + err = alloc_css_id(ss, parent, cgrp);
> > + if (err)
> > goto err_destroy;
> > + }
> > /* At error, ->destroy() callback has to free assigned ID. */
> > }
> >
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-01-27 0:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-26 8:16 [PATCH 1/2] cgroups: Fix to return errno in a failure path Li Zefan
2010-01-26 8:17 ` [PATCH 2/2] cgroups: Clean up cgroup_pidlist_find() a bit Li Zefan
2010-01-26 23:08 ` Paul Menage
2010-01-26 14:50 ` [PATCH 1/2] cgroups: Fix to return errno in a failure path Serge E. Hallyn
2010-01-26 23:01 ` Paul Menage
2010-01-27 0:05 ` KAMEZAWA Hiroyuki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox