Re: [dm-devel] [PATCH] libmultipath: fix memory leaks in coalesce_paths

[Benjamin Marzinski <bmarzins@redhat.com>]

On Fri, Oct 16, 2020 at 02:23:58PM +0800, lixiaokeng wrote:
> When multipath -F are executed firstly and multipath -v2 or
> -d are executed later, asan will warn memory leaks. The
> reason is that the mpp allocated in coalesce_paths isn't
> freed. Here we add newmp in configure(multipath) to store
> mpp and free it.
> 
> Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> Signed-off-by: Linfeilong <linfeilong@huawei.com>
> ---
>  libmultipath/configure.c | 12 ++++++++++--
>  multipath/main.c         |  7 +++++--
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index 6fb477fc..fb2c3f73 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -1270,8 +1270,14 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
>  				goto out;
>  			}
>  		}
> -		if (r == DOMAP_DRY)
> +		if (r == DOMAP_DRY) {
> +			if (!vector_alloc_slot(newmp)) {
> +				remove_map(mpp, vecs->pathvec, vecs->mpvec, KEEP_VEC);
> +				goto out;
> +			}
> +			vector_set_slot(newmp, mpp);
>  			continue;
> +		}
> 
>  		if (r == DOMAP_EXIST && mpp->action == ACT_NOTHING &&
>  		    force_reload == FORCE_RELOAD_WEAK)
> @@ -1309,8 +1315,10 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
> 
>  		if (newmp) {
>  			if (mpp->action != ACT_REJECT) {
> -				if (!vector_alloc_slot(newmp))
> +				if (!vector_alloc_slot(newmp)) {
> +					remove_map(mpp, vecs->pathvec, vecs->mpvec, KEEP_VEC);
>  					goto out;
> +				}
>  				vector_set_slot(newmp, mpp);
>  			}
>  			else
> diff --git a/multipath/main.c b/multipath/main.c
> index 9e920d89..5f5b435a 100644
> --- a/multipath/main.c
> +++ b/multipath/main.c

You should rebase this on top of Martin's "libmultipath: improve cleanup
on exit" patchset, since it conflicts with those patches. He's just
posted another version of them.

-Ben

> @@ -472,6 +472,7 @@ configure (struct config *conf, enum mpath_cmds cmd,
>  {
>  	vector curmp = NULL;
>  	vector pathvec = NULL;
> +	vector newmp = NULL;
>  	struct vectors vecs;
>  	int r = RTVL_FAIL, rc;
>  	int di_flag = 0;
> @@ -483,8 +484,9 @@ configure (struct config *conf, enum mpath_cmds cmd,
>  	 */
>  	curmp = vector_alloc();
>  	pathvec = vector_alloc();
> +	newmp = vector_alloc();
> 
> -	if (!curmp || !pathvec) {
> +	if (!curmp || !pathvec || !newmp) {
>  		condlog(0, "can not allocate memory");
>  		goto out;
>  	}
> @@ -586,7 +588,7 @@ configure (struct config *conf, enum mpath_cmds cmd,
>  	/*
>  	 * core logic entry point
>  	 */
> -	rc = coalesce_paths(&vecs, NULL, refwwid,
> +	rc = coalesce_paths(&vecs, newmp, refwwid,
>  			   conf->force_reload, cmd);
>  	r = rc == CP_RETRY ? RTVL_RETRY : rc == CP_OK ? RTVL_OK : RTVL_FAIL;
> 
> @@ -595,6 +597,7 @@ out:
>  		FREE(refwwid);
> 
>  	free_multipathvec(curmp, KEEP_PATHS);
> +	free_multipathvec(newmp, KEEP_PATHS);
>  	free_pathvec(pathvec, FREE_PATHS);
> 
>  	return r;
> -- 

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel