All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kernel/resource.c: fix memory leak
@ 2010-01-09 14:46 Alexander Beregalov
  2010-01-09 16:57 ` Américo Wang
  2010-01-09 17:34 ` Linus Torvalds
  0 siblings, 2 replies; 3+ messages in thread
From: Alexander Beregalov @ 2010-01-09 14:46 UTC (permalink / raw)
  To: torvalds, linux-kernel; +Cc: Alexander Beregalov

Free res before exit.
Found by cppcheck.

Signed-off-by: Alexander Beregalov <a.beregalov@gmail.com>
---
 kernel/resource.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index af96c1e..ae2e177 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -545,11 +545,12 @@ static void __init __reserve_region_with_split(struct resource *root,
 	res->flags = IORESOURCE_BUSY;
 
 	conflict = __request_resource(parent, res);
+	kfree(res);
+
 	if (!conflict)
 		return;
 
 	/* failed, split and try again */
-	kfree(res);
 
 	/* conflict covered whole area */
 	if (conflict->start <= start && conflict->end >= end)
-- 
1.6.6


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

* Re: [PATCH] kernel/resource.c: fix memory leak
  2010-01-09 14:46 [PATCH] kernel/resource.c: fix memory leak Alexander Beregalov
@ 2010-01-09 16:57 ` Américo Wang
  2010-01-09 17:34 ` Linus Torvalds
  1 sibling, 0 replies; 3+ messages in thread
From: Américo Wang @ 2010-01-09 16:57 UTC (permalink / raw)
  To: Alexander Beregalov; +Cc: torvalds, linux-kernel

On Sat, Jan 09, 2010 at 05:46:35PM +0300, Alexander Beregalov wrote:
>Free res before exit.
>Found by cppcheck.
>
>Signed-off-by: Alexander Beregalov <a.beregalov@gmail.com>
>---
> kernel/resource.c |    3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
>diff --git a/kernel/resource.c b/kernel/resource.c
>index af96c1e..ae2e177 100644
>--- a/kernel/resource.c
>+++ b/kernel/resource.c
>@@ -545,11 +545,12 @@ static void __init __reserve_region_with_split(struct resource *root,
> 	res->flags = IORESOURCE_BUSY;
> 
> 	conflict = __request_resource(parent, res);
>+	kfree(res);
>+
> 	if (!conflict)
> 		return;
> 
> 	/* failed, split and try again */
>-	kfree(res);
> 
> 	/* conflict covered whole area */
> 	if (conflict->start <= start && conflict->end >= end)

NAK.

If I read it correctly, 'res' will be used if !conflict, we
should't kfree it.


-- 
Live like a child, think like the god.
 

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

* Re: [PATCH] kernel/resource.c: fix memory leak
  2010-01-09 14:46 [PATCH] kernel/resource.c: fix memory leak Alexander Beregalov
  2010-01-09 16:57 ` Américo Wang
@ 2010-01-09 17:34 ` Linus Torvalds
  1 sibling, 0 replies; 3+ messages in thread
From: Linus Torvalds @ 2010-01-09 17:34 UTC (permalink / raw)
  To: Alexander Beregalov; +Cc: linux-kernel



On Sat, 9 Jan 2010, Alexander Beregalov wrote:
>
> Free res before exit.
> Found by cppcheck.
> 
> Signed-off-by: Alexander Beregalov <a.beregalov@gmail.com>
> ---
>  kernel/resource.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/resource.c b/kernel/resource.c
> index af96c1e..ae2e177 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -545,11 +545,12 @@ static void __init __reserve_region_with_split(struct resource *root,
>  	res->flags = IORESOURCE_BUSY;
>  
>  	conflict = __request_resource(parent, res);
> +	kfree(res);
> +
>  	if (!conflict)
>  		return;
>  
>  	/* failed, split and try again */
> -	kfree(res);

As Amerigo pointed out, this is very wrong indeed.

If we don't have a conflict, then __request_resource() will have inserted 
the resource into the resource tree, and we absolutely must _not_ free it.

So that cppcheck tool is way too simplistic, and wrong.

			Linus

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

end of thread, other threads:[~2010-01-09 17:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-09 14:46 [PATCH] kernel/resource.c: fix memory leak Alexander Beregalov
2010-01-09 16:57 ` Américo Wang
2010-01-09 17:34 ` Linus Torvalds

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.