mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] x86/pat: fix memory leak in free_memtype
@ 2010-05-25  9:52 Xiaotian Feng
  2010-05-26  0:12 ` Suresh Siddha
  0 siblings, 1 reply; 6+ messages in thread
From: Xiaotian Feng @ 2010-05-25  9:52 UTC (permalink / raw)
  To: x86
  Cc: linux-kernel, Xiaotian Feng, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Venkatesh Pallipadi, Jack Steiner, Suresh Siddha

reserve_memtype will allocate memory for new memtype, but
in free_memtype, after the memtype erased from rbtree, the
memory is not freed.

Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Cc: Jack Steiner <steiner@sgi.com>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
---
 arch/x86/mm/pat_rbtree.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/mm/pat_rbtree.c b/arch/x86/mm/pat_rbtree.c
index 07de4cb..fb8c1e5 100644
--- a/arch/x86/mm/pat_rbtree.c
+++ b/arch/x86/mm/pat_rbtree.c
@@ -15,6 +15,7 @@
 #include <linux/rbtree.h>
 #include <linux/sched.h>
 #include <linux/gfp.h>
+#include <linux/slab.h>
 
 #include <asm/pgtable.h>
 #include <asm/pat.h>
@@ -240,6 +241,7 @@ int rbt_memtype_erase(u64 start, u64 end)
 		return -EINVAL;
 
 	rb_erase(&data->rb, &memtype_rbroot);
+	kfree(data);
 	return 0;
 }
 
-- 
1.7.0.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/pat: fix memory leak in free_memtype
  2010-05-25  9:52 [PATCH] x86/pat: fix memory leak in free_memtype Xiaotian Feng
@ 2010-05-26  0:12 ` Suresh Siddha
  2010-05-26  0:27   ` Suresh Siddha
  2010-05-26  1:51   ` [PATCH V2] " Xiaotian Feng
  0 siblings, 2 replies; 6+ messages in thread
From: Suresh Siddha @ 2010-05-26  0:12 UTC (permalink / raw)
  To: Xiaotian Feng
  Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Jack Steiner, Venkatesh Pallipadi

On Tue, 2010-05-25 at 02:52 -0700, Xiaotian Feng wrote:
> reserve_memtype will allocate memory for new memtype, but
> in free_memtype, after the memtype erased from rbtree, the
> memory is not freed.
> 
> Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> Cc: Jack Steiner <steiner@sgi.com>
> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> ---
>  arch/x86/mm/pat_rbtree.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/mm/pat_rbtree.c b/arch/x86/mm/pat_rbtree.c
> index 07de4cb..fb8c1e5 100644
> --- a/arch/x86/mm/pat_rbtree.c
> +++ b/arch/x86/mm/pat_rbtree.c
> @@ -15,6 +15,7 @@
>  #include <linux/rbtree.h>
>  #include <linux/sched.h>
>  #include <linux/gfp.h>
> +#include <linux/slab.h>
>  
>  #include <asm/pgtable.h>
>  #include <asm/pat.h>
> @@ -240,6 +241,7 @@ int rbt_memtype_erase(u64 start, u64 end)
>  		return -EINVAL;
>  
>  	rb_erase(&data->rb, &memtype_rbroot);
> +	kfree(data);
>  	return 0;
>  }

Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>

Peter, Please apply this for x86/urgent, this is introduced in the post
2.6.34 changes.

thanks,
suresh


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/pat: fix memory leak in free_memtype
  2010-05-26  0:12 ` Suresh Siddha
@ 2010-05-26  0:27   ` Suresh Siddha
  2010-05-26  1:51   ` [PATCH V2] " Xiaotian Feng
  1 sibling, 0 replies; 6+ messages in thread
From: Suresh Siddha @ 2010-05-26  0:27 UTC (permalink / raw)
  To: Xiaotian Feng
  Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Jack Steiner, Venkatesh Pallipadi

On Tue, 2010-05-25 at 17:12 -0700, Suresh Siddha wrote:
> On Tue, 2010-05-25 at 02:52 -0700, Xiaotian Feng wrote:
> > reserve_memtype will allocate memory for new memtype, but
> > in free_memtype, after the memtype erased from rbtree, the
> > memory is not freed.
> > 
> > Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> > Cc: Jack Steiner <steiner@sgi.com>
> > Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> > ---
> >  arch/x86/mm/pat_rbtree.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/x86/mm/pat_rbtree.c b/arch/x86/mm/pat_rbtree.c
> > index 07de4cb..fb8c1e5 100644
> > --- a/arch/x86/mm/pat_rbtree.c
> > +++ b/arch/x86/mm/pat_rbtree.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/rbtree.h>
> >  #include <linux/sched.h>
> >  #include <linux/gfp.h>
> > +#include <linux/slab.h>
> >  
> >  #include <asm/pgtable.h>
> >  #include <asm/pat.h>
> > @@ -240,6 +241,7 @@ int rbt_memtype_erase(u64 start, u64 end)
> >  		return -EINVAL;
> >  
> >  	rb_erase(&data->rb, &memtype_rbroot);
> > +	kfree(data);
> >  	return 0;
> >  }
> 
> Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
> 
> Peter, Please apply this for x86/urgent, this is introduced in the post
> 2.6.34 changes.

Xiaotian, Also I think more cleaner fix is to do this in free_memtype().
Can you please resend the patch with this change?

thanks,
suresh


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH V2] x86/pat: fix memory leak in free_memtype
  2010-05-26  0:12 ` Suresh Siddha
  2010-05-26  0:27   ` Suresh Siddha
@ 2010-05-26  1:51   ` Xiaotian Feng
  2010-05-26 16:47     ` Suresh Siddha
  2010-05-26 20:22     ` [tip:x86/urgent] x86, pat: Fix " tip-bot for Xiaotian Feng
  1 sibling, 2 replies; 6+ messages in thread
From: Xiaotian Feng @ 2010-05-26  1:51 UTC (permalink / raw)
  To: x86
  Cc: linux-kernel, Xiaotian Feng, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Venkatesh Pallipadi, Jack Steiner, Suresh Siddha

reserve_memtype will allocate memory for new memtype, but
in free_memtype, after the memtype erased from rbtree, the
memory is not freed.

Changes since V1:
	make rbt_memtype_erase return erased memtype so that 
	it can be freed in free_memtype.

Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Cc: Jack Steiner <steiner@sgi.com>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
---
 arch/x86/mm/pat.c          |   10 +++++++---
 arch/x86/mm/pat_internal.h |    6 +++---
 arch/x86/mm/pat_rbtree.c   |    7 ++++---
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index bbe5502..bf2b5fa 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -336,6 +336,7 @@ int free_memtype(u64 start, u64 end)
 {
 	int err = -EINVAL;
 	int is_range_ram;
+	struct memtype *entry;
 
 	if (!pat_enabled)
 		return 0;
@@ -355,17 +356,20 @@ int free_memtype(u64 start, u64 end)
 	}
 
 	spin_lock(&memtype_lock);
-	err = rbt_memtype_erase(start, end);
+	entry = rbt_memtype_erase(start, end);
 	spin_unlock(&memtype_lock);
 
-	if (err) {
+	if (!entry) {
 		printk(KERN_INFO "%s:%d freeing invalid memtype %Lx-%Lx\n",
 			current->comm, current->pid, start, end);
+		return -EINVAL;
 	}
+	
+	kfree(entry);
 
 	dprintk("free_memtype request 0x%Lx-0x%Lx\n", start, end);
 
-	return err;
+	return 0;
 }
 
 
diff --git a/arch/x86/mm/pat_internal.h b/arch/x86/mm/pat_internal.h
index 4f39eef..77e5ba1 100644
--- a/arch/x86/mm/pat_internal.h
+++ b/arch/x86/mm/pat_internal.h
@@ -28,15 +28,15 @@ static inline char *cattr_name(unsigned long flags)
 #ifdef CONFIG_X86_PAT
 extern int rbt_memtype_check_insert(struct memtype *new,
 					unsigned long *new_type);
-extern int rbt_memtype_erase(u64 start, u64 end);
+extern struct memtype *rbt_memtype_erase(u64 start, u64 end);
 extern struct memtype *rbt_memtype_lookup(u64 addr);
 extern int rbt_memtype_copy_nth_element(struct memtype *out, loff_t pos);
 #else
 static inline int rbt_memtype_check_insert(struct memtype *new,
 					unsigned long *new_type)
 { return 0; }
-static inline int rbt_memtype_erase(u64 start, u64 end)
-{ return 0; }
+static inline struct memtype *rbt_memtype_erase(u64 start, u64 end)
+{ return NULL; }
 static inline struct memtype *rbt_memtype_lookup(u64 addr)
 { return NULL; }
 static inline int rbt_memtype_copy_nth_element(struct memtype *out, loff_t pos)
diff --git a/arch/x86/mm/pat_rbtree.c b/arch/x86/mm/pat_rbtree.c
index 07de4cb..f537087 100644
--- a/arch/x86/mm/pat_rbtree.c
+++ b/arch/x86/mm/pat_rbtree.c
@@ -231,16 +231,17 @@ int rbt_memtype_check_insert(struct memtype *new, unsigned long *ret_type)
 	return err;
 }
 
-int rbt_memtype_erase(u64 start, u64 end)
+struct memtype *rbt_memtype_erase(u64 start, u64 end)
 {
 	struct memtype *data;
 
 	data = memtype_rb_exact_match(&memtype_rbroot, start, end);
 	if (!data)
-		return -EINVAL;
+		goto out;
 
 	rb_erase(&data->rb, &memtype_rbroot);
-	return 0;
+out:
+	return data;
 }
 
 struct memtype *rbt_memtype_lookup(u64 addr)
-- 
1.6.6.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] x86/pat: fix memory leak in free_memtype
  2010-05-26  1:51   ` [PATCH V2] " Xiaotian Feng
@ 2010-05-26 16:47     ` Suresh Siddha
  2010-05-26 20:22     ` [tip:x86/urgent] x86, pat: Fix " tip-bot for Xiaotian Feng
  1 sibling, 0 replies; 6+ messages in thread
From: Suresh Siddha @ 2010-05-26 16:47 UTC (permalink / raw)
  To: Xiaotian Feng
  Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Jack Steiner, Venkatesh Pallipadi

On Tue, 2010-05-25 at 18:51 -0700, Xiaotian Feng wrote:
> reserve_memtype will allocate memory for new memtype, but
> in free_memtype, after the memtype erased from rbtree, the
> memory is not freed.
> 
> Changes since V1:
> 	make rbt_memtype_erase return erased memtype so that 
> 	it can be freed in free_memtype.
> 
> Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> Cc: Jack Steiner <steiner@sgi.com>
> Cc: Suresh Siddha <suresh.b.siddha@intel.com>

It looks bigger than I expected. But it is a bit cleaner (as both
allocation/free happens in pat.c API)

Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>

> ---
>  arch/x86/mm/pat.c          |   10 +++++++---
>  arch/x86/mm/pat_internal.h |    6 +++---
>  arch/x86/mm/pat_rbtree.c   |    7 ++++---
>  3 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
> index bbe5502..bf2b5fa 100644
> --- a/arch/x86/mm/pat.c
> +++ b/arch/x86/mm/pat.c
> @@ -336,6 +336,7 @@ int free_memtype(u64 start, u64 end)
>  {
>  	int err = -EINVAL;
>  	int is_range_ram;
> +	struct memtype *entry;
>  
>  	if (!pat_enabled)
>  		return 0;
> @@ -355,17 +356,20 @@ int free_memtype(u64 start, u64 end)
>  	}
>  
>  	spin_lock(&memtype_lock);
> -	err = rbt_memtype_erase(start, end);
> +	entry = rbt_memtype_erase(start, end);
>  	spin_unlock(&memtype_lock);
>  
> -	if (err) {
> +	if (!entry) {
>  		printk(KERN_INFO "%s:%d freeing invalid memtype %Lx-%Lx\n",
>  			current->comm, current->pid, start, end);
> +		return -EINVAL;
>  	}
> +	
> +	kfree(entry);
>  
>  	dprintk("free_memtype request 0x%Lx-0x%Lx\n", start, end);
>  
> -	return err;
> +	return 0;
>  }
>  
> 
> diff --git a/arch/x86/mm/pat_internal.h b/arch/x86/mm/pat_internal.h
> index 4f39eef..77e5ba1 100644
> --- a/arch/x86/mm/pat_internal.h
> +++ b/arch/x86/mm/pat_internal.h
> @@ -28,15 +28,15 @@ static inline char *cattr_name(unsigned long flags)
>  #ifdef CONFIG_X86_PAT
>  extern int rbt_memtype_check_insert(struct memtype *new,
>  					unsigned long *new_type);
> -extern int rbt_memtype_erase(u64 start, u64 end);
> +extern struct memtype *rbt_memtype_erase(u64 start, u64 end);
>  extern struct memtype *rbt_memtype_lookup(u64 addr);
>  extern int rbt_memtype_copy_nth_element(struct memtype *out, loff_t pos);
>  #else
>  static inline int rbt_memtype_check_insert(struct memtype *new,
>  					unsigned long *new_type)
>  { return 0; }
> -static inline int rbt_memtype_erase(u64 start, u64 end)
> -{ return 0; }
> +static inline struct memtype *rbt_memtype_erase(u64 start, u64 end)
> +{ return NULL; }
>  static inline struct memtype *rbt_memtype_lookup(u64 addr)
>  { return NULL; }
>  static inline int rbt_memtype_copy_nth_element(struct memtype *out, loff_t pos)
> diff --git a/arch/x86/mm/pat_rbtree.c b/arch/x86/mm/pat_rbtree.c
> index 07de4cb..f537087 100644
> --- a/arch/x86/mm/pat_rbtree.c
> +++ b/arch/x86/mm/pat_rbtree.c
> @@ -231,16 +231,17 @@ int rbt_memtype_check_insert(struct memtype *new, unsigned long *ret_type)
>  	return err;
>  }
>  
> -int rbt_memtype_erase(u64 start, u64 end)
> +struct memtype *rbt_memtype_erase(u64 start, u64 end)
>  {
>  	struct memtype *data;
>  
>  	data = memtype_rb_exact_match(&memtype_rbroot, start, end);
>  	if (!data)
> -		return -EINVAL;
> +		goto out;
>  
>  	rb_erase(&data->rb, &memtype_rbroot);
> -	return 0;
> +out:
> +	return data;
>  }
>  
>  struct memtype *rbt_memtype_lookup(u64 addr)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:x86/urgent] x86, pat: Fix memory leak in free_memtype
  2010-05-26  1:51   ` [PATCH V2] " Xiaotian Feng
  2010-05-26 16:47     ` Suresh Siddha
@ 2010-05-26 20:22     ` tip-bot for Xiaotian Feng
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Xiaotian Feng @ 2010-05-26 20:22 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, venkatesh.pallipadi, steiner,
	suresh.b.siddha, tglx, dfeng, hpa

Commit-ID:  20413f27163fb1b8b806c0c219dc95eae67c633a
Gitweb:     http://git.kernel.org/tip/20413f27163fb1b8b806c0c219dc95eae67c633a
Author:     Xiaotian Feng <dfeng@redhat.com>
AuthorDate: Wed, 26 May 2010 09:51:10 +0800
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Wed, 26 May 2010 11:26:04 -0700

x86, pat: Fix memory leak in free_memtype

Reserve_memtype will allocate memory for new memtype, but
in free_memtype, after the memtype erased from rbtree, the
memory is not freed.

Changes since V1:
	make rbt_memtype_erase return erased memtype so that
	it can be freed in free_memtype.

[ hpa: not for -stable: 2.6.34 and earlier not affected ]

Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
LKML-Reference: <1274838670-8731-1-git-send-email-dfeng@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Cc: Jack Steiner <steiner@sgi.com>
Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/mm/pat.c          |   10 +++++++---
 arch/x86/mm/pat_internal.h |    6 +++---
 arch/x86/mm/pat_rbtree.c   |    7 ++++---
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index bbe5502..acc15b2 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -336,6 +336,7 @@ int free_memtype(u64 start, u64 end)
 {
 	int err = -EINVAL;
 	int is_range_ram;
+	struct memtype *entry;
 
 	if (!pat_enabled)
 		return 0;
@@ -355,17 +356,20 @@ int free_memtype(u64 start, u64 end)
 	}
 
 	spin_lock(&memtype_lock);
-	err = rbt_memtype_erase(start, end);
+	entry = rbt_memtype_erase(start, end);
 	spin_unlock(&memtype_lock);
 
-	if (err) {
+	if (!entry) {
 		printk(KERN_INFO "%s:%d freeing invalid memtype %Lx-%Lx\n",
 			current->comm, current->pid, start, end);
+		return -EINVAL;
 	}
 
+	kfree(entry);
+
 	dprintk("free_memtype request 0x%Lx-0x%Lx\n", start, end);
 
-	return err;
+	return 0;
 }
 
 
diff --git a/arch/x86/mm/pat_internal.h b/arch/x86/mm/pat_internal.h
index 4f39eef..77e5ba1 100644
--- a/arch/x86/mm/pat_internal.h
+++ b/arch/x86/mm/pat_internal.h
@@ -28,15 +28,15 @@ static inline char *cattr_name(unsigned long flags)
 #ifdef CONFIG_X86_PAT
 extern int rbt_memtype_check_insert(struct memtype *new,
 					unsigned long *new_type);
-extern int rbt_memtype_erase(u64 start, u64 end);
+extern struct memtype *rbt_memtype_erase(u64 start, u64 end);
 extern struct memtype *rbt_memtype_lookup(u64 addr);
 extern int rbt_memtype_copy_nth_element(struct memtype *out, loff_t pos);
 #else
 static inline int rbt_memtype_check_insert(struct memtype *new,
 					unsigned long *new_type)
 { return 0; }
-static inline int rbt_memtype_erase(u64 start, u64 end)
-{ return 0; }
+static inline struct memtype *rbt_memtype_erase(u64 start, u64 end)
+{ return NULL; }
 static inline struct memtype *rbt_memtype_lookup(u64 addr)
 { return NULL; }
 static inline int rbt_memtype_copy_nth_element(struct memtype *out, loff_t pos)
diff --git a/arch/x86/mm/pat_rbtree.c b/arch/x86/mm/pat_rbtree.c
index 07de4cb..f537087 100644
--- a/arch/x86/mm/pat_rbtree.c
+++ b/arch/x86/mm/pat_rbtree.c
@@ -231,16 +231,17 @@ int rbt_memtype_check_insert(struct memtype *new, unsigned long *ret_type)
 	return err;
 }
 
-int rbt_memtype_erase(u64 start, u64 end)
+struct memtype *rbt_memtype_erase(u64 start, u64 end)
 {
 	struct memtype *data;
 
 	data = memtype_rb_exact_match(&memtype_rbroot, start, end);
 	if (!data)
-		return -EINVAL;
+		goto out;
 
 	rb_erase(&data->rb, &memtype_rbroot);
-	return 0;
+out:
+	return data;
 }
 
 struct memtype *rbt_memtype_lookup(u64 addr)

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-05-26 20:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-25  9:52 [PATCH] x86/pat: fix memory leak in free_memtype Xiaotian Feng
2010-05-26  0:12 ` Suresh Siddha
2010-05-26  0:27   ` Suresh Siddha
2010-05-26  1:51   ` [PATCH V2] " Xiaotian Feng
2010-05-26 16:47     ` Suresh Siddha
2010-05-26 20:22     ` [tip:x86/urgent] x86, pat: Fix " tip-bot for Xiaotian Feng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®