[PATCH 1/1] z3fold: fix memory leak

[PATCH 1/1] z3fold: fix memory leak

[Xidong Wang]

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

Re: [PATCH 1/1] z3fold: fix memory leak

[Andrew Morton]

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;
 }
 
_

Re: [PATCH 1/1] z3fold: fix memory leak

[Andrew Morton]

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;
}

Re: [PATCH 1/1] z3fold: fix memory leak

[kbuild test robot]

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

Re: [PATCH 1/1] z3fold: fix memory leak

[kbuild test robot]

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