| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20080820155909.cf3f73bc.kamezawa.hiroyu@jp.fujitsu.com>
Date: Wed, 20 Aug 2008 15:59:09 +0900
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To: Li Zefan <lizf@...fujitsu.com>
Cc: Jiri Slaby <jirislaby@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
containers@...ts.linux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] mm_owner: fix cgroup null dereference
On Wed, 20 Aug 2008 13:20:40 +0800
Li Zefan <lizf@...fujitsu.com> wrote:
> Balbir Singh wote:
> > * Jiri Slaby <jirislaby@...il.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@...fujitsu.com>
>
Thank you, it seems you fixed a problem which I met in my test.
(*) When I migrate task under cpuset repeatedly, I can see Oops easily.
Tested-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...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@...il.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@...ux.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
> > _
> >
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@...ck.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@...ck.org"> email@...ck.org </a>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists