From: Phil Carmody <[email protected]>
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 <[email protected]>
---
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
On Wed, 2009-12-09 at 19:45 +0200, Phil Carmody wrote:
> From: Phil Carmody <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Phil Carmody <[email protected]>
> ---
> 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]);
> }
Phil Carmody wrote:
> From: Phil Carmody <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
On Thu, 2009-12-10 at 10:39 +0800, Helight.Xu wrote:
> Phil Carmody wrote:
> > From: Phil Carmody <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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.
* Phil Carmody <[email protected]> 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
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 <[email protected]>
---
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
Commit-ID: dfc12eb26a285df316be68a808af86964f3bff86
Gitweb: http://git.kernel.org/tip/dfc12eb26a285df316be68a808af86964f3bff86
Author: Phil Carmody <[email protected]>
AuthorDate: Thu, 10 Dec 2009 14:29:37 +0200
Committer: Ingo Molnar <[email protected]>
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 <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
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;
}