# Re: \[PATCH\] bcache-tools: fix potential memoryleak problem in, may\_add\_item()

- From: Zhiqiang Liu \<hidden\>
- Date: 2021-03-03 15:46:42 
- List: linux-bcache
- Page: https://ratatoskr.run/linux-bcache/2021/03/7646487
- Thread: https://ratatoskr.run/linux-bcache/2021/02/7646484/t.md

```text
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 <redacted>
> 
> 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)
>>
> 
> 
> .
> 

```