2009-10-29 00:33:09

by Kamezawa Hiroyuki

Subject: [BUGFIX][PATCH] memcg: fix wrong pointer initialization at page migration when memcg is disabled.

Lee Schermerhorn reported that he saw bad pointer dereference
in mem_cgroup_end_migration() when he disabled memcg by boot option.

memcg's page migration logic works as

mem_cgroup_prepare_migration(page, &ptr);
do page migration
mem_cgroup_end_migration(page, ptr);

Now, ptr is not initialized in prepare_migration when memcg is disabled
by boot option. This causes panic in end_migration. This patch fixes it.

Reported-by: Lee Schermerhorn <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: Daisuke Nishimura <[email protected]>
Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
---
mm/memcontrol.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6.32-rc5/mm/memcontrol.c
===================================================================
--- linux-2.6.32-rc5.orig/mm/memcontrol.c
+++ linux-2.6.32-rc5/mm/memcontrol.c
@@ -1990,7 +1990,8 @@ int mem_cgroup_prepare_migration(struct
struct page_cgroup *pc;
struct mem_cgroup *mem = NULL;
int ret = 0;
-
+ /* this pointer will be checked at end_migration */
+ *ptr = NULL;
if (mem_cgroup_disabled())
return 0;

2009-10-29 00:59:13

by Daisuke Nishimura

Subject: Re: [BUGFIX][PATCH] memcg: fix wrong pointer initialization at page migration when memcg is disabled.

On Thu, 29 Oct 2009 09:30:13 +0900, KAMEZAWA Hiroyuki <[email protected]> wrote:
>
> Lee Schermerhorn reported that he saw bad pointer dereference
> in mem_cgroup_end_migration() when he disabled memcg by boot option.
>
> memcg's page migration logic works as
>
> mem_cgroup_prepare_migration(page, &ptr);
> do page migration
> mem_cgroup_end_migration(page, ptr);
>
> Now, ptr is not initialized in prepare_migration when memcg is disabled
> by boot option. This causes panic in end_migration. This patch fixes it.
>
> Reported-by: Lee Schermerhorn <[email protected]>
> Cc: Balbir Singh <[email protected]>
> Cc: Daisuke Nishimura <[email protected]>
> Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
> ---
> mm/memcontrol.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.32-rc5/mm/memcontrol.c
> ===================================================================
> --- linux-2.6.32-rc5.orig/mm/memcontrol.c
> +++ linux-2.6.32-rc5/mm/memcontrol.c
> @@ -1990,7 +1990,8 @@ int mem_cgroup_prepare_migration(struct
> struct page_cgroup *pc;
> struct mem_cgroup *mem = NULL;
> int ret = 0;
> -
> + /* this pointer will be checked at end_migration */
> + *ptr = NULL;
> if (mem_cgroup_disabled())
> return 0;
>
>
I thought unmap_and_move() itself initializes "mem" to NULL, but it doesn't...
I personaly prefer initializing "mem" to NULL in unmap_and_move(), but anyway
I think this patch is also correct.

Reviewed-by: Daisuke Nishimura <[email protected]>

And I think we should send a fix for this bug to -stable too.


Thanks,
Daisuke Nishimura.

2009-10-29 01:07:57

by Kamezawa Hiroyuki

Subject: Re: [BUGFIX][PATCH] memcg: fix wrong pointer initialization at page migration when memcg is disabled.

On Thu, 29 Oct 2009 09:50:51 +0900
Daisuke Nishimura <[email protected]> wrote:
> > Index: linux-2.6.32-rc5/mm/memcontrol.c
> > ===================================================================
> > --- linux-2.6.32-rc5.orig/mm/memcontrol.c
> > +++ linux-2.6.32-rc5/mm/memcontrol.c
> > @@ -1990,7 +1990,8 @@ int mem_cgroup_prepare_migration(struct
> > struct page_cgroup *pc;
> > struct mem_cgroup *mem = NULL;
> > int ret = 0;
> > -
> > + /* this pointer will be checked at end_migration */
> > + *ptr = NULL;
> > if (mem_cgroup_disabled())
> > return 0;
> >
> >
> I thought unmap_and_move() itself initializes "mem" to NULL, but it doesn't...
> I personaly prefer initializing "mem" to NULL in unmap_and_move(), but anyway
> I think this patch is also correct.
>
> Reviewed-by: Daisuke Nishimura <[email protected]>
>
Ok, here
> And I think we should send a fix for this bug to -stable too.
I think so, too.


==
Lee Schermerhorn reported that he saw bad pointer dereference
in mem_cgroup_end_migration() when he disabled memcg by boot option.

memcg's page migration logic works as

mem_cgroup_prepare_migration(page, &ptr);
do page migration
mem_cgroup_end_migration(page, ptr);

Now, ptr is not initialized when memcg is disabled by boot option.
This patch fixes it.

Changelog: 2009/10/29
- modified "fix" from memcontrol.c to migrate.c


Reported-by: Lee Schermerhorn <[email protected]>
Cc: Balbir Singh <[email protected]>
Cc: Daisuke Nishimura <[email protected]>
Reviewed-by: Daisuke Nishimura <[email protected]>
Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
---
mm/migrate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.32-rc5/mm/migrate.c
===================================================================
--- linux-2.6.32-rc5.orig/mm/migrate.c
+++ linux-2.6.32-rc5/mm/migrate.c
@@ -602,7 +602,7 @@ static int unmap_and_move(new_page_t get
struct page *newpage = get_new_page(page, private, &result);
int rcu_locked = 0;
int charge = 0;
- struct mem_cgroup *mem;
+ struct mem_cgroup *mem = NULL;

if (!newpage)
return -ENOMEM;