diff mbox series

fs/squashfs: fix memory leak in sqfs_read()

Message ID 5f5f0e0c75b54de8b2ef24eb17863d2e@Airspan.com
State Needs Review / ACK
Delegated to: Tom Rini
Headers show
Series fs/squashfs: fix memory leak in sqfs_read() | expand

Commit Message

Barbaros Tokaoglu Oct. 23, 2020, 1:26 p.m. UTC

Comments

João Marcos Costa Oct. 25, 2020, 6:12 p.m. UTC | #1
Reviewed-by: João Marcos Costa <jmcosta944@gmail.com>

Em dom., 25 de out. de 2020 às 14:46, Barbaros Tokaoglu <
btokaoglu@airspan.com> escreveu:

> data_buffer should be freed on each iteration
>
> Signed-off-by: Barbaros Tokaoglu <btokaoglu@airspan.com>
> ---
>  fs/squashfs/sqfs.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index 15208b4..c7ddb0d 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -1355,7 +1355,8 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>   * image with mksquashfs's -b <block_size> option.
>   */
>   printf("Error: too many data blocks to be read.\n");
> - goto free_buffer;
> + free(data_buffer);
> + goto free_datablk;
>   }
>
>   data = data_buffer + table_offset;
> @@ -1365,8 +1366,10 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>   dest_len = get_unaligned_le32(&sblk->block_size);
>   ret = sqfs_decompress(&ctxt, datablock, &dest_len,
>         data, table_size);
> - if (ret)
> - goto free_buffer;
> + if (ret) {
> + free(data_buffer);
> + goto free_datablk;
> + }
>
>   memcpy(buf + offset + *actread, datablock, dest_len);
>   *actread += dest_len;
> @@ -1376,6 +1379,8 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>   }
>
>   data_offset += table_size;
> +
> + free(data_buffer);
>   }
>
>   free(finfo.blk_sizes);
> @@ -1385,7 +1390,7 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>   */
>   if (!finfo.frag) {
>   ret = 0;
> - goto free_buffer;
> + goto free_datablk;
>   }
>
>   start = frag_entry.start / ctxt.cur_dev->blksz;
> @@ -1397,7 +1402,7 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>
>   if (!fragment) {
>   ret = -ENOMEM;
> - goto free_buffer;
> + goto free_datablk;
>   }
>
>   ret = sqfs_disk_read(start, n_blks, fragment);
> @@ -1439,12 +1444,8 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>
>  free_fragment:
>   free(fragment);
> -free_buffer:
> - if (datablk_count)
> - free(data_buffer);
>  free_datablk:
> - if (datablk_count)
> - free(datablock);
> + free(datablock);
>  free_paths:
>   free(file);
>   free(dir);
> --
> 2.7.4
>
> ------------------------------
> *From:* Barbaros Tokaoglu
> *Sent:* Friday, October 23, 2020 4:26:02 PM
> *To:* u-boot@lists.denx.de
> *Cc:* Metin Kaya; jmcosta944@gmail.com
> *Subject:* [PATCH] fs/squashfs: fix memory leak in sqfs_read()
>
>
> On each iteration data_buffer is malloc'ed but not freed which causes
> memory leak and malloc failure on next iterations with bigger files.
>
>
> The patch is to fix this by freeing data_buffer on each iteration.
>
diff mbox series

Patch

From: Barbaros Tokaoglu <btokaoglu@airspan.com>
Date: Fri, 23 Oct 2020 15:52:50 +0300
Subject: [PATCH] fs/squashfs: fix memory leak in sqfs_read()

data_buffer should be freed on each iteration

Signed-off-by: Barbaros Tokaoglu <btokaoglu@airspan.com>
---
 fs/squashfs/sqfs.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 15208b4..c7ddb0d 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1355,7 +1355,8 @@  int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 			 * image with mksquashfs's -b <block_size> option.
 			 */
 			printf("Error: too many data blocks to be read.\n");
-			goto free_buffer;
+			free(data_buffer);
+			goto free_datablk;
 		}
 
 		data = data_buffer + table_offset;
@@ -1365,8 +1366,10 @@  int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 			dest_len = get_unaligned_le32(&sblk->block_size);
 			ret = sqfs_decompress(&ctxt, datablock, &dest_len,
 					      data, table_size);
-			if (ret)
-				goto free_buffer;
+			if (ret) {
+				free(data_buffer);
+				goto free_datablk;
+			}
 
 			memcpy(buf + offset + *actread, datablock, dest_len);
 			*actread += dest_len;
@@ -1376,6 +1379,8 @@  int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 		}
 
 		data_offset += table_size;
+
+		free(data_buffer);
 	}
 
 	free(finfo.blk_sizes);
@@ -1385,7 +1390,7 @@  int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 	 */
 	if (!finfo.frag) {
 		ret = 0;
-		goto free_buffer;
+		goto free_datablk;
 	}
 
 	start = frag_entry.start / ctxt.cur_dev->blksz;
@@ -1397,7 +1402,7 @@  int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 
 	if (!fragment) {
 		ret = -ENOMEM;
-		goto free_buffer;
+		goto free_datablk;
 	}
 
 	ret = sqfs_disk_read(start, n_blks, fragment);
@@ -1439,12 +1444,8 @@  int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 
 free_fragment:
 	free(fragment);
-free_buffer:
-	if (datablk_count)
-		free(data_buffer);
 free_datablk:
-	if (datablk_count)
-		free(datablock);
+	free(datablock);
 free_paths:
 	free(file);
 	free(dir);
-- 
2.7.4