cgroups: fix a serious bug in cgroupstats
PATCH 1/1 cgroups: fix a serious bug in cgroupstats
Try this, and you'll get oops immediately:
# cd Documentation/accounting/ # gcc -o getdelays getdelays.c # mount -t cgroup -o debug xxx /mnt # ./getdelays -C /mnt/tasks
Because a normal file's dentry->d_fsdata is a pointer to struct cftype, not struct cgroup.
After the patch, it returns EINVAL if we try to get cgroupstats from a normal file.
--- kernel/cgroup.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
kernel/cgroup.c +5 -2
@@ -2039,10 +2039,13 @@ int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry)struct cgroup *cgrp;struct cgroup_iter it;struct task_struct *tsk; +/* -* Validate dentry by checking the superblock operations +* Validate dentry by checking the superblock operations, +* and make sure it's a directory. */ -if (dentry->d_sb->s_op != &cgroup_ops) +if (dentry->d_sb->s_op != &cgroup_ops || + !S_ISDIR (dentry->d_inode->i_mode)) goto err; ret =0 ;
Signature · 1 line
1.5.4.rc3
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 1 line · On Wed, 19 Nov 2008 16:14:41 +0800 Li Zefan…
Try this, and you'll get oops immediately:
I guess that's wrong.
Quoted 27 lines
# cd Documentation/accounting/ # gcc -o getdelays getdelays.c # mount -t cgroup -o debug xxx /mnt # ./getdelays -C /mnt/tasksBecause a normal file's
dentry->d_fsdatais a pointer to struct cftype, not struct cgroup.After the patch, it returns EINVAL if we try to get cgroupstats from a normal file.
CC Balbir Singh <[email protected]>Signed-off-by Li Zefan <[email protected]>--- kernel/cgroup.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)kernel/cgroup.c +1
-0@@ -2039,10 +2039,13 @@ int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry)struct cgroup *cgrp;struct cgroup_iter it;struct task_struct *tsk; +/*
kernel/cgroup.c int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry) · line 2051
Quoted 10 lines
kernel/cgroup.c +4
-2-* Validate dentry by checking the superblock operations +* Validate dentry by checking the superblock operations, +* and make sure it's a directory. */ -if (dentry->d_sb->s_op != &cgroup_ops) +if (dentry->d_sb->s_op != &cgroup_ops || + !S_ISDIR (dentry->d_inode->i_mode)) goto err; ret =0 ;
The patch applies OK to 2.6.25, 2.6.26 and to 2.6.27. I marked it as needing backport to those kernel versions. Please let me know if that was inappropriate.
Paul Menage
Acked-byQuoted 6 lines · Li Zefan
Try this, and you'll get oops immediately:
# cd Documentation/accounting/ # gcc -o getdelays getdelays.c # mount -t cgroup -o debug xxx /mnt # ./getdelays -C /mnt/tasks
Quoted 8 lines
Because a normal file's
dentry->d_fsdatais a pointer to struct cftype, not struct cgroup.After the patch, it returns EINVAL if we try to get cgroupstats from a normal file.
CC Balbir Singh <[email protected]>Signed-off-by Li Zefan <[email protected]>
Good catch, thanks.
Quoted 27 lines
--- kernel/cgroup.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)kernel/cgroup.c +5
-3@@ -2039,10 +2039,13 @@ int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry)struct cgroup *cgrp;struct cgroup_iter it;struct task_struct *tsk; +/* -* Validate dentry by checking the superblock operations +* Validate dentry by checking the superblock operations, +* and make sure it's a directory. */ -if (dentry->d_sb->s_op != &cgroup_ops) +if (dentry->d_sb->s_op != &cgroup_ops || + !S_ISDIR (dentry->d_inode->i_mode)) goto err; ret =0 ; --1.5.4.rc3
Quoted 1 line · On Wed, 19 Nov 2008 16:14:41 +0800 Li Zefan <[email protected]> wrote:
Try this, and you'll get oops immediately:
I guess that's wrong.
Quoted 27 lines
# cd Documentation/accounting/ # gcc -o getdelays getdelays.c # mount -t cgroup -o debug xxx /mnt # ./getdelays -C /mnt/tasksBecause a normal file's
dentry->d_fsdatais a pointer to struct cftype, not struct cgroup.After the patch, it returns EINVAL if we try to get cgroupstats from a normal file.
CC Balbir Singh <[email protected]>Signed-off-by Li Zefan <[email protected]>--- kernel/cgroup.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)kernel/cgroup.c +1
-0@@ -2039,10 +2039,13 @@ int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry)struct cgroup *cgrp;struct cgroup_iter it;struct task_struct *tsk; +/*
kernel/cgroup.c int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry) · line 2051
Quoted 10 lines
kernel/cgroup.c +4
-2-* Validate dentry by checking the superblock operations +* Validate dentry by checking the superblock operations, +* and make sure it's a directory. */ -if (dentry->d_sb->s_op != &cgroup_ops) +if (dentry->d_sb->s_op != &cgroup_ops || + !S_ISDIR (dentry->d_inode->i_mode)) goto err; ret =0 ;
The patch applies OK to 2.6.25, 2.6.26 and to 2.6.27. I marked it as needing backport to those kernel versions. Please let me know if that was inappropriate.
A message that is not in the archive (sent off-list or not yet received): 20081119002552.3840e3d2.akpm-de/[email protected]
Quoted 4 lines
The patch applies OK to 2.6.25, 2.6.26 and to 2.6.27. I marked it as needing backport to those kernel versions. Please let me know if that was inappropriate.
Forgot to mention this:
The bug exists from 2.6.24 (when cgroupstats is introduced) to now, though I didn't test in all those versions. I guess not many people used cgroupstats, so it never gets triggered.
I think it can be applied to 2.6.24 too.
Quoted 42 lines · Andrew Morton
Quoted 1 line · On Wed, 19 Nov 2008 16:14:41 +0800 Li Zefan…
Try this, and you'll get oops immediately:
I guess that's wrong.
Quoted 36 lines
# cd Documentation/accounting/ # gcc -o getdelays getdelays.c # mount -t cgroup -o debug xxx /mnt # ./getdelays -C /mnt/tasksBecause a normal file's
dentry->d_fsdatais a pointer to struct cftype, not struct cgroup.After the patch, it returns EINVAL if we try to get cgroupstats from a normal file.
CC Balbir Singh <[email protected]>Signed-off-by Li Zefan <[email protected]>--- kernel/cgroup.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)kernel/cgroup.c +5
-2@@ -2039,10 +2039,13 @@ int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry)struct cgroup *cgrp;struct cgroup_iter it;struct task_struct *tsk; +/* -* Validate dentry by checking the superblock operations +* Validate dentry by checking the superblock operations, +* and make sure it's a directory. */ -if (dentry->d_sb->s_op != &cgroup_ops) +if (dentry->d_sb->s_op != &cgroup_ops || + !S_ISDIR (dentry->d_inode->i_mode)) goto err;
Quoted 6 lines
kernel/cgroup.c int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry) · line 2051
Quoted 1 line
kernel/cgroup.c +0
-0ret =0 ;The patch applies OK to 2.6.25, 2.6.26 and to 2.6.27. I marked it as needing backport to those kernel versions. Please let me know if that was inappropriate.
Thanks, it does need to be backported. Thanks Li for spotting the problem.
Signature · 1 line
Balbir
Quoted 4 lines
The patch applies OK to 2.6.25, 2.6.26 and to 2.6.27. I marked it as needing backport to those kernel versions. Please let me know if that was inappropriate.
Forgot to mention this:
The bug exists from 2.6.24 (when cgroupstats is introduced) to now, though I didn't test in all those versions. I guess not many people used cgroupstats, so it never gets triggered.
I think it can be applied to 2.6.24 too.
Quoted 42 lines · Andrew Morton
Quoted 1 line · On Wed, 19 Nov 2008 16:14:41 +0800 Li Zefan <[email protected]> wrote:
Try this, and you'll get oops immediately:
I guess that's wrong.
Quoted 36 lines
# cd Documentation/accounting/ # gcc -o getdelays getdelays.c # mount -t cgroup -o debug xxx /mnt # ./getdelays -C /mnt/tasksBecause a normal file's
dentry->d_fsdatais a pointer to struct cftype, not struct cgroup.After the patch, it returns EINVAL if we try to get cgroupstats from a normal file.
CC Balbir Singh <[email protected]>Signed-off-by Li Zefan <[email protected]>--- kernel/cgroup.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)kernel/cgroup.c +5
-2@@ -2039,10 +2039,13 @@ int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry)struct cgroup *cgrp;struct cgroup_iter it;struct task_struct *tsk; +/* -* Validate dentry by checking the superblock operations +* Validate dentry by checking the superblock operations, +* and make sure it's a directory. */ -if (dentry->d_sb->s_op != &cgroup_ops) +if (dentry->d_sb->s_op != &cgroup_ops || + !S_ISDIR (dentry->d_inode->i_mode)) goto err;
Quoted 6 lines
kernel/cgroup.c int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry) · line 2051
Quoted 1 line
kernel/cgroup.c +0
-0ret =0 ;The patch applies OK to 2.6.25, 2.6.26 and to 2.6.27. I marked it as needing backport to those kernel versions. Please let me know if that was inappropriate.
Thanks, it does need to be backported. Thanks Li for spotting the problem.
Signature · 1 line
Balbir
Quoted 6 lines · Li Zefan
Try this, and you'll get oops immediately:
# cd Documentation/accounting/ # gcc -o getdelays getdelays.c # mount -t cgroup -o debug xxx /mnt # ./getdelays -C /mnt/tasks
Quoted 8 lines
Because a normal file's
dentry->d_fsdatais a pointer to struct cftype, not struct cgroup.After the patch, it returns EINVAL if we try to get cgroupstats from a normal file.
CC Balbir Singh <[email protected]>Signed-off-by Li Zefan <[email protected]>
Good catch, thanks.
Quoted 27 lines
--- kernel/cgroup.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)kernel/cgroup.c +5
-3@@ -2039,10 +2039,13 @@ int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry)struct cgroup *cgrp;struct cgroup_iter it;struct task_struct *tsk; +/* -* Validate dentry by checking the superblock operations +* Validate dentry by checking the superblock operations, +* and make sure it's a directory. */ -if (dentry->d_sb->s_op != &cgroup_ops) +if (dentry->d_sb->s_op != &cgroup_ops || + !S_ISDIR (dentry->d_inode->i_mode)) goto err; ret =0 ; --1.5.4.rc3