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

[Zhiqiang Liu <liuzhiqiang26@huawei.com>]

Thank you for doing that.

Zhiqiang Liu

On 2021/3/2 21:33, Coly Li wrote:
> 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)
>>
> 
> 
> .
>