* [PATCH 1/1] z3fold: fix memory leak @ 2018-04-04 0:51 Xidong Wang 2018-04-04 22:20 ` Andrew Morton 2018-04-05 14:57 ` kbuild test robot 0 siblings, 2 replies; 4+ messages in thread From: Xidong Wang @ 2018-04-04 0:51 UTC (permalink / raw) To: Andrew Morton, Vitaly Wool, Mike Rapoport Cc: wangxidong_97, linux-mm, linux-kernel 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. Signed-off-by: Xidong Wang <wangxidong_97@163.com> --- mm/z3fold.c | 1 + 1 file changed, 1 insertion(+) diff --git a/mm/z3fold.c b/mm/z3fold.c index d589d31..b987cc5 100644 --- 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; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] z3fold: fix memory leak 2018-04-04 0:51 [PATCH 1/1] z3fold: fix memory leak Xidong Wang @ 2018-04-04 22:20 ` Andrew Morton 2018-04-04 22:23 ` Andrew Morton 2018-04-05 14:57 ` kbuild test robot 1 sibling, 1 reply; 4+ messages in thread From: Andrew Morton @ 2018-04-04 22:20 UTC (permalink / raw) To: Xidong Wang; +Cc: Vitaly Wool, Mike Rapoport, linux-mm, linux-kernel 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; } _ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] z3fold: fix memory leak 2018-04-04 22:20 ` Andrew Morton @ 2018-04-04 22:23 ` Andrew Morton 0 siblings, 0 replies; 4+ messages in thread From: Andrew Morton @ 2018-04-04 22:23 UTC (permalink / raw) To: Xidong Wang, Vitaly Wool, Mike Rapoport, linux-mm, linux-kernel 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; } ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] z3fold: fix memory leak 2018-04-04 0:51 [PATCH 1/1] z3fold: fix memory leak Xidong Wang 2018-04-04 22:20 ` Andrew Morton @ 2018-04-05 14:57 ` kbuild test robot 1 sibling, 0 replies; 4+ messages in thread From: kbuild test robot @ 2018-04-05 14:57 UTC (permalink / raw) To: Xidong Wang Cc: kbuild-all, Andrew Morton, Vitaly Wool, Mike Rapoport, linux-mm, linux-kernel Hi Xidong, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on mmotm/master] [also build test WARNING on v4.16 next-20180405] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Xidong-Wang/z3fold-fix-memory-leak/20180404-114952 base: git://git.cmpxchg.org/linux-mmotm.git master smatch warnings: mm/z3fold.c:493 z3fold_create_pool() error: potential null dereference 'pool'. (kzalloc returns null) mm/z3fold.c:493 z3fold_create_pool() error: we previously assumed 'pool' could be null (see line 465) vim +/pool +493 mm/z3fold.c 443 444 445 /* 446 * API Functions 447 */ 448 449 /** 450 * z3fold_create_pool() - create a new z3fold pool 451 * @name: pool name 452 * @gfp: gfp flags when allocating the z3fold pool structure 453 * @ops: user-defined operations for the z3fold pool 454 * 455 * Return: pointer to the new z3fold pool or NULL if the metadata allocation 456 * failed. 457 */ 458 static struct z3fold_pool *z3fold_create_pool(const char *name, gfp_t gfp, 459 const struct z3fold_ops *ops) 460 { 461 struct z3fold_pool *pool = NULL; 462 int i, cpu; 463 464 pool = kzalloc(sizeof(struct z3fold_pool), gfp); > 465 if (!pool) 466 goto out; 467 spin_lock_init(&pool->lock); 468 spin_lock_init(&pool->stale_lock); 469 pool->unbuddied = __alloc_percpu(sizeof(struct list_head)*NCHUNKS, 2); 470 for_each_possible_cpu(cpu) { 471 struct list_head *unbuddied = 472 per_cpu_ptr(pool->unbuddied, cpu); 473 for_each_unbuddied_list(i, 0) 474 INIT_LIST_HEAD(&unbuddied[i]); 475 } 476 INIT_LIST_HEAD(&pool->lru); 477 INIT_LIST_HEAD(&pool->stale); 478 atomic64_set(&pool->pages_nr, 0); 479 pool->name = name; 480 pool->compact_wq = create_singlethread_workqueue(pool->name); 481 if (!pool->compact_wq) 482 goto out; 483 pool->release_wq = create_singlethread_workqueue(pool->name); 484 if (!pool->release_wq) 485 goto out_wq; 486 INIT_WORK(&pool->work, free_pages_work); 487 pool->ops = ops; 488 return pool; 489 490 out_wq: 491 destroy_workqueue(pool->compact_wq); 492 out: > 493 free_percpu(pool->unbuddied); 494 kfree(pool); 495 return NULL; 496 } 497 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-04-05 14:57 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-04-04 0:51 [PATCH 1/1] z3fold: fix memory leak Xidong Wang 2018-04-04 22:20 ` Andrew Morton 2018-04-04 22:23 ` Andrew Morton 2018-04-05 14:57 ` kbuild test robot
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).