lkml.org 
[lkml]   [2018]   [Apr]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 1/1] z3fold: fix memory leak
On Wed, 4 Apr 2018 15:20:39 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 4 Apr 2018 08:51:51 +0800 Xidong Wang <wangxidong_97@163.com> wrote:
>
> > In function z3fold_create_pool(), the memory allocated by
> > __alloc_percpu() is not released on the error path that pool->compact_wq
> > , which holds the return value of create_singlethread_workqueue(), is NULL.
> > This will result in a memory leak bug.
> >
> > ...
> >
> > --- a/mm/z3fold.c
> > +++ b/mm/z3fold.c
> > @@ -490,6 +490,7 @@ static struct z3fold_pool *z3fold_create_pool(const char *name, gfp_t gfp,
> > out_wq:
> > destroy_workqueue(pool->compact_wq);
> > out:
> > + free_percpu(pool->unbuddied);
> > kfree(pool);
> > return NULL;
> > }
>
> That isn't right. If the initial kzallc fails we'll goto out with
> pool==NULL.
>
> Please check:
>
> --- a/mm/z3fold.c~z3fold-fix-memory-leak-fix
> +++ a/mm/z3fold.c
> @@ -479,7 +479,7 @@ static struct z3fold_pool *z3fold_create
> pool->name = name;
> pool->compact_wq = create_singlethread_workqueue(pool->name);
> if (!pool->compact_wq)
> - goto out;
> + goto out_unbuddied;
> pool->release_wq = create_singlethread_workqueue(pool->name);
> if (!pool->release_wq)
> goto out_wq;
> @@ -489,9 +489,10 @@ static struct z3fold_pool *z3fold_create
>
> out_wq:
> destroy_workqueue(pool->compact_wq);
> -out:
> +out_unbuddied:
> free_percpu(pool->unbuddied);
> kfree(pool);
> +out:
> return NULL;
> }

We may as well check that __alloc_percpu() return value, too:

--- a/mm/z3fold.c~z3fold-fix-memory-leak-fix
+++ a/mm/z3fold.c
@@ -467,6 +467,8 @@ static struct z3fold_pool *z3fold_create
spin_lock_init(&pool->lock);
spin_lock_init(&pool->stale_lock);
pool->unbuddied = __alloc_percpu(sizeof(struct list_head)*NCHUNKS, 2);
+ if (!pool->unbuddied)
+ goto out_pool;
for_each_possible_cpu(cpu) {
struct list_head *unbuddied =
per_cpu_ptr(pool->unbuddied, cpu);
@@ -479,7 +481,7 @@ static struct z3fold_pool *z3fold_create
pool->name = name;
pool->compact_wq = create_singlethread_workqueue(pool->name);
if (!pool->compact_wq)
- goto out;
+ goto out_unbuddied;
pool->release_wq = create_singlethread_workqueue(pool->name);
if (!pool->release_wq)
goto out_wq;
@@ -489,9 +491,11 @@ static struct z3fold_pool *z3fold_create

out_wq:
destroy_workqueue(pool->compact_wq);
-out:
+out_unbuddied:
free_percpu(pool->unbuddied);
+out_pool:
kfree(pool);
+out:
return NULL;
}

End result:

static struct z3fold_pool *z3fold_create_pool(const char *name, gfp_t gfp,
const struct z3fold_ops *ops)
{
struct z3fold_pool *pool = NULL;
int i, cpu;
pool = kzalloc(sizeof(struct z3fold_pool), gfp);
if (!pool)
goto out;
spin_lock_init(&pool->lock);
spin_lock_init(&pool->stale_lock);
pool->unbuddied = __alloc_percpu(sizeof(struct list_head)*NCHUNKS, 2);
if (!pool->unbuddied)
goto out_pool;
for_each_possible_cpu(cpu) {
struct list_head *unbuddied =
per_cpu_ptr(pool->unbuddied, cpu);
for_each_unbuddied_list(i, 0)
INIT_LIST_HEAD(&unbuddied[i]);
}
INIT_LIST_HEAD(&pool->lru);
INIT_LIST_HEAD(&pool->stale);
atomic64_set(&pool->pages_nr, 0);
pool->name = name;
pool->compact_wq = create_singlethread_workqueue(pool->name);
if (!pool->compact_wq)
goto out_unbuddied;
pool->release_wq = create_singlethread_workqueue(pool->name);
if (!pool->release_wq)
goto out_wq;
INIT_WORK(&pool->work, free_pages_work);
pool->ops = ops;
return pool;
out_wq:
destroy_workqueue(pool->compact_wq);
out_unbuddied:
free_percpu(pool->unbuddied);
out_pool:
kfree(pool);
out:
return NULL;
}
\
 
 \ /
  Last update: 2018-04-05 00:23    [W:0.051 / U:0.348 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site