Re: [PATCH] Fix memory leak for function Manage_subdevs Manage_add Kill V2

[Coly Li <colyli@suse.de>]

On Wed, Feb 22, 2023 at 04:30:53PM +0800, miaoguanqin wrote:
> When we test mdadm with asan,we found some memory leaks.
> We fix these memory leaks based on code logic.
> 
> Signed-off-by: miaoguanqin <miaoguanqin@huawei.com>
> ---
>  Assemble.c | 16 +++++++++++++---
>  Kill.c     | 10 +++++++++-
>  Manage.c   | 16 +++++++++++++++-
>  mdadm.c    |  6 ++++++
>  4 files changed, 43 insertions(+), 5 deletions(-)
>

[snipped]
 
> diff --git a/Kill.c b/Kill.c
> index d4767e2..073288e 100644
> --- a/Kill.c
> +++ b/Kill.c

[snipped]

> @@ -77,6 +80,11 @@ int Kill(char *dev, struct supertype *st, int force, int
> verbose, int noexcl)
>  			rv = 0;
>  		}
>  	}
> +	if (flags == 1 && st) {
> +		if (st->sb)
> +			free(st->sb);

May I ask why not call st->ss->free_super(st) ?


> +		free(st);
> +	}
>  	close(fd);
>  	return rv;
>  }

[snipped]

> diff --git a/mdadm.c b/mdadm.c
> index da66c76..981fa98 100644
> --- a/mdadm.c
> +++ b/mdadm.c
> @@ -1765,6 +1765,12 @@ int main(int argc, char *argv[])
>  		autodetect();
>  		break;
>  	}
> +	if (ss) {
> +		if (ss->sb)
> +			free(ss->sb);

Same question, why not call ss->ss->free_super(ss) ?

> +		free(ss);
> +	
> +	}
>  	if (locked)
>  		cluster_release_dlmlock();
>  	if (mdfd > 0)


Overall the patch is fine to me. But it might be better to split it into
multiple patches that each has the changes for a single file.

Thanks.

-- 
Coly Li