Re: [PATCH 1/4] Fix memory leak in file Assemble

[Jes Sorensen <jes@trained-monkey.org>]

On 4/6/23 07:40, Guanqin Miao wrote:
> When we test mdadm with asan, we found some memory leaks in Assemble.c
> We fix these memory leaks based on code logic.
> 
> Signed-off-by: Guanqin Miao <miaoguanqin@huawei.com>
> Signed-off-by: Li Xiao Keng <lixiaokeng@huawei.com>
> ---
>  Assemble.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/Assemble.c b/Assemble.c
> index 49804941..574865eb 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -341,8 +341,10 @@ static int select_devices(struct mddev_dev *devlist,
>  				st->ss->free_super(st);
>  			dev_policy_free(pol);
>  			domain_free(domains);
> -			if (tst)
> +			if (tst) {
>  				tst->ss->free_super(tst);
> +				free(tst);
> +			}
>  			return -1;
>  		}
>  
> @@ -417,6 +419,7 @@ static int select_devices(struct mddev_dev *devlist,
>  				st->ss->free_super(st);
>  				dev_policy_free(pol);
>  				domain_free(domains);
> +				free(st);
>  				return -1;
>  			}
>  			if (c->verbose > 0)
> @@ -425,6 +428,8 @@ static int select_devices(struct mddev_dev *devlist,
>  
>  			/* make sure we finished the loop */
>  			tmpdev = NULL;
> +			if (st)
> +				free(st);

free(NULL) is valid

>  			goto loop;
>  		} else {
>  			content = *contentp;
> @@ -533,6 +538,7 @@ static int select_devices(struct mddev_dev *devlist,
>  				st->ss->free_super(st);
>  				dev_policy_free(pol);
>  				domain_free(domains);
> +				free(tst);
>  				return -1;
>  			}
>  			tmpdev->used = 1;
> @@ -546,8 +552,10 @@ static int select_devices(struct mddev_dev *devlist,
>  		}
>  		dev_policy_free(pol);
>  		pol = NULL;
> -		if (tst)
> +		if (tst) {
>  			tst->ss->free_super(tst);
> +			free(tst);
> +		}
>  	}
>  
>  	/* Check if we found some imsm spares but no members */
> @@ -839,6 +847,7 @@ static int load_devices(struct devs *devices, char *devmap,
>  				close(mdfd);
>  				free(devices);
>  				free(devmap);
> +				free(best);
>  				*stp = st;
>  				return -1;
>  			}
> @@ -1950,6 +1959,8 @@ out:
>  	} else if (mdfd >= 0)
>  		close(mdfd);
>  
> +	if (best)
> +		free(best);

Same here

>  	/* '2' means 'OK, but not started yet' */
>  	if (rv == -1) {
>  		free(devices);