On Wed, 4 Apr 2018 15:20:39 -0700 Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > On Wed, 4 Apr 2018 08:51:51 +0800 Xidong Wang <wangxidong_97@xxxxxxx> 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; }