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

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:

quoted

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

quoted

---
 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)

.