linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sched: Memory leak in two error corner cases
@ 2009-12-09 17:45 Phil Carmody
  2009-12-09 17:54 ` Peter Zijlstra
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Phil Carmody @ 2009-12-09 17:45 UTC (permalink / raw)
  To: mingo; +Cc: peterz, linux-kernel

From: Phil Carmody <ext-phil.2.carmody@nokia.com>

If the second in each of these pairs of allocations fails, then
the first one will not be freed in the error route out.

Found by a static code analysis tool.

Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
---
 kernel/sched.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index e7f2cfa..29ebc4a 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -9841,8 +9841,10 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
 
 		se = kzalloc_node(sizeof(struct sched_entity),
 				  GFP_KERNEL, cpu_to_node(i));
-		if (!se)
+		if (!se) {
+			kfree(cfs_rq);
 			goto err;
+		}
 
 		init_tg_cfs_entry(tg, cfs_rq, se, i, 0, parent->se[i]);
 	}
@@ -9929,8 +9931,10 @@ int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent)
 
 		rt_se = kzalloc_node(sizeof(struct sched_rt_entity),
 				     GFP_KERNEL, cpu_to_node(i));
-		if (!rt_se)
+		if (!rt_se) {
+			kfree(rt_rq);
 			goto err;
+		}
 
 		init_tg_rt_entry(tg, rt_rq, rt_se, i, 0, parent->rt_se[i]);
 	}
-- 
1.6.0.4


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

* Re: [PATCH] sched: Memory leak in two error corner cases
  2009-12-09 17:45 [PATCH] sched: Memory leak in two error corner cases Phil Carmody
@ 2009-12-09 17:54 ` Peter Zijlstra
  2009-12-10  2:39 ` Helight.Xu
  2009-12-10  8:02 ` Ingo Molnar
  2 siblings, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2009-12-09 17:54 UTC (permalink / raw)
  To: Phil Carmody; +Cc: mingo, linux-kernel

On Wed, 2009-12-09 at 19:45 +0200, Phil Carmody wrote:
> From: Phil Carmody <ext-phil.2.carmody@nokia.com>
> 
> If the second in each of these pairs of allocations fails, then
> the first one will not be freed in the error route out.
> 
> Found by a static code analysis tool.

nice find.

Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

> Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
> ---
>  kernel/sched.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched.c b/kernel/sched.c
> index e7f2cfa..29ebc4a 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -9841,8 +9841,10 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
>  
>  		se = kzalloc_node(sizeof(struct sched_entity),
>  				  GFP_KERNEL, cpu_to_node(i));
> -		if (!se)
> +		if (!se) {
> +			kfree(cfs_rq);
>  			goto err;
> +		}
>  
>  		init_tg_cfs_entry(tg, cfs_rq, se, i, 0, parent->se[i]);
>  	}
> @@ -9929,8 +9931,10 @@ int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent)
>  
>  		rt_se = kzalloc_node(sizeof(struct sched_rt_entity),
>  				     GFP_KERNEL, cpu_to_node(i));
> -		if (!rt_se)
> +		if (!rt_se) {
> +			kfree(rt_rq);
>  			goto err;
> +		}
>  
>  		init_tg_rt_entry(tg, rt_rq, rt_se, i, 0, parent->rt_se[i]);
>  	}



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

* Re: [PATCH] sched: Memory leak in two error corner cases
  2009-12-09 17:45 [PATCH] sched: Memory leak in two error corner cases Phil Carmody
  2009-12-09 17:54 ` Peter Zijlstra
@ 2009-12-10  2:39 ` Helight.Xu
  2009-12-10  5:12   ` Peter Zijlstra
  2009-12-10  8:02 ` Ingo Molnar
  2 siblings, 1 reply; 7+ messages in thread
From: Helight.Xu @ 2009-12-10  2:39 UTC (permalink / raw)
  To: Phil Carmody; +Cc: mingo, peterz, linux-kernel

Phil Carmody wrote:
> From: Phil Carmody <ext-phil.2.carmody@nokia.com>
>
> If the second in each of these pairs of allocations fails, then
> the first one will not be freed in the error route out.
>
> Found by a static code analysis tool.
>
> Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
> ---
>  kernel/sched.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched.c b/kernel/sched.c
> index e7f2cfa..29ebc4a 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -9841,8 +9841,10 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
>  
>  		se = kzalloc_node(sizeof(struct sched_entity),
>  				  GFP_KERNEL, cpu_to_node(i));
> -		if (!se)
> +		if (!se) {
> +			kfree(cfs_rq);
>  			goto err;
> +		}
>   
if here has menory leak, why not  here!

    tg->cfs_rq = kzalloc(sizeof(cfs_rq) * nr_cpu_ids, GFP_KERNEL);
    if (!tg->cfs_rq)
         goto err;
    tg->se = kzalloc(sizeof(se) * nr_cpu_ids, GFP_KERNEL);
    if (!tg->se)
         goto err;
should I fix here?
>  
>  		init_tg_cfs_entry(tg, cfs_rq, se, i, 0, parent->se[i]);
>  	}
> @@ -9929,8 +9931,10 @@ int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent)
>  
>  		rt_se = kzalloc_node(sizeof(struct sched_rt_entity),
>  				     GFP_KERNEL, cpu_to_node(i));
> -		if (!rt_se)
> +		if (!rt_se) {
> +			kfree(rt_rq);
>  			goto err;
> +		}
>  
>  		init_tg_rt_entry(tg, rt_rq, rt_se, i, 0, parent->rt_se[i]);
>  	}
>   


-- 
---------------------------------
Zhenwen Xu - Open and Free
Home Page:	http://zhwen.org


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

* Re: [PATCH] sched: Memory leak in two error corner cases
  2009-12-10  2:39 ` Helight.Xu
@ 2009-12-10  5:12   ` Peter Zijlstra
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2009-12-10  5:12 UTC (permalink / raw)
  To: Helight.Xu; +Cc: Phil Carmody, mingo, linux-kernel

On Thu, 2009-12-10 at 10:39 +0800, Helight.Xu wrote:
> Phil Carmody wrote:
> > From: Phil Carmody <ext-phil.2.carmody@nokia.com>
> >
> > If the second in each of these pairs of allocations fails, then
> > the first one will not be freed in the error route out.
> >
> > Found by a static code analysis tool.
> >
> > Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
> > ---
> >  kernel/sched.c |    8 ++++++--
> >  1 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/sched.c b/kernel/sched.c
> > index e7f2cfa..29ebc4a 100644
> > --- a/kernel/sched.c
> > +++ b/kernel/sched.c
> > @@ -9841,8 +9841,10 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
> >  
> >  		se = kzalloc_node(sizeof(struct sched_entity),
> >  				  GFP_KERNEL, cpu_to_node(i));
> > -		if (!se)
> > +		if (!se) {
> > +			kfree(cfs_rq);
> >  			goto err;
> > +		}
> >   
> if here has menory leak, why not  here!
> 
>     tg->cfs_rq = kzalloc(sizeof(cfs_rq) * nr_cpu_ids, GFP_KERNEL);
>     if (!tg->cfs_rq)
>          goto err;
>     tg->se = kzalloc(sizeof(se) * nr_cpu_ids, GFP_KERNEL);
>     if (!tg->se)
>          goto err;
> should I fix here?

The error path deals with that.

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

* Re: [PATCH] sched: Memory leak in two error corner cases
  2009-12-09 17:45 [PATCH] sched: Memory leak in two error corner cases Phil Carmody
  2009-12-09 17:54 ` Peter Zijlstra
  2009-12-10  2:39 ` Helight.Xu
@ 2009-12-10  8:02 ` Ingo Molnar
  2009-12-10 12:29   ` [PATCH] [v2] " Phil Carmody
  2 siblings, 1 reply; 7+ messages in thread
From: Ingo Molnar @ 2009-12-10  8:02 UTC (permalink / raw)
  To: Phil Carmody; +Cc: peterz, linux-kernel


* Phil Carmody <ext-phil.2.carmody@nokia.com> wrote:

> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -9841,8 +9841,10 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
>  
>  		se = kzalloc_node(sizeof(struct sched_entity),
>  				  GFP_KERNEL, cpu_to_node(i));
> -		if (!se)
> +		if (!se) {
> +			kfree(cfs_rq);
>  			goto err;
> +		}

would be nice to turn this into a regular pattern of:

		if (!se)
			goto err_kfree;

where err_kfree does the kfree.

Thanks,

	Ingo

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

* [PATCH] [v2] sched: Memory leak in two error corner cases
  2009-12-10  8:02 ` Ingo Molnar
@ 2009-12-10 12:29   ` Phil Carmody
  2009-12-10 13:37     ` [tip:sched/urgent] sched: Fix memory " tip-bot for Phil Carmody
  0 siblings, 1 reply; 7+ messages in thread
From: Phil Carmody @ 2009-12-10 12:29 UTC (permalink / raw)
  To: mingo; +Cc: peterz, linux-kernel

If the second in each of these pairs of allocations fails, then
the first one will not be freed in the error route out.

Found by a static code analysis tool.

Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
---
 kernel/sched.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 3c11ae0..ee8046f 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -9765,13 +9765,15 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
 		se = kzalloc_node(sizeof(struct sched_entity),
 				  GFP_KERNEL, cpu_to_node(i));
 		if (!se)
-			goto err;
+			goto err_free_rq;
 
 		init_tg_cfs_entry(tg, cfs_rq, se, i, 0, parent->se[i]);
 	}
 
 	return 1;
 
+ err_free_rq:
+	kfree(cfs_rq);
  err:
 	return 0;
 }
@@ -9853,13 +9855,15 @@ int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent)
 		rt_se = kzalloc_node(sizeof(struct sched_rt_entity),
 				     GFP_KERNEL, cpu_to_node(i));
 		if (!rt_se)
-			goto err;
+			goto err_free_rq;
 
 		init_tg_rt_entry(tg, rt_rq, rt_se, i, 0, parent->rt_se[i]);
 	}
 
 	return 1;
 
+ err_free_rq:
+	kfree(rt_rq);
  err:
 	return 0;
 }
-- 
1.6.0.4


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

* [tip:sched/urgent] sched: Fix memory leak in two error corner cases
  2009-12-10 12:29   ` [PATCH] [v2] " Phil Carmody
@ 2009-12-10 13:37     ` tip-bot for Phil Carmody
  0 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Phil Carmody @ 2009-12-10 13:37 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, ext-phil.2.carmody, tglx, mingo

Commit-ID:  dfc12eb26a285df316be68a808af86964f3bff86
Gitweb:     http://git.kernel.org/tip/dfc12eb26a285df316be68a808af86964f3bff86
Author:     Phil Carmody <ext-phil.2.carmody@nokia.com>
AuthorDate: Thu, 10 Dec 2009 14:29:37 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Thu, 10 Dec 2009 14:28:10 +0100

sched: Fix memory leak in two error corner cases

If the second in each of these pairs of allocations fails, then the
first one will not be freed in the error route out.

Found by a static code analysis tool.

Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1260448177-28448-1-git-send-email-ext-phil.2.carmody@nokia.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 3de3dea..36cc05a 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -9855,13 +9855,15 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
 		se = kzalloc_node(sizeof(struct sched_entity),
 				  GFP_KERNEL, cpu_to_node(i));
 		if (!se)
-			goto err;
+			goto err_free_rq;
 
 		init_tg_cfs_entry(tg, cfs_rq, se, i, 0, parent->se[i]);
 	}
 
 	return 1;
 
+ err_free_rq:
+	kfree(cfs_rq);
  err:
 	return 0;
 }
@@ -9943,13 +9945,15 @@ int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent)
 		rt_se = kzalloc_node(sizeof(struct sched_rt_entity),
 				     GFP_KERNEL, cpu_to_node(i));
 		if (!rt_se)
-			goto err;
+			goto err_free_rq;
 
 		init_tg_rt_entry(tg, rt_rq, rt_se, i, 0, parent->rt_se[i]);
 	}
 
 	return 1;
 
+ err_free_rq:
+	kfree(rt_rq);
  err:
 	return 0;
 }

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

end of thread, other threads:[~2009-12-10 13:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-09 17:45 [PATCH] sched: Memory leak in two error corner cases Phil Carmody
2009-12-09 17:54 ` Peter Zijlstra
2009-12-10  2:39 ` Helight.Xu
2009-12-10  5:12   ` Peter Zijlstra
2009-12-10  8:02 ` Ingo Molnar
2009-12-10 12:29   ` [PATCH] [v2] " Phil Carmody
2009-12-10 13:37     ` [tip:sched/urgent] sched: Fix memory " tip-bot for Phil Carmody

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).