* [PATCH] bcache-tools: fix potential memoryleak problem in, may_add_item()
@ 2021-02-27 2:38 Zhiqiang Liu
2021-03-02 13:33 ` Coly Li
0 siblings, 1 reply; 3+ messages in thread
From: Zhiqiang Liu @ 2021-02-27 2:38 UTC (permalink / raw)
To: linux-bcache, colyli; +Cc: liuzhiqiang26, linfeilong, lixiaokeng
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>
---
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)
--
2.30.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] bcache-tools: fix potential memoryleak problem in, may_add_item()
2021-02-27 2:38 [PATCH] bcache-tools: fix potential memoryleak problem in, may_add_item() Zhiqiang Liu
@ 2021-03-02 13:33 ` Coly Li
2021-03-03 0:50 ` Zhiqiang Liu
0 siblings, 1 reply; 3+ messages in thread
From: Coly Li @ 2021-03-02 13:33 UTC (permalink / raw)
To: Zhiqiang Liu; +Cc: linfeilong, lixiaokeng, linux-bcache
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)
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] bcache-tools: fix potential memoryleak problem in, may_add_item()
2021-03-02 13:33 ` Coly Li
@ 2021-03-03 0:50 ` Zhiqiang Liu
0 siblings, 0 replies; 3+ messages in thread
From: Zhiqiang Liu @ 2021-03-03 0:50 UTC (permalink / raw)
To: Coly Li; +Cc: linfeilong, lixiaokeng, linux-bcache
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)
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-03 15:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-27 2:38 [PATCH] bcache-tools: fix potential memoryleak problem in, may_add_item() Zhiqiang Liu
2021-03-02 13:33 ` Coly Li
2021-03-03 0:50 ` Zhiqiang Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).