lkml.org 
[lkml]   [2008]   [Aug]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 1/1] mm_owner: fix cgroup null dereference
Balbir Singh wote:
> * Jiri Slaby <jirislaby@gmail.com> [2008-08-14 22:16:53]:
>
>> Hi,
>>
>> found this in mmotm, a fix for
>> mm-owner-fix-race-between-swap-and-exit.patch
>>
>
> Does the patch below fix your problem, it's against mmotm 19th August
> 2008.
>

I just triggered this bug. I think you also need the following change:

make memrlimit_cgroup_mm_owner_changed() aware that old_cgrp can be NULL,
and note we can't call memrlimit_cgroup_from_cgrp() with NULL argument.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

diff --git a/mm/memrlimitcgroup.c.orig b/mm/memrlimitcgroup.c
index f7536dc..6559470 100644
--- a/mm/memrlimitcgroup.c.orig
+++ b/mm/memrlimitcgroup.c
@@ -249,17 +249,23 @@ static void memrlimit_cgroup_mm_owner_changed(struct cgroup_subsys *ss,
struct mm_struct *mm = get_task_mm(p);

BUG_ON(!mm);
- memrcg = memrlimit_cgroup_from_cgrp(cgrp);
- old_memrcg = memrlimit_cgroup_from_cgrp(old_cgrp);

/*
* If we don't have a new cgroup, we just uncharge from the old one.
* It means that the task is going away
*/
- if (memrcg &&
- res_counter_charge(&memrcg->as_res, (mm->total_vm << PAGE_SHIFT)))
- goto out;
- res_counter_uncharge(&old_memrcg->as_res, (mm->total_vm << PAGE_SHIFT));
+ if (cgrp) {
+ memrcg = memrlimit_cgroup_from_cgrp(cgrp);
+ if (res_counter_charge(&memrcg->as_res,
+ mm->total_vm << PAGE_SHIFT))
+ goto out;
+ }
+
+ if (old_cgrp) {
+ old_memrcg = memrlimit_cgroup_from_cgrp(old_cgrp);
+ res_counter_uncharge(&old_memrcg->as_res,
+ mm->total_vm <<PAGE_SHIFT);
+ }
out:
mmput(mm);
}

>
> Reported-by: jirislaby@gmail.com
>
> Jiri reported a problem and saw an oops when the memrlimit-fix-race-with-swap
> patch is applied. He sent his patch on top to fix the problem, but ran into
> another issue. The root cause of the problem is that we are not suppose
> to call task_cgroup on NULL tasks. This patch reverts Jiri's patch and
> does not call task_cgroup if the passed task_struct (old) is NULL.
>
>
> Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
> ---
>
> kernel/cgroup.c | 5 +++--
> kernel/exit.c | 2 +-
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff -puN kernel/exit.c~memrlimit-fix-race-with-swap-oops kernel/exit.c
> --- linux-2.6.27-rc3/kernel/exit.c~memrlimit-fix-race-with-swap-oops 2008-08-19 18:50:39.000000000 +0530
> +++ linux-2.6.27-rc3-balbir/kernel/exit.c 2008-08-19 18:51:05.000000000 +0530
> @@ -641,8 +641,8 @@ retry:
> * the callback and take action
> */
> down_write(&mm->mmap_sem);
> - cgroup_mm_owner_callbacks(mm->owner, NULL);
> mm->owner = NULL;
> + cgroup_mm_owner_callbacks(mm->owner, NULL);
> up_write(&mm->mmap_sem);
> return;
>
> diff -puN kernel/cgroup.c~memrlimit-fix-race-with-swap-oops kernel/cgroup.c
> --- linux-2.6.27-rc3/kernel/cgroup.c~memrlimit-fix-race-with-swap-oops 2008-08-19 18:50:39.000000000 +0530
> +++ linux-2.6.27-rc3-balbir/kernel/cgroup.c 2008-08-19 18:55:38.000000000 +0530
> @@ -2743,13 +2743,14 @@ void cgroup_fork_callbacks(struct task_s
> */
> void cgroup_mm_owner_callbacks(struct task_struct *old, struct task_struct *new)
> {
> - struct cgroup *oldcgrp, *newcgrp = NULL;
> + struct cgroup *oldcgrp = NULL, *newcgrp = NULL;
>
> if (need_mm_owner_callback) {
> int i;
> for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
> struct cgroup_subsys *ss = subsys[i];
> - oldcgrp = task_cgroup(old, ss->subsys_id);
> + if (old)
> + oldcgrp = task_cgroup(old, ss->subsys_id);
> if (new)
> newcgrp = task_cgroup(new, ss->subsys_id);
> if (oldcgrp == newcgrp)
> diff -puN mm/memrlimitcgroup.c~memrlimit-fix-race-with-swap-oops mm/memrlimitcgroup.c
> _
>



\
 
 \ /
  Last update: 2008-08-20 07:25    [W:0.133 / U:0.388 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site