Re: [PATCH] bcache-tools: fix potential memoryleak problem in, may_add_item()

[Coly Li <colyli@suse.de>]

On 2/27/21 10:38 AM, Zhiqiang Liu wrote:
> 
> In may_add_item(), it will directly return 1 without freeing
> variable tmp and closing fd, when the return value of detail_base()
> is not equal to 0. In addition, we do not check whether
> allocating memory for tmp is successful.
> 
> Here, we will check whether malloc() returns NULL, and
> will free tmp and close fd when detail_base() fails.
> 
> Signed-off-by: ZhiqiangLiu <lzhq28@mail.ustc.edu.cn>

Applied. BTW, I change your above name string from ZhiqiangLiu to
Zhiqiang Liu, because of the checkpatch.pl warning.


Thanks.

Coly Li


> ---
>  lib.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/lib.c b/lib.c
> index 6341c61..745dab6 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -382,7 +382,7 @@ int may_add_item(char *devname, struct list_head *head)
>  	struct cache_sb sb;
>  	char dev[512];
>  	struct dev *tmp;
> -	int ret;
> +	int ret = 0;
> 
>  	if (strcmp(devname, ".") == 0 || strcmp(devname, "..") == 0)
>  		return 0;
> @@ -392,27 +392,33 @@ int may_add_item(char *devname, struct list_head *head)
>  	if (fd == -1)
>  		return 0;
> 
> -	if (pread(fd, &sb_disk, sizeof(sb_disk), SB_START) != sizeof(sb_disk)) {
> -		close(fd);
> -		return 0;
> -	}
> +	if (pread(fd, &sb_disk, sizeof(sb_disk), SB_START) != sizeof(sb_disk))
> +		goto out;
> 
> -	if (memcmp(sb_disk.magic, bcache_magic, 16)) {
> -		close(fd);
> -		return 0;
> -	}
> +	if (memcmp(sb_disk.magic, bcache_magic, 16))
> +		goto out;
> 
>  	to_cache_sb(&sb, &sb_disk);
> 
>  	tmp = (struct dev *) malloc(DEVLEN);
> +	if (tmp == NULL) {
> +		fprintf(stderr, "Error: fail to allocate memory buffer\n");
> +		ret = 1;
> +		goto out;
> +	}
> +
>  	tmp->csum = le64_to_cpu(sb_disk.csum);
>  	ret = detail_base(dev, sb, tmp);
>  	if (ret != 0) {
>  		fprintf(stderr, "Failed to get information for %s\n", dev);
> -		return 1;
> +		free(tmp);
> +		goto out;
>  	}
>  	list_add_tail(&tmp->dev_list, head);
> -	return 0;
> +
> +out:
> +	close(fd);
> +	return ret;
>  }
> 
>  int list_bdevs(struct list_head *head)
>