[PATCH v2 00/28] fs/squashfs: fix memory leaks and introduce exists() function

[PATCH v2 00/28] fs/squashfs: fix memory leaks and introduce exists() function

[Richard Genoud]

This patch series fix several memory leaks, some use of dangling
pointers (leading to cpu freeze) and finally introduce the exists()
function for squashfs.
This function enable testing the existence of a file, which is mandatory
for using the distro_bootcmd
Those fixes have been cut into several patches to be easier to review

Changes since v1:
- patch 5: *dir = *file = NULL; is split in 2 lines
- For consistency, sqfs_frag_lookup is modified to use a single "goto
  out"
( cf https://lists.denx.de/pipermail/u-boot/2020-October/429645.html )
- more memory leak fixes in sqfs_get_abs_path, sqfs_read and sqfs_probe
- a missing error check typo in sqfs_get_abs_path
- some missing reseting ctxt.sblk to NULL to prevent double free
- reset cur_dev/cur_part_info to NULL when they are freed
- return value of sqfs_decompressor_init() wasn't used
- use "len" in sqfs_read to prevent writing beyond buffer
- prevent reading with an offset since it doesn't work
- prevent reading fragmented files since it doesn't work

Richard Genoud (28):
  fs/squashfs: fix board hang-up when calling .exists()
  fs/squashfs: sqfs_opendir: fix some memory leaks and dangling pointers
  fs/squashfs: sqfs_opendir: simplify error handling
  fs/squashfs: sqfs_closedir: fix memory leak
  fs/squashfs: sqfs_split_path: fix memory leak and dangling pointers
  fs/squashfs: sqfs_read_directory_table: fix memory leak
  fs/squashfs: sqfs_search_dir: fix dangling pointer
  fs/squashfs: sqfs_search_dir: fix memory leaks
  fs/squashfs: sqfs_read_inode_table: fix dangling pointer
  fs/squashfs: sqfs_concat_tokens: check if malloc succeeds
  fs/squashfs: sqfs_size: fix dangling pointer dirs->entry
  fs/squashfs: sqfs_size: remove useless sqfs_closedir()
  fs/squashfs: sqfs_read: fix dangling pointer dirs->entry
  fs/squashfs: sqfs_read: remove useless sqfs_closedir()
  fs/squashfs: sqfs_read: fix memory leak
  fs/squashfs: sqfs_read: fix another memory leak
  fs/squashfs: sqfs_frag_lookup: simplify error handling
  fs/squashfs: sqfs_get_abs_path: fix error check
  fs/squashfs: sqfs_get_abs_path: fix possible memory leak on error
  fs/squashfs: sqfs_read: fix memory leak on finfo.blk_sizes
  fs/squashfs: sqfs_probe: fix possible memory leak on error
  fs/squashfs: sqfs_close/sqfs_read_sblk: set ctxt.sblk to NULL after
    free
  fs/squashfs: sqfs_probe: reset cur_dev/cur_part_info to NULL on error
  fs/squashfs: sqfs_probe: use sqfs_decompressor_init() return value
  fs/squashfs: sqfs_read: don't write beyond buffer size
  fs/squashfs: sqfs_read: remove buggy offset functionality
  fs/squashfs: sqfs_read: fragmented files are not supported
  fs/squashfs: implement exists() function

 fs/fs.c            |   7 +
 fs/squashfs/sqfs.c | 399 +++++++++++++++++++++++++++++++--------------
 include/squashfs.h |   1 +
 3 files changed, 286 insertions(+), 121 deletions(-)

[PATCH v2 01/28] fs/squashfs: fix board hang-up when calling .exists()

[Richard Genoud]

add missing squashfs function to prevent dangling or null pointers.
For exemple, when calling test [ -e somefile ], squashfs.exists may be
called.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/fs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/fs.c b/fs/fs.c
index 29ad4d1a695..fb27c910d4f 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -287,6 +287,7 @@ static struct fstype_info fstypes[] = {
 	{
 		.fstype = FS_TYPE_SQUASHFS,
 		.name = "squashfs",
+		.null_dev_desc_ok = false,
 		.probe = sqfs_probe,
 		.opendir = sqfs_opendir,
 		.readdir = sqfs_readdir,
@@ -295,6 +296,12 @@ static struct fstype_info fstypes[] = {
 		.size = sqfs_size,
 		.close = sqfs_close,
 		.closedir = sqfs_closedir,
+		.exists = fs_exists_unsupported,
+		.uuid = fs_uuid_unsupported,
+		.write = fs_write_unsupported,
+		.ln = fs_ln_unsupported,
+		.unlink = fs_unlink_unsupported,
+		.mkdir = fs_mkdir_unsupported,
 	},
 #endif
 	{

[PATCH v2 01/28] fs/squashfs: fix board hang-up when calling .exists()

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> add missing squashfs function to prevent dangling or null pointers.
> For exemple, when calling test [ -e somefile ], squashfs.exists may be
> called.
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/fs.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/fs/fs.c b/fs/fs.c
> index 29ad4d1a695..fb27c910d4f 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -287,6 +287,7 @@ static struct fstype_info fstypes[] = {
>         {
>                 .fstype = FS_TYPE_SQUASHFS,
>                 .name = "squashfs",
> +               .null_dev_desc_ok = false,
>                 .probe = sqfs_probe,
>                 .opendir = sqfs_opendir,
>                 .readdir = sqfs_readdir,
> @@ -295,6 +296,12 @@ static struct fstype_info fstypes[] = {
>                 .size = sqfs_size,
>                 .close = sqfs_close,
>                 .closedir = sqfs_closedir,
> +               .exists = fs_exists_unsupported,
> +               .uuid = fs_uuid_unsupported,
> +               .write = fs_write_unsupported,
> +               .ln = fs_ln_unsupported,
> +               .unlink = fs_unlink_unsupported,
> +               .mkdir = fs_mkdir_unsupported,
>         },
>  #endif
>         {
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 01/28] fs/squashfs: fix board hang-up when calling .exists()

[Tom Rini]

On Tue, Nov 03, 2020 at 12:10:59PM +0100, Richard Genoud wrote:

> add missing squashfs function to prevent dangling or null pointers.
> For exemple, when calling test [ -e somefile ], squashfs.exists may be
> called.
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/b9a796e0/attachment.sig>

[PATCH v2 02/28] fs/squashfs: sqfs_opendir: fix some memory leaks and dangling pointers

[Richard Genoud]

When trying to load an non-existing file, the cpu hangs!

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 15208b4dab0..1fdb9ac534b 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -821,22 +821,37 @@ int sqfs_opendir(const char *filename, struct fs_dir_stream **dirsp)
 	if (!dirs)
 		return -EINVAL;
 
+	/* these should be set to NULL to prevent dangling pointers */
+	dirs->dir_header = NULL;
+	dirs->entry = NULL;
+	dirs->table = NULL;
+	dirs->inode_table = NULL;
+	dirs->dir_table = NULL;
+
 	ret = sqfs_read_inode_table(&inode_table);
-	if (ret)
-		return -EINVAL;
+	if (ret) {
+		ret = -EINVAL;
+		goto free_dirs;
+	}
 
 	metablks_count = sqfs_read_directory_table(&dir_table, &pos_list);
-	if (metablks_count < 1)
-		return -EINVAL;
+	if (metablks_count < 1) {
+		ret = -EINVAL;
+		goto free_inode_table;
+	}
 
 	/* Tokenize filename */
 	token_count = sqfs_count_tokens(filename);
-	if (token_count < 0)
-		return -EINVAL;
+	if (token_count < 0) {
+		ret = -EINVAL;
+		goto free_inode_table;
+	}
 
 	path = strdup(filename);
-	if (!path)
-		return -ENOMEM;
+	if (!path) {
+		ret = -EINVAL;
+		goto free_inode_table;
+	}
 
 	token_list = malloc(token_count * sizeof(char *));
 	if (!token_list) {
@@ -882,6 +897,12 @@ free_tokens:
 	free(pos_list);
 free_path:
 	free(path);
+free_inode_table:
+	if (ret)
+		free(inode_table);
+free_dirs:
+	if (ret)
+		free(dirs);
 
 	return ret;
 }

[PATCH v2 02/28] fs/squashfs: sqfs_opendir: fix some memory leaks and dangling pointers

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:00PM +0100, Richard Genoud wrote:

> When trying to load an non-existing file, the cpu hangs!
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/7fcb06c0/attachment.sig>

[PATCH v2 03/28] fs/squashfs: sqfs_opendir: simplify error handling

[Richard Genoud]

Using only one label permits to prevents bugs when moving code around.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 1fdb9ac534b..b94a9715205 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -812,9 +812,9 @@ free_dtb:
 int sqfs_opendir(const char *filename, struct fs_dir_stream **dirsp)
 {
 	unsigned char *inode_table = NULL, *dir_table = NULL;
-	int j, token_count, ret = 0, metablks_count;
+	int j, token_count = 0, ret = 0, metablks_count;
 	struct squashfs_dir_stream *dirs;
-	char **token_list, *path;
+	char **token_list = NULL, *path = NULL;
 	u32 *pos_list = NULL;
 
 	dirs = malloc(sizeof(*dirs));
@@ -831,38 +831,38 @@ int sqfs_opendir(const char *filename, struct fs_dir_stream **dirsp)
 	ret = sqfs_read_inode_table(&inode_table);
 	if (ret) {
 		ret = -EINVAL;
-		goto free_dirs;
+		goto out;
 	}
 
 	metablks_count = sqfs_read_directory_table(&dir_table, &pos_list);
 	if (metablks_count < 1) {
 		ret = -EINVAL;
-		goto free_inode_table;
+		goto out;
 	}
 
 	/* Tokenize filename */
 	token_count = sqfs_count_tokens(filename);
 	if (token_count < 0) {
 		ret = -EINVAL;
-		goto free_inode_table;
+		goto out;
 	}
 
 	path = strdup(filename);
 	if (!path) {
 		ret = -EINVAL;
-		goto free_inode_table;
+		goto out;
 	}
 
 	token_list = malloc(token_count * sizeof(char *));
 	if (!token_list) {
 		ret = -EINVAL;
-		goto free_path;
+		goto out;
 	}
 
 	/* Fill tokens list */
 	ret = sqfs_tokenize(token_list, token_count, path);
 	if (ret)
-		goto free_tokens;
+		goto out;
 	/*
 	 * ldir's (extended directory) size is greater than dir, so it works as
 	 * a general solution for the malloc size, since 'i' is a union.
@@ -872,7 +872,7 @@ int sqfs_opendir(const char *filename, struct fs_dir_stream **dirsp)
 	ret = sqfs_search_dir(dirs, token_list, token_count, pos_list,
 			      metablks_count);
 	if (ret)
-		goto free_tokens;
+		goto out;
 
 	if (le16_to_cpu(dirs->i_dir.inode_type) == SQFS_DIR_TYPE)
 		dirs->size = le16_to_cpu(dirs->i_dir.file_size);
@@ -890,19 +890,16 @@ int sqfs_opendir(const char *filename, struct fs_dir_stream **dirsp)
 
 	*dirsp = (struct fs_dir_stream *)dirs;
 
-free_tokens:
+out:
 	for (j = 0; j < token_count; j++)
 		free(token_list[j]);
 	free(token_list);
 	free(pos_list);
-free_path:
 	free(path);
-free_inode_table:
-	if (ret)
+	if (ret) {
 		free(inode_table);
-free_dirs:
-	if (ret)
 		free(dirs);
+	}
 
 	return ret;
 }

[PATCH v2 03/28] fs/squashfs: sqfs_opendir: simplify error handling

[João Marcos Costa]

I am not sure if this simplification follows the most commonly used
strategy for handling errors (in U-Boot's source code), but it does not
bother me either.
Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>


Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> Using only one label permits to prevents bugs when moving code around.
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index 1fdb9ac534b..b94a9715205 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -812,9 +812,9 @@ free_dtb:
>  int sqfs_opendir(const char *filename, struct fs_dir_stream **dirsp)
>  {
>         unsigned char *inode_table = NULL, *dir_table = NULL;
> -       int j, token_count, ret = 0, metablks_count;
> +       int j, token_count = 0, ret = 0, metablks_count;
>         struct squashfs_dir_stream *dirs;
> -       char **token_list, *path;
> +       char **token_list = NULL, *path = NULL;
>         u32 *pos_list = NULL;
>
>         dirs = malloc(sizeof(*dirs));
> @@ -831,38 +831,38 @@ int sqfs_opendir(const char *filename, struct
> fs_dir_stream **dirsp)
>         ret = sqfs_read_inode_table(&inode_table);
>         if (ret) {
>                 ret = -EINVAL;
> -               goto free_dirs;
> +               goto out;
>         }
>
>         metablks_count = sqfs_read_directory_table(&dir_table, &pos_list);
>         if (metablks_count < 1) {
>                 ret = -EINVAL;
> -               goto free_inode_table;
> +               goto out;
>         }
>
>         /* Tokenize filename */
>         token_count = sqfs_count_tokens(filename);
>         if (token_count < 0) {
>                 ret = -EINVAL;
> -               goto free_inode_table;
> +               goto out;
>         }
>
>         path = strdup(filename);
>         if (!path) {
>                 ret = -EINVAL;
> -               goto free_inode_table;
> +               goto out;
>         }
>
>         token_list = malloc(token_count * sizeof(char *));
>         if (!token_list) {
>                 ret = -EINVAL;
> -               goto free_path;
> +               goto out;
>         }
>
>         /* Fill tokens list */
>         ret = sqfs_tokenize(token_list, token_count, path);
>         if (ret)
> -               goto free_tokens;
> +               goto out;
>         /*
>          * ldir's (extended directory) size is greater than dir, so it
> works as
>          * a general solution for the malloc size, since 'i' is a union.
> @@ -872,7 +872,7 @@ int sqfs_opendir(const char *filename, struct
> fs_dir_stream **dirsp)
>         ret = sqfs_search_dir(dirs, token_list, token_count, pos_list,
>                               metablks_count);
>         if (ret)
> -               goto free_tokens;
> +               goto out;
>
>         if (le16_to_cpu(dirs->i_dir.inode_type) == SQFS_DIR_TYPE)
>                 dirs->size = le16_to_cpu(dirs->i_dir.file_size);
> @@ -890,19 +890,16 @@ int sqfs_opendir(const char *filename, struct
> fs_dir_stream **dirsp)
>
>         *dirsp = (struct fs_dir_stream *)dirs;
>
> -free_tokens:
> +out:
>         for (j = 0; j < token_count; j++)
>                 free(token_list[j]);
>         free(token_list);
>         free(pos_list);
> -free_path:
>         free(path);
> -free_inode_table:
> -       if (ret)
> +       if (ret) {
>                 free(inode_table);
> -free_dirs:
> -       if (ret)
>                 free(dirs);
> +       }
>
>         return ret;
>  }
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 03/28] fs/squashfs: sqfs_opendir: simplify error handling

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:01PM +0100, Richard Genoud wrote:

> Using only one label permits to prevents bugs when moving code around.
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/34050cbf/attachment.sig>

[PATCH v2 04/28] fs/squashfs: sqfs_closedir: fix memory leak

[Richard Genoud]

sqfs_dirs wasn't freed anywhere.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index b94a9715205..0ac922af9e7 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1573,4 +1573,5 @@ void sqfs_closedir(struct fs_dir_stream *dirs)
 	free(sqfs_dirs->inode_table);
 	free(sqfs_dirs->dir_table);
 	free(sqfs_dirs->dir_header);
+	free(sqfs_dirs);
 }

[PATCH v2 04/28] fs/squashfs: sqfs_closedir: fix memory leak

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> sqfs_dirs wasn't freed anywhere.
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index b94a9715205..0ac922af9e7 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -1573,4 +1573,5 @@ void sqfs_closedir(struct fs_dir_stream *dirs)
>         free(sqfs_dirs->inode_table);
>         free(sqfs_dirs->dir_table);
>         free(sqfs_dirs->dir_header);
> +       free(sqfs_dirs);
>  }
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 04/28] fs/squashfs: sqfs_closedir: fix memory leak

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:02PM +0100, Richard Genoud wrote:

> sqfs_dirs wasn't freed anywhere.
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/66376014/attachment.sig>

[PATCH v2 05/28] fs/squashfs: sqfs_split_path: fix memory leak and dangling pointers

[Richard Genoud]

*file and *dir were not freed on error

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 0ac922af9e7..58b8bfc66dc 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1089,15 +1089,27 @@ static int sqfs_split_path(char **file, char **dir, const char *path)
 	char *dirc, *basec, *bname, *dname, *tmp_path;
 	int ret = 0;
 
+	*file = NULL;
+	*dir = NULL;
+	dirc = NULL;
+	basec = NULL;
+	bname = NULL;
+	dname = NULL;
+	tmp_path = NULL;
+
 	/* check for first slash in path*/
 	if (path[0] == '/') {
 		tmp_path = strdup(path);
-		if (!tmp_path)
-			return -ENOMEM;
+		if (!tmp_path) {
+			ret = -ENOMEM;
+			goto out;
+		}
 	} else {
 		tmp_path = malloc(strlen(path) + 2);
-		if (!tmp_path)
-			return -ENOMEM;
+		if (!tmp_path) {
+			ret = -ENOMEM;
+			goto out;
+		}
 		tmp_path[0] = '/';
 		strcpy(tmp_path + 1, path);
 	}
@@ -1106,13 +1118,13 @@ static int sqfs_split_path(char **file, char **dir, const char *path)
 	dirc = strdup(tmp_path);
 	if (!dirc) {
 		ret = -ENOMEM;
-		goto free_tmp;
+		goto out;
 	}
 
 	basec = strdup(tmp_path);
 	if (!basec) {
 		ret = -ENOMEM;
-		goto free_dirc;
+		goto out;
 	}
 
 	dname = sqfs_dirname(dirc);
@@ -1122,14 +1134,14 @@ static int sqfs_split_path(char **file, char **dir, const char *path)
 
 	if (!*file) {
 		ret = -ENOMEM;
-		goto free_basec;
+		goto out;
 	}
 
 	if (*dname == '\0') {
 		*dir = malloc(2);
 		if (!*dir) {
 			ret = -ENOMEM;
-			goto free_basec;
+			goto out;
 		}
 
 		(*dir)[0] = '/';
@@ -1138,15 +1150,19 @@ static int sqfs_split_path(char **file, char **dir, const char *path)
 		*dir = strdup(dname);
 		if (!*dir) {
 			ret = -ENOMEM;
-			goto free_basec;
+			goto out;
 		}
 	}
 
-free_basec:
+out:
+	if (ret) {
+		free(*file);
+		free(*dir);
+		*dir = NULL;
+		*file = NULL;
+	}
 	free(basec);
-free_dirc:
 	free(dirc);
-free_tmp:
 	free(tmp_path);
 
 	return ret;

[PATCH v2 05/28] fs/squashfs: sqfs_split_path: fix memory leak and dangling pointers

[João Marcos Costa]

Same comment about the error path simplification.
Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> *file and *dir were not freed on error
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 40 ++++++++++++++++++++++++++++------------
>  1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index 0ac922af9e7..58b8bfc66dc 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -1089,15 +1089,27 @@ static int sqfs_split_path(char **file, char
> **dir, const char *path)
>         char *dirc, *basec, *bname, *dname, *tmp_path;
>         int ret = 0;
>
> +       *file = NULL;
> +       *dir = NULL;
> +       dirc = NULL;
> +       basec = NULL;
> +       bname = NULL;
> +       dname = NULL;
> +       tmp_path = NULL;
> +
>         /* check for first slash in path*/
>         if (path[0] == '/') {
>                 tmp_path = strdup(path);
> -               if (!tmp_path)
> -                       return -ENOMEM;
> +               if (!tmp_path) {
> +                       ret = -ENOMEM;
> +                       goto out;
> +               }
>         } else {
>                 tmp_path = malloc(strlen(path) + 2);
> -               if (!tmp_path)
> -                       return -ENOMEM;
> +               if (!tmp_path) {
> +                       ret = -ENOMEM;
> +                       goto out;
> +               }
>                 tmp_path[0] = '/';
>                 strcpy(tmp_path + 1, path);
>         }
> @@ -1106,13 +1118,13 @@ static int sqfs_split_path(char **file, char
> **dir, const char *path)
>         dirc = strdup(tmp_path);
>         if (!dirc) {
>                 ret = -ENOMEM;
> -               goto free_tmp;
> +               goto out;
>         }
>
>         basec = strdup(tmp_path);
>         if (!basec) {
>                 ret = -ENOMEM;
> -               goto free_dirc;
> +               goto out;
>         }
>
>         dname = sqfs_dirname(dirc);
> @@ -1122,14 +1134,14 @@ static int sqfs_split_path(char **file, char
> **dir, const char *path)
>
>         if (!*file) {
>                 ret = -ENOMEM;
> -               goto free_basec;
> +               goto out;
>         }
>
>         if (*dname == '\0') {
>                 *dir = malloc(2);
>                 if (!*dir) {
>                         ret = -ENOMEM;
> -                       goto free_basec;
> +                       goto out;
>                 }
>
>                 (*dir)[0] = '/';
> @@ -1138,15 +1150,19 @@ static int sqfs_split_path(char **file, char
> **dir, const char *path)
>                 *dir = strdup(dname);
>                 if (!*dir) {
>                         ret = -ENOMEM;
> -                       goto free_basec;
> +                       goto out;
>                 }
>         }
>
> -free_basec:
> +out:
> +       if (ret) {
> +               free(*file);
> +               free(*dir);
> +               *dir = NULL;
> +               *file = NULL;
> +       }
>         free(basec);
> -free_dirc:
>         free(dirc);
> -free_tmp:
>         free(tmp_path);
>
>         return ret;
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 05/28] fs/squashfs: sqfs_split_path: fix memory leak and dangling pointers

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:03PM +0100, Richard Genoud wrote:

> *file and *dir were not freed on error
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/a5fac421/attachment.sig>

[PATCH v2 06/28] fs/squashfs: sqfs_read_directory_table: fix memory leak

[Richard Genoud]

pos_list wasn't freed on every error

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 58b8bfc66dc..9d460e8bed6 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -722,6 +722,8 @@ static int sqfs_read_directory_table(unsigned char **dir_table, u32 **pos_list)
 	unsigned long dest_len = 0;
 	bool compressed;
 
+	*dir_table = NULL;
+	*pos_list = NULL;
 	/* DIRECTORY TABLE */
 	table_size = get_unaligned_le64(&sblk->fragment_table_start) -
 		get_unaligned_le64(&sblk->directory_table_start);
@@ -736,35 +738,31 @@ static int sqfs_read_directory_table(unsigned char **dir_table, u32 **pos_list)
 		return -ENOMEM;
 
 	if (sqfs_disk_read(start, n_blks, dtb) < 0)
-		goto free_dtb;
+		goto out;
 
 	/* Parse directory table (metadata block) header */
 	ret = sqfs_read_metablock(dtb, table_offset, &compressed, &src_len);
 	if (ret)
-		goto free_dtb;
+		goto out;
 
 	/* Calculate total size to store the whole decompressed table */
 	metablks_count = sqfs_count_metablks(dtb, table_offset, table_size);
 	if (metablks_count < 1)
-		goto free_dtb;
+		goto out;
 
 	*dir_table = malloc(metablks_count * SQFS_METADATA_BLOCK_SIZE);
 	if (!*dir_table)
-		goto free_dtb;
+		goto out;
 
 	*pos_list = malloc(metablks_count * sizeof(u32));
-	if (!*pos_list) {
-		free(*dir_table);
-		goto free_dtb;
-	}
+	if (!*pos_list)
+		goto out;
 
 	ret = sqfs_get_metablk_pos(*pos_list, dtb, table_offset,
 				   metablks_count);
 	if (ret) {
 		metablks_count = -1;
-		free(*dir_table);
-		free(*pos_list);
-		goto free_dtb;
+		goto out;
 	}
 
 	src_table = dtb + table_offset + SQFS_HEADER_SIZE;
@@ -780,8 +778,7 @@ static int sqfs_read_directory_table(unsigned char **dir_table, u32 **pos_list)
 					      &dest_len, src_table, src_len);
 			if (ret) {
 				metablks_count = -1;
-				free(*dir_table);
-				goto free_dtb;
+				goto out;
 			}
 
 			if (dest_len < SQFS_METADATA_BLOCK_SIZE) {
@@ -803,7 +800,13 @@ static int sqfs_read_directory_table(unsigned char **dir_table, u32 **pos_list)
 		src_table += src_len + SQFS_HEADER_SIZE;
 	}
 
-free_dtb:
+out:
+	if (metablks_count < 1) {
+		free(*dir_table);
+		free(*pos_list);
+		*dir_table = NULL;
+		*pos_list = NULL;
+	}
 	free(dtb);
 
 	return metablks_count;

[PATCH v2 06/28] fs/squashfs: sqfs_read_directory_table: fix memory leak

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> pos_list wasn't freed on every error
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index 58b8bfc66dc..9d460e8bed6 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -722,6 +722,8 @@ static int sqfs_read_directory_table(unsigned char
> **dir_table, u32 **pos_list)
>         unsigned long dest_len = 0;
>         bool compressed;
>
> +       *dir_table = NULL;
> +       *pos_list = NULL;
>         /* DIRECTORY TABLE */
>         table_size = get_unaligned_le64(&sblk->fragment_table_start) -
>                 get_unaligned_le64(&sblk->directory_table_start);
> @@ -736,35 +738,31 @@ static int sqfs_read_directory_table(unsigned char
> **dir_table, u32 **pos_list)
>                 return -ENOMEM;
>
>         if (sqfs_disk_read(start, n_blks, dtb) < 0)
> -               goto free_dtb;
> +               goto out;
>
>         /* Parse directory table (metadata block) header */
>         ret = sqfs_read_metablock(dtb, table_offset, &compressed,
> &src_len);
>         if (ret)
> -               goto free_dtb;
> +               goto out;
>
>         /* Calculate total size to store the whole decompressed table */
>         metablks_count = sqfs_count_metablks(dtb, table_offset,
> table_size);
>         if (metablks_count < 1)
> -               goto free_dtb;
> +               goto out;
>
>         *dir_table = malloc(metablks_count * SQFS_METADATA_BLOCK_SIZE);
>         if (!*dir_table)
> -               goto free_dtb;
> +               goto out;
>
>         *pos_list = malloc(metablks_count * sizeof(u32));
> -       if (!*pos_list) {
> -               free(*dir_table);
> -               goto free_dtb;
> -       }
> +       if (!*pos_list)
> +               goto out;
>
>         ret = sqfs_get_metablk_pos(*pos_list, dtb, table_offset,
>                                    metablks_count);
>         if (ret) {
>                 metablks_count = -1;
> -               free(*dir_table);
> -               free(*pos_list);
> -               goto free_dtb;
> +               goto out;
>         }
>
>         src_table = dtb + table_offset + SQFS_HEADER_SIZE;
> @@ -780,8 +778,7 @@ static int sqfs_read_directory_table(unsigned char
> **dir_table, u32 **pos_list)
>                                               &dest_len, src_table,
> src_len);
>                         if (ret) {
>                                 metablks_count = -1;
> -                               free(*dir_table);
> -                               goto free_dtb;
> +                               goto out;
>                         }
>
>                         if (dest_len < SQFS_METADATA_BLOCK_SIZE) {
> @@ -803,7 +800,13 @@ static int sqfs_read_directory_table(unsigned char
> **dir_table, u32 **pos_list)
>                 src_table += src_len + SQFS_HEADER_SIZE;
>         }
>
> -free_dtb:
> +out:
> +       if (metablks_count < 1) {
> +               free(*dir_table);
> +               free(*pos_list);
> +               *dir_table = NULL;
> +               *pos_list = NULL;
> +       }
>         free(dtb);
>
>         return metablks_count;
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 06/28] fs/squashfs: sqfs_read_directory_table: fix memory leak

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:04PM +0100, Richard Genoud wrote:

> pos_list wasn't freed on every error
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/d332a0a5/attachment.sig>

[PATCH v2 07/28] fs/squashfs: sqfs_search_dir: fix dangling pointer

[Richard Genoud]

dirs->entry shouldn't be left dangling as it could be freed twice.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 9d460e8bed6..78893b5c85d 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -485,6 +485,7 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 			if (!ret)
 				break;
 			free(dirs->entry);
+			dirs->entry = NULL;
 		}
 
 		if (ret) {
@@ -530,6 +531,7 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 			if (ret)
 				return -EINVAL;
 			free(dirs->entry);
+			dirs->entry = NULL;
 
 			ret = sqfs_search_dir(dirs, sym_tokens, token_count,
 					      m_list, m_count);
@@ -537,6 +539,7 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 		} else if (!sqfs_is_dir(get_unaligned_le16(&dir->inode_type))) {
 			printf("** Cannot find directory. **\n");
 			free(dirs->entry);
+			dirs->entry = NULL;
 			return -EINVAL;
 		}
 
@@ -556,6 +559,7 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 		if (sqfs_is_empty_dir(table)) {
 			printf("Empty directory.\n");
 			free(dirs->entry);
+			dirs->entry = NULL;
 			return SQFS_EMPTY_DIR;
 		}
 
@@ -564,6 +568,7 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 		dirs->entry_count = dirs->dir_header->count + 1;
 		dirs->size -= SQFS_DIR_HEADER_SIZE;
 		free(dirs->entry);
+		dirs->entry = NULL;
 	}
 
 	offset = sqfs_dir_offset(table, m_list, m_count);

[PATCH v2 07/28] fs/squashfs: sqfs_search_dir: fix dangling pointer

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:05PM +0100, Richard Genoud wrote:

> dirs->entry shouldn't be left dangling as it could be freed twice.
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/2b5712ca/attachment.sig>

[PATCH v2 08/28] fs/squashfs: sqfs_search_dir: fix memory leaks

[Richard Genoud]

path, target, res, rem and sym_tokens were not free on error nor success.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 64 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 51 insertions(+), 13 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 78893b5c85d..1714306e747 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -434,7 +434,7 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 {
 	struct squashfs_super_block *sblk = ctxt.sblk;
 	char *path, *target, **sym_tokens, *res, *rem;
-	int j, ret, new_inode_number, offset;
+	int j, ret = 0, new_inode_number, offset;
 	struct squashfs_symlink_inode *sym;
 	struct squashfs_ldir_inode *ldir;
 	struct squashfs_dir_inode *dir;
@@ -442,6 +442,12 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 	struct fs_dirent *dent;
 	unsigned char *table;
 
+	res = NULL;
+	rem = NULL;
+	path = NULL;
+	target = NULL;
+	sym_tokens = NULL;
+
 	dirsp = (struct fs_dir_stream *)dirs;
 
 	/* Start by root inode */
@@ -477,7 +483,8 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 	for (j = 0; j < token_count; j++) {
 		if (!sqfs_is_dir(get_unaligned_le16(&dir->inode_type))) {
 			printf("** Cannot find directory. **\n");
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out;
 		}
 
 		while (!sqfs_readdir(dirsp, &dent)) {
@@ -490,7 +497,8 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 
 		if (ret) {
 			printf("** Cannot find directory. **\n");
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out;
 		}
 
 		/* Redefine inode as the found token */
@@ -507,40 +515,63 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 			sym = (struct squashfs_symlink_inode *)table;
 			/* Get first j + 1 tokens */
 			path = sqfs_concat_tokens(token_list, j + 1);
+			if (!path) {
+				ret = -ENOMEM;
+				goto out;
+			}
 			/* Resolve for these tokens */
 			target = sqfs_resolve_symlink(sym, path);
+			if (!target) {
+				ret = -ENOMEM;
+				goto out;
+			}
 			/* Join remaining tokens */
 			rem = sqfs_concat_tokens(token_list + j + 1, token_count -
 						 j - 1);
+			if (!rem) {
+				ret = -ENOMEM;
+				goto out;
+			}
 			/* Concatenate remaining tokens and symlink's target */
 			res = malloc(strlen(rem) + strlen(target) + 1);
+			if (!res) {
+				ret = -ENOMEM;
+				goto out;
+			}
 			strcpy(res, target);
 			res[strlen(target)] = '/';
 			strcpy(res + strlen(target) + 1, rem);
 			token_count = sqfs_count_tokens(res);
 
-			if (token_count < 0)
-				return -EINVAL;
+			if (token_count < 0) {
+				ret = -EINVAL;
+				goto out;
+			}
 
 			sym_tokens = malloc(token_count * sizeof(char *));
-			if (!sym_tokens)
-				return -EINVAL;
+			if (!sym_tokens) {
+				ret = -EINVAL;
+				goto out;
+			}
 
 			/* Fill tokens list */
 			ret = sqfs_tokenize(sym_tokens, token_count, res);
-			if (ret)
-				return -EINVAL;
+			if (ret) {
+				ret = -EINVAL;
+				goto out;
+			}
 			free(dirs->entry);
 			dirs->entry = NULL;
 
 			ret = sqfs_search_dir(dirs, sym_tokens, token_count,
 					      m_list, m_count);
-			return ret;
+			goto out;
 		} else if (!sqfs_is_dir(get_unaligned_le16(&dir->inode_type))) {
 			printf("** Cannot find directory. **\n");
 			free(dirs->entry);
 			dirs->entry = NULL;
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out;
 		}
 
 		/* Check if it is an extended dir. */
@@ -560,7 +591,8 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 			printf("Empty directory.\n");
 			free(dirs->entry);
 			dirs->entry = NULL;
-			return SQFS_EMPTY_DIR;
+			ret = SQFS_EMPTY_DIR;
+			goto out;
 		}
 
 		dirs->table += SQFS_DIR_HEADER_SIZE;
@@ -579,7 +611,13 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 	else
 		memcpy(&dirs->i_ldir, ldir, sizeof(*ldir));
 
-	return 0;
+out:
+	free(res);
+	free(rem);
+	free(path);
+	free(target);
+	free(sym_tokens);
+	return ret;
 }
 
 /*

[PATCH v2 08/28] fs/squashfs: sqfs_search_dir: fix memory leaks

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> path, target, res, rem and sym_tokens were not free on error nor success.
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 64 ++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 51 insertions(+), 13 deletions(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index 78893b5c85d..1714306e747 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -434,7 +434,7 @@ static int sqfs_search_dir(struct squashfs_dir_stream
> *dirs, char **token_list,
>  {
>         struct squashfs_super_block *sblk = ctxt.sblk;
>         char *path, *target, **sym_tokens, *res, *rem;
> -       int j, ret, new_inode_number, offset;
> +       int j, ret = 0, new_inode_number, offset;
>         struct squashfs_symlink_inode *sym;
>         struct squashfs_ldir_inode *ldir;
>         struct squashfs_dir_inode *dir;
> @@ -442,6 +442,12 @@ static int sqfs_search_dir(struct squashfs_dir_stream
> *dirs, char **token_list,
>         struct fs_dirent *dent;
>         unsigned char *table;
>
> +       res = NULL;
> +       rem = NULL;
> +       path = NULL;
> +       target = NULL;
> +       sym_tokens = NULL;
> +
>         dirsp = (struct fs_dir_stream *)dirs;
>
>         /* Start by root inode */
> @@ -477,7 +483,8 @@ static int sqfs_search_dir(struct squashfs_dir_stream
> *dirs, char **token_list,
>         for (j = 0; j < token_count; j++) {
>                 if (!sqfs_is_dir(get_unaligned_le16(&dir->inode_type))) {
>                         printf("** Cannot find directory. **\n");
> -                       return -EINVAL;
> +                       ret = -EINVAL;
> +                       goto out;
>                 }
>
>                 while (!sqfs_readdir(dirsp, &dent)) {
> @@ -490,7 +497,8 @@ static int sqfs_search_dir(struct squashfs_dir_stream
> *dirs, char **token_list,
>
>                 if (ret) {
>                         printf("** Cannot find directory. **\n");
> -                       return -EINVAL;
> +                       ret = -EINVAL;
> +                       goto out;
>                 }
>
>                 /* Redefine inode as the found token */
> @@ -507,40 +515,63 @@ static int sqfs_search_dir(struct
> squashfs_dir_stream *dirs, char **token_list,
>                         sym = (struct squashfs_symlink_inode *)table;
>                         /* Get first j + 1 tokens */
>                         path = sqfs_concat_tokens(token_list, j + 1);
> +                       if (!path) {
> +                               ret = -ENOMEM;
> +                               goto out;
> +                       }
>                         /* Resolve for these tokens */
>                         target = sqfs_resolve_symlink(sym, path);
> +                       if (!target) {
> +                               ret = -ENOMEM;
> +                               goto out;
> +                       }
>                         /* Join remaining tokens */
>                         rem = sqfs_concat_tokens(token_list + j + 1,
> token_count -
>                                                  j - 1);
> +                       if (!rem) {
> +                               ret = -ENOMEM;
> +                               goto out;
> +                       }
>                         /* Concatenate remaining tokens and symlink's
> target */
>                         res = malloc(strlen(rem) + strlen(target) + 1);
> +                       if (!res) {
> +                               ret = -ENOMEM;
> +                               goto out;
> +                       }
>                         strcpy(res, target);
>                         res[strlen(target)] = '/';
>                         strcpy(res + strlen(target) + 1, rem);
>                         token_count = sqfs_count_tokens(res);
>
> -                       if (token_count < 0)
> -                               return -EINVAL;
> +                       if (token_count < 0) {
> +                               ret = -EINVAL;
> +                               goto out;
> +                       }
>
>                         sym_tokens = malloc(token_count * sizeof(char *));
> -                       if (!sym_tokens)
> -                               return -EINVAL;
> +                       if (!sym_tokens) {
> +                               ret = -EINVAL;
> +                               goto out;
> +                       }
>
>                         /* Fill tokens list */
>                         ret = sqfs_tokenize(sym_tokens, token_count, res);
> -                       if (ret)
> -                               return -EINVAL;
> +                       if (ret) {
> +                               ret = -EINVAL;
> +                               goto out;
> +                       }
>                         free(dirs->entry);
>                         dirs->entry = NULL;
>
>                         ret = sqfs_search_dir(dirs, sym_tokens,
> token_count,
>                                               m_list, m_count);
> -                       return ret;
> +                       goto out;
>                 } else if
> (!sqfs_is_dir(get_unaligned_le16(&dir->inode_type))) {
>                         printf("** Cannot find directory. **\n");
>                         free(dirs->entry);
>                         dirs->entry = NULL;
> -                       return -EINVAL;
> +                       ret = -EINVAL;
> +                       goto out;
>                 }
>
>                 /* Check if it is an extended dir. */
> @@ -560,7 +591,8 @@ static int sqfs_search_dir(struct squashfs_dir_stream
> *dirs, char **token_list,
>                         printf("Empty directory.\n");
>                         free(dirs->entry);
>                         dirs->entry = NULL;
> -                       return SQFS_EMPTY_DIR;
> +                       ret = SQFS_EMPTY_DIR;
> +                       goto out;
>                 }
>
>                 dirs->table += SQFS_DIR_HEADER_SIZE;
> @@ -579,7 +611,13 @@ static int sqfs_search_dir(struct squashfs_dir_stream
> *dirs, char **token_list,
>         else
>                 memcpy(&dirs->i_ldir, ldir, sizeof(*ldir));
>
> -       return 0;
> +out:
> +       free(res);
> +       free(rem);
> +       free(path);
> +       free(target);
> +       free(sym_tokens);
> +       return ret;
>  }
>
>  /*
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 08/28] fs/squashfs: sqfs_search_dir: fix memory leaks

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:06PM +0100, Richard Genoud wrote:

> path, target, res, rem and sym_tokens were not free on error nor success.
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/5a4d1b0e/attachment.sig>

[PATCH v2 09/28] fs/squashfs: sqfs_read_inode_table: fix dangling pointer

[Richard Genoud]

inode_table should not be left dangling as it may be freed in sqfs_opendir

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 1714306e747..72181f38332 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -731,6 +731,7 @@ static int sqfs_read_inode_table(unsigned char **inode_table)
 					      src_table, src_len);
 			if (ret) {
 				free(*inode_table);
+				*inode_table = NULL;
 				goto free_itb;
 			}
 

[PATCH v2 09/28] fs/squashfs: sqfs_read_inode_table: fix dangling pointer

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:07PM +0100, Richard Genoud wrote:

> inode_table should not be left dangling as it may be freed in sqfs_opendir
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/c6ccb396/attachment.sig>

[PATCH v2 10/28] fs/squashfs: sqfs_concat_tokens: check if malloc succeeds

[Richard Genoud]

memory allocation should always be checked

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 72181f38332..7da2e09cc36 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -242,6 +242,9 @@ static char *sqfs_concat_tokens(char **token_list, int token_count)
 	length = sqfs_get_tokens_length(token_list, token_count);
 
 	result = malloc(length + 1);
+	if (!result)
+		return NULL;
+
 	result[length] = '\0';
 
 	for (i = 0; i < token_count; i++) {

[PATCH v2 10/28] fs/squashfs: sqfs_concat_tokens: check if malloc succeeds

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> memory allocation should always be checked
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index 72181f38332..7da2e09cc36 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -242,6 +242,9 @@ static char *sqfs_concat_tokens(char **token_list, int
> token_count)
>         length = sqfs_get_tokens_length(token_list, token_count);
>
>         result = malloc(length + 1);
> +       if (!result)
> +               return NULL;
> +
>         result[length] = '\0';
>
>         for (i = 0; i < token_count; i++) {
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 10/28] fs/squashfs: sqfs_concat_tokens: check if malloc succeeds

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:08PM +0100, Richard Genoud wrote:

> memory allocation should always be checked
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/59ba4d16/attachment.sig>

[PATCH v2 11/28] fs/squashfs: sqfs_size: fix dangling pointer dirs->entry

[Richard Genoud]

dirs->entry shouldn't be left dangling as it could be freed twice.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 7da2e09cc36..3b008b5235c 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1569,6 +1569,7 @@ int sqfs_size(const char *filename, loff_t *size)
 		if (!ret)
 			break;
 		free(dirs->entry);
+		dirs->entry = NULL;
 	}
 
 	if (ret) {
@@ -1582,6 +1583,7 @@ int sqfs_size(const char *filename, loff_t *size)
 	ipos = sqfs_find_inode(dirs->inode_table, i_number, sblk->inodes,
 			       sblk->block_size);
 	free(dirs->entry);
+	dirs->entry = NULL;
 
 	base = (struct squashfs_base_inode *)ipos;
 	switch (get_unaligned_le16(&base->inode_type)) {

[PATCH v2 11/28] fs/squashfs: sqfs_size: fix dangling pointer dirs->entry

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:09PM +0100, Richard Genoud wrote:

> dirs->entry shouldn't be left dangling as it could be freed twice.
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/d3d882e4/attachment.sig>

[PATCH v2 12/28] fs/squashfs: sqfs_size: remove useless sqfs_closedir()

[Richard Genoud]

as sqfs_opendir failed, there's no need to call sqfs_closedir

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 3b008b5235c..f4cac3e4bf0 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1557,7 +1557,6 @@ int sqfs_size(const char *filename, loff_t *size)
 	 */
 	ret = sqfs_opendir(dir, &dirsp);
 	if (ret) {
-		sqfs_closedir(dirsp);
 		ret = -EINVAL;
 		goto free_strings;
 	}

[PATCH v2 12/28] fs/squashfs: sqfs_size: remove useless sqfs_closedir()

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:10PM +0100, Richard Genoud wrote:

> as sqfs_opendir failed, there's no need to call sqfs_closedir
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/1a61e138/attachment.sig>

[PATCH v2 13/28] fs/squashfs: sqfs_read: fix dangling pointer dirs->entry

[Richard Genoud]

dirs->entry shouldn't be left dangling as it could be freed twice.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index f4cac3e4bf0..13e64bea934 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1336,6 +1336,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 			break;
 
 		free(dirs->entry);
+		dirs->entry = NULL;
 	}
 
 	if (ret) {

[PATCH v2 13/28] fs/squashfs: sqfs_read: fix dangling pointer dirs->entry

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:11PM +0100, Richard Genoud wrote:

> dirs->entry shouldn't be left dangling as it could be freed twice.
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/0d29c09e/attachment.sig>

[PATCH v2 14/28] fs/squashfs: sqfs_read: remove useless sqfs_closedir()

[Richard Genoud]

as sqfs_opendir failed, there's no need to call sqfs_closedir

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 13e64bea934..1ac07625889 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1323,7 +1323,6 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 	sqfs_split_path(&file, &dir, filename);
 	ret = sqfs_opendir(dir, &dirsp);
 	if (ret) {
-		sqfs_closedir(dirsp);
 		goto free_paths;
 	}
 

[PATCH v2 14/28] fs/squashfs: sqfs_read: remove useless sqfs_closedir()

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:12PM +0100, Richard Genoud wrote:

> as sqfs_opendir failed, there's no need to call sqfs_closedir
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/34884c5a/attachment.sig>

[PATCH v2 15/28] fs/squashfs: sqfs_read: fix memory leak

[Richard Genoud]

sqfs_closedir() should be called to free memory allocated by
sqfs_opendir()

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 1ac07625889..a9e803cbac2 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1341,7 +1341,6 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 	if (ret) {
 		printf("File not found.\n");
 		*actread = 0;
-		sqfs_closedir(dirsp);
 		ret = -ENOENT;
 		goto free_paths;
 	}
@@ -1532,6 +1531,7 @@ free_datablk:
 free_paths:
 	free(file);
 	free(dir);
+	sqfs_closedir(dirsp);
 
 	return ret;
 }

[PATCH v2 15/28] fs/squashfs: sqfs_read: fix memory leak

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> sqfs_closedir() should be called to free memory allocated by
> sqfs_opendir()
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index 1ac07625889..a9e803cbac2 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -1341,7 +1341,6 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>         if (ret) {
>                 printf("File not found.\n");
>                 *actread = 0;
> -               sqfs_closedir(dirsp);
>                 ret = -ENOENT;
>                 goto free_paths;
>         }
> @@ -1532,6 +1531,7 @@ free_datablk:
>  free_paths:
>         free(file);
>         free(dir);
> +       sqfs_closedir(dirsp);
>
>         return ret;
>  }
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 15/28] fs/squashfs: sqfs_read: fix memory leak

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:13PM +0100, Richard Genoud wrote:

> sqfs_closedir() should be called to free memory allocated by
> sqfs_opendir()
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/0e2c09c2/attachment.sig>

[PATCH v2 16/28] fs/squashfs: sqfs_read: fix another memory leak

[Richard Genoud]

data_buffer was allocated in a loop and freed only once.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index a9e803cbac2..cfea313e34b 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1459,6 +1459,8 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 		}
 
 		data_offset += table_size;
+		free(data_buffer);
+		data_buffer = NULL;
 	}
 
 	free(finfo.blk_sizes);

[PATCH v2 16/28] fs/squashfs: sqfs_read: fix another memory leak

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:14PM +0100, Richard Genoud wrote:

> data_buffer was allocated in a loop and freed only once.
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/a1bc25ce/attachment.sig>

[PATCH v2 17/28] fs/squashfs: sqfs_frag_lookup: simplify error handling

[Richard Genoud]

For consistency with other functions.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index cfea313e34b..b97a961c5e3 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -106,6 +106,10 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 	int block, offset, ret;
 	u16 header;
 
+	metadata_buffer = NULL;
+	entries = NULL;
+	table = NULL;
+
 	if (inode_fragment_index >= get_unaligned_le32(&sblk->fragments))
 		return -EINVAL;
 
@@ -117,12 +121,14 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 
 	/* Allocate a proper sized buffer to store the fragment index table */
 	table = malloc_cache_aligned(n_blks * ctxt.cur_dev->blksz);
-	if (!table)
-		return -ENOMEM;
+	if (!table) {
+		ret = -ENOMEM;
+		goto out;
+	}
 
 	if (sqfs_disk_read(start, n_blks, table) < 0) {
-		free(table);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	block = SQFS_FRAGMENT_INDEX(inode_fragment_index);
@@ -142,12 +148,12 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 	metadata_buffer = malloc_cache_aligned(n_blks * ctxt.cur_dev->blksz);
 	if (!metadata_buffer) {
 		ret = -ENOMEM;
-		goto free_table;
+		goto out;
 	}
 
 	if (sqfs_disk_read(start, n_blks, metadata_buffer) < 0) {
 		ret = -EINVAL;
-		goto free_buffer;
+		goto out;
 	}
 
 	/* Every metadata block starts with a 16-bit header */
@@ -156,13 +162,13 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 
 	if (!metadata || !header) {
 		ret = -ENOMEM;
-		goto free_buffer;
+		goto out;
 	}
 
 	entries = malloc(SQFS_METADATA_BLOCK_SIZE);
 	if (!entries) {
 		ret = -ENOMEM;
-		goto free_buffer;
+		goto out;
 	}
 
 	if (SQFS_COMPRESSED_METADATA(header)) {
@@ -172,7 +178,7 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 				      src_len);
 		if (ret) {
 			ret = -EINVAL;
-			goto free_entries;
+			goto out;
 		}
 	} else {
 		memcpy(entries, metadata, SQFS_METADATA_SIZE(header));
@@ -181,11 +187,9 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 	*e = entries[offset];
 	ret = SQFS_COMPRESSED_BLOCK(e->size);
 
-free_entries:
+out:
 	free(entries);
-free_buffer:
 	free(metadata_buffer);
-free_table:
 	free(table);
 
 	return ret;

[PATCH v2 17/28] fs/squashfs: sqfs_frag_lookup: simplify error handling

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> For consistency with other functions.
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index cfea313e34b..b97a961c5e3 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -106,6 +106,10 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
>         int block, offset, ret;
>         u16 header;
>
> +       metadata_buffer = NULL;
> +       entries = NULL;
> +       table = NULL;
> +
>         if (inode_fragment_index >= get_unaligned_le32(&sblk->fragments))
>                 return -EINVAL;
>
> @@ -117,12 +121,14 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
>
>         /* Allocate a proper sized buffer to store the fragment index
> table */
>         table = malloc_cache_aligned(n_blks * ctxt.cur_dev->blksz);
> -       if (!table)
> -               return -ENOMEM;
> +       if (!table) {
> +               ret = -ENOMEM;
> +               goto out;
> +       }
>
>         if (sqfs_disk_read(start, n_blks, table) < 0) {
> -               free(table);
> -               return -EINVAL;
> +               ret = -EINVAL;
> +               goto out;
>         }
>
>         block = SQFS_FRAGMENT_INDEX(inode_fragment_index);
> @@ -142,12 +148,12 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
>         metadata_buffer = malloc_cache_aligned(n_blks *
> ctxt.cur_dev->blksz);
>         if (!metadata_buffer) {
>                 ret = -ENOMEM;
> -               goto free_table;
> +               goto out;
>         }
>
>         if (sqfs_disk_read(start, n_blks, metadata_buffer) < 0) {
>                 ret = -EINVAL;
> -               goto free_buffer;
> +               goto out;
>         }
>
>         /* Every metadata block starts with a 16-bit header */
> @@ -156,13 +162,13 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
>
>         if (!metadata || !header) {
>                 ret = -ENOMEM;
> -               goto free_buffer;
> +               goto out;
>         }
>
>         entries = malloc(SQFS_METADATA_BLOCK_SIZE);
>         if (!entries) {
>                 ret = -ENOMEM;
> -               goto free_buffer;
> +               goto out;
>         }
>
>         if (SQFS_COMPRESSED_METADATA(header)) {
> @@ -172,7 +178,7 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
>                                       src_len);
>                 if (ret) {
>                         ret = -EINVAL;
> -                       goto free_entries;
> +                       goto out;
>                 }
>         } else {
>                 memcpy(entries, metadata, SQFS_METADATA_SIZE(header));
> @@ -181,11 +187,9 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
>         *e = entries[offset];
>         ret = SQFS_COMPRESSED_BLOCK(e->size);
>
> -free_entries:
> +out:
>         free(entries);
> -free_buffer:
>         free(metadata_buffer);
> -free_table:
>         free(table);
>
>         return ret;
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 17/28] fs/squashfs: sqfs_frag_lookup: simplify error handling

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:15PM +0100, Richard Genoud wrote:

> For consistency with other functions.
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/4f4194d7/attachment.sig>

[PATCH v2 18/28] fs/squashfs: sqfs_get_abs_path: fix error check

[Richard Genoud]

the return value of sqfs_tokenize(rel_tokens, rc, rel); wasn't checked.
(but "ret" value was !)
This is obviouly a typo.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index b97a961c5e3..825d5d13fa2 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -359,7 +359,7 @@ static char *sqfs_get_abs_path(const char *base, const char *rel)
 	if (ret)
 		goto free_r_tokens;
 
-	sqfs_tokenize(rel_tokens, rc, rel);
+	ret = sqfs_tokenize(rel_tokens, rc, rel);
 	if (ret)
 		goto free_r_tokens;
 

[PATCH v2 18/28] fs/squashfs: sqfs_get_abs_path: fix error check

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> the return value of sqfs_tokenize(rel_tokens, rc, rel); wasn't checked.
> (but "ret" value was !)
> This is obviouly a typo.
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index b97a961c5e3..825d5d13fa2 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -359,7 +359,7 @@ static char *sqfs_get_abs_path(const char *base, const
> char *rel)
>         if (ret)
>                 goto free_r_tokens;
>
> -       sqfs_tokenize(rel_tokens, rc, rel);
> +       ret = sqfs_tokenize(rel_tokens, rc, rel);
>         if (ret)
>                 goto free_r_tokens;
>
>

-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 18/28] fs/squashfs: sqfs_get_abs_path: fix error check

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:16PM +0100, Richard Genoud wrote:

> the return value of sqfs_tokenize(rel_tokens, rc, rel); wasn't checked.
> (but "ret" value was !)
> This is obviouly a typo.
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/27aad6ca/attachment.sig>

[PATCH v2 19/28] fs/squashfs: sqfs_get_abs_path: fix possible memory leak on error

[Richard Genoud]

if  sqfs_tokenize(rel_tokens, rc, rel); fails, the function exits
without freeing the array base_tokens.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 825d5d13fa2..f41deece0ae 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -340,28 +340,31 @@ static char *sqfs_get_abs_path(const char *base, const char *rel)
 	char **base_tokens, **rel_tokens, *resolved = NULL;
 	int ret, bc, rc, i, updir = 0, resolved_size = 0, offset = 0;
 
+	base_tokens = NULL;
+	rel_tokens = NULL;
+
 	/* Memory allocation for the token lists */
 	bc = sqfs_count_tokens(base);
 	rc = sqfs_count_tokens(rel);
 	if (bc < 1 || rc < 1)
 		return NULL;
 
-	base_tokens = malloc(bc * sizeof(char *));
+	base_tokens = calloc(bc, sizeof(char *));
 	if (!base_tokens)
 		return NULL;
 
-	rel_tokens = malloc(rc * sizeof(char *));
+	rel_tokens = calloc(rc, sizeof(char *));
 	if (!rel_tokens)
-		goto free_b_tokens;
+		goto out;
 
 	/* Fill token lists */
 	ret = sqfs_tokenize(base_tokens, bc, base);
 	if (ret)
-		goto free_r_tokens;
+		goto out;
 
 	ret = sqfs_tokenize(rel_tokens, rc, rel);
 	if (ret)
-		goto free_r_tokens;
+		goto out;
 
 	/* count '..' occurrences in target path */
 	for (i = 0; i < rc; i++) {
@@ -372,7 +375,7 @@ static char *sqfs_get_abs_path(const char *base, const char *rel)
 	/* Remove the last token and the '..' occurrences */
 	bc = sqfs_clean_base_path(base_tokens, bc, updir);
 	if (bc < 0)
-		goto free_r_tokens;
+		goto out;
 
 	/* Calculate resolved path size */
 	if (!bc)
@@ -383,7 +386,7 @@ static char *sqfs_get_abs_path(const char *base, const char *rel)
 
 	resolved = malloc(resolved_size + 1);
 	if (!resolved)
-		goto free_r_tokens_loop;
+		goto out;
 
 	/* Set resolved path */
 	memset(resolved, '\0', resolved_size + 1);
@@ -391,14 +394,15 @@ static char *sqfs_get_abs_path(const char *base, const char *rel)
 	resolved[offset++] = '/';
 	offset += sqfs_join(rel_tokens, resolved + offset, updir, rc, '/');
 
-free_r_tokens_loop:
-	for (i = 0; i < rc; i++)
-		free(rel_tokens[i]);
-	for (i = 0; i < bc; i++)
-		free(base_tokens[i]);
-free_r_tokens:
+out:
+	if (rel_tokens)
+		for (i = 0; i < rc; i++)
+			free(rel_tokens[i]);
+	if (base_tokens)
+		for (i = 0; i < bc; i++)
+			free(base_tokens[i]);
+
 	free(rel_tokens);
-free_b_tokens:
 	free(base_tokens);
 
 	return resolved;

[PATCH v2 19/28] fs/squashfs: sqfs_get_abs_path: fix possible memory leak on error

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> if  sqfs_tokenize(rel_tokens, rc, rel); fails, the function exits
> without freeing the array base_tokens.
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index 825d5d13fa2..f41deece0ae 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -340,28 +340,31 @@ static char *sqfs_get_abs_path(const char *base,
> const char *rel)
>         char **base_tokens, **rel_tokens, *resolved = NULL;
>         int ret, bc, rc, i, updir = 0, resolved_size = 0, offset = 0;
>
> +       base_tokens = NULL;
> +       rel_tokens = NULL;
> +
>         /* Memory allocation for the token lists */
>         bc = sqfs_count_tokens(base);
>         rc = sqfs_count_tokens(rel);
>         if (bc < 1 || rc < 1)
>                 return NULL;
>
> -       base_tokens = malloc(bc * sizeof(char *));
> +       base_tokens = calloc(bc, sizeof(char *));
>         if (!base_tokens)
>                 return NULL;
>
> -       rel_tokens = malloc(rc * sizeof(char *));
> +       rel_tokens = calloc(rc, sizeof(char *));
>         if (!rel_tokens)
> -               goto free_b_tokens;
> +               goto out;
>
>         /* Fill token lists */
>         ret = sqfs_tokenize(base_tokens, bc, base);
>         if (ret)
> -               goto free_r_tokens;
> +               goto out;
>
>         ret = sqfs_tokenize(rel_tokens, rc, rel);
>         if (ret)
> -               goto free_r_tokens;
> +               goto out;
>
>         /* count '..' occurrences in target path */
>         for (i = 0; i < rc; i++) {
> @@ -372,7 +375,7 @@ static char *sqfs_get_abs_path(const char *base, const
> char *rel)
>         /* Remove the last token and the '..' occurrences */
>         bc = sqfs_clean_base_path(base_tokens, bc, updir);
>         if (bc < 0)
> -               goto free_r_tokens;
> +               goto out;
>
>         /* Calculate resolved path size */
>         if (!bc)
> @@ -383,7 +386,7 @@ static char *sqfs_get_abs_path(const char *base, const
> char *rel)
>
>         resolved = malloc(resolved_size + 1);
>         if (!resolved)
> -               goto free_r_tokens_loop;
> +               goto out;
>
>         /* Set resolved path */
>         memset(resolved, '\0', resolved_size + 1);
> @@ -391,14 +394,15 @@ static char *sqfs_get_abs_path(const char *base,
> const char *rel)
>         resolved[offset++] = '/';
>         offset += sqfs_join(rel_tokens, resolved + offset, updir, rc, '/');
>
> -free_r_tokens_loop:
> -       for (i = 0; i < rc; i++)
> -               free(rel_tokens[i]);
> -       for (i = 0; i < bc; i++)
> -               free(base_tokens[i]);
> -free_r_tokens:
> +out:
> +       if (rel_tokens)
> +               for (i = 0; i < rc; i++)
> +                       free(rel_tokens[i]);
> +       if (base_tokens)
> +               for (i = 0; i < bc; i++)
> +                       free(base_tokens[i]);
> +
>         free(rel_tokens);
> -free_b_tokens:
>         free(base_tokens);
>
>         return resolved;
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 19/28] fs/squashfs: sqfs_get_abs_path: fix possible memory leak on error

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:17PM +0100, Richard Genoud wrote:

> if  sqfs_tokenize(rel_tokens, rc, rel); fails, the function exits
> without freeing the array base_tokens.
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/8f322bed/attachment.sig>

[PATCH v2 20/28] fs/squashfs: sqfs_read: fix memory leak on finfo.blk_sizes

[Richard Genoud]

finfo.blk_sizes may not be freed in case of error in the for loop
Setting it to null and freeing it at the end makes prevents that from
happening.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 48 +++++++++++++++++++++-------------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index f41deece0ae..d8d4584fbfd 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1305,8 +1305,8 @@ static int sqfs_get_lregfile_info(struct squashfs_lreg_inode *lreg,
 int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 	      loff_t *actread)
 {
-	char *dir, *fragment_block, *datablock = NULL, *data_buffer = NULL;
-	char *fragment, *file, *resolved, *data;
+	char *dir = NULL, *fragment_block, *datablock = NULL, *data_buffer = NULL;
+	char *fragment = NULL, *file = NULL, *resolved, *data;
 	u64 start, n_blks, table_size, data_offset, table_offset;
 	int ret, j, i_number, datablk_count = 0;
 	struct squashfs_super_block *sblk = ctxt.sblk;
@@ -1331,7 +1331,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 	sqfs_split_path(&file, &dir, filename);
 	ret = sqfs_opendir(dir, &dirsp);
 	if (ret) {
-		goto free_paths;
+		goto out;
 	}
 
 	dirs = (struct squashfs_dir_stream *)dirsp;
@@ -1350,7 +1350,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 		printf("File not found.\n");
 		*actread = 0;
 		ret = -ENOENT;
-		goto free_paths;
+		goto out;
 	}
 
 	i_number = dirs->dir_header->inode_number + dirs->entry->inode_offset;
@@ -1365,7 +1365,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 						      sblk->block_size);
 		if (datablk_count < 0) {
 			ret = -EINVAL;
-			goto free_paths;
+			goto out;
 		}
 
 		memcpy(finfo.blk_sizes, ipos + sizeof(*reg),
@@ -1378,7 +1378,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 						       sblk->block_size);
 		if (datablk_count < 0) {
 			ret = -EINVAL;
-			goto free_paths;
+			goto out;
 		}
 
 		memcpy(finfo.blk_sizes, ipos + sizeof(*lreg),
@@ -1390,7 +1390,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 		resolved = sqfs_resolve_symlink(symlink, filename);
 		ret = sqfs_read(resolved, buf, offset, len, actread);
 		free(resolved);
-		goto free_paths;
+		goto out;
 	case SQFS_BLKDEV_TYPE:
 	case SQFS_CHRDEV_TYPE:
 	case SQFS_LBLKDEV_TYPE:
@@ -1402,14 +1402,14 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 	default:
 		printf("Unsupported entry type\n");
 		ret = -EINVAL;
-		goto free_paths;
+		goto out;
 	}
 
 	/* If the user specifies a length, check its sanity */
 	if (len) {
 		if (len > finfo.size) {
 			ret = -EINVAL;
-			goto free_paths;
+			goto out;
 		}
 
 		finfo.size = len;
@@ -1420,7 +1420,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 		datablock = malloc(get_unaligned_le32(&sblk->block_size));
 		if (!datablock) {
 			ret = -ENOMEM;
-			goto free_paths;
+			goto out;
 		}
 	}
 
@@ -1435,7 +1435,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 
 		if (!data_buffer) {
 			ret = -ENOMEM;
-			goto free_datablk;
+			goto out;
 		}
 
 		ret = sqfs_disk_read(start, n_blks, data_buffer);
@@ -1446,7 +1446,7 @@ 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;
+			goto out;
 		}
 
 		data = data_buffer + table_offset;
@@ -1457,7 +1457,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 			ret = sqfs_decompress(&ctxt, datablock, &dest_len,
 					      data, table_size);
 			if (ret)
-				goto free_buffer;
+				goto out;
 
 			memcpy(buf + offset + *actread, datablock, dest_len);
 			*actread += dest_len;
@@ -1471,14 +1471,12 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 		data_buffer = NULL;
 	}
 
-	free(finfo.blk_sizes);
-
 	/*
 	 * There is no need to continue if the file is not fragmented.
 	 */
 	if (!finfo.frag) {
 		ret = 0;
-		goto free_buffer;
+		goto out;
 	}
 
 	start = frag_entry.start / ctxt.cur_dev->blksz;
@@ -1490,12 +1488,12 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 
 	if (!fragment) {
 		ret = -ENOMEM;
-		goto free_buffer;
+		goto out;
 	}
 
 	ret = sqfs_disk_read(start, n_blks, fragment);
 	if (ret < 0)
-		goto free_fragment;
+		goto out;
 
 	/* File compressed and fragmented */
 	if (finfo.frag && finfo.comp) {
@@ -1503,7 +1501,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 		fragment_block = malloc(dest_len);
 		if (!fragment_block) {
 			ret = -ENOMEM;
-			goto free_fragment;
+			goto out;
 		}
 
 		ret = sqfs_decompress(&ctxt, fragment_block, &dest_len,
@@ -1511,7 +1509,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 				      frag_entry.size);
 		if (ret) {
 			free(fragment_block);
-			goto free_fragment;
+			goto out;
 		}
 
 		for (j = offset + *actread; j < finfo.size; j++) {
@@ -1530,17 +1528,15 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 		}
 	}
 
-free_fragment:
+out:
 	free(fragment);
-free_buffer:
-	if (datablk_count)
+	if (datablk_count) {
 		free(data_buffer);
-free_datablk:
-	if (datablk_count)
 		free(datablock);
-free_paths:
+	}
 	free(file);
 	free(dir);
+	free(finfo.blk_sizes);
 	sqfs_closedir(dirsp);
 
 	return ret;

[PATCH v2 20/28] fs/squashfs: sqfs_read: fix memory leak on finfo.blk_sizes

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> finfo.blk_sizes may not be freed in case of error in the for loop
> Setting it to null and freeing it at the end makes prevents that from
> happening.
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 48 +++++++++++++++++++++-------------------------
>  1 file changed, 22 insertions(+), 26 deletions(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index f41deece0ae..d8d4584fbfd 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -1305,8 +1305,8 @@ static int sqfs_get_lregfile_info(struct
> squashfs_lreg_inode *lreg,
>  int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
>               loff_t *actread)
>  {
> -       char *dir, *fragment_block, *datablock = NULL, *data_buffer = NULL;
> -       char *fragment, *file, *resolved, *data;
> +       char *dir = NULL, *fragment_block, *datablock = NULL, *data_buffer
> = NULL;
> +       char *fragment = NULL, *file = NULL, *resolved, *data;
>         u64 start, n_blks, table_size, data_offset, table_offset;
>         int ret, j, i_number, datablk_count = 0;
>         struct squashfs_super_block *sblk = ctxt.sblk;
> @@ -1331,7 +1331,7 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>         sqfs_split_path(&file, &dir, filename);
>         ret = sqfs_opendir(dir, &dirsp);
>         if (ret) {
> -               goto free_paths;
> +               goto out;
>         }
>
>         dirs = (struct squashfs_dir_stream *)dirsp;
> @@ -1350,7 +1350,7 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>                 printf("File not found.\n");
>                 *actread = 0;
>                 ret = -ENOENT;
> -               goto free_paths;
> +               goto out;
>         }
>
>         i_number = dirs->dir_header->inode_number +
> dirs->entry->inode_offset;
> @@ -1365,7 +1365,7 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>                                                       sblk->block_size);
>                 if (datablk_count < 0) {
>                         ret = -EINVAL;
> -                       goto free_paths;
> +                       goto out;
>                 }
>
>                 memcpy(finfo.blk_sizes, ipos + sizeof(*reg),
> @@ -1378,7 +1378,7 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>                                                        sblk->block_size);
>                 if (datablk_count < 0) {
>                         ret = -EINVAL;
> -                       goto free_paths;
> +                       goto out;
>                 }
>
>                 memcpy(finfo.blk_sizes, ipos + sizeof(*lreg),
> @@ -1390,7 +1390,7 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>                 resolved = sqfs_resolve_symlink(symlink, filename);
>                 ret = sqfs_read(resolved, buf, offset, len, actread);
>                 free(resolved);
> -               goto free_paths;
> +               goto out;
>         case SQFS_BLKDEV_TYPE:
>         case SQFS_CHRDEV_TYPE:
>         case SQFS_LBLKDEV_TYPE:
> @@ -1402,14 +1402,14 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>         default:
>                 printf("Unsupported entry type\n");
>                 ret = -EINVAL;
> -               goto free_paths;
> +               goto out;
>         }
>
>         /* If the user specifies a length, check its sanity */
>         if (len) {
>                 if (len > finfo.size) {
>                         ret = -EINVAL;
> -                       goto free_paths;
> +                       goto out;
>                 }
>
>                 finfo.size = len;
> @@ -1420,7 +1420,7 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>                 datablock = malloc(get_unaligned_le32(&sblk->block_size));
>                 if (!datablock) {
>                         ret = -ENOMEM;
> -                       goto free_paths;
> +                       goto out;
>                 }
>         }
>
> @@ -1435,7 +1435,7 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>
>                 if (!data_buffer) {
>                         ret = -ENOMEM;
> -                       goto free_datablk;
> +                       goto out;
>                 }
>
>                 ret = sqfs_disk_read(start, n_blks, data_buffer);
> @@ -1446,7 +1446,7 @@ 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;
> +                       goto out;
>                 }
>
>                 data = data_buffer + table_offset;
> @@ -1457,7 +1457,7 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>                         ret = sqfs_decompress(&ctxt, datablock, &dest_len,
>                                               data, table_size);
>                         if (ret)
> -                               goto free_buffer;
> +                               goto out;
>
>                         memcpy(buf + offset + *actread, datablock,
> dest_len);
>                         *actread += dest_len;
> @@ -1471,14 +1471,12 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>                 data_buffer = NULL;
>         }
>
> -       free(finfo.blk_sizes);
> -
>         /*
>          * There is no need to continue if the file is not fragmented.
>          */
>         if (!finfo.frag) {
>                 ret = 0;
> -               goto free_buffer;
> +               goto out;
>         }
>
>         start = frag_entry.start / ctxt.cur_dev->blksz;
> @@ -1490,12 +1488,12 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>
>         if (!fragment) {
>                 ret = -ENOMEM;
> -               goto free_buffer;
> +               goto out;
>         }
>
>         ret = sqfs_disk_read(start, n_blks, fragment);
>         if (ret < 0)
> -               goto free_fragment;
> +               goto out;
>
>         /* File compressed and fragmented */
>         if (finfo.frag && finfo.comp) {
> @@ -1503,7 +1501,7 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>                 fragment_block = malloc(dest_len);
>                 if (!fragment_block) {
>                         ret = -ENOMEM;
> -                       goto free_fragment;
> +                       goto out;
>                 }
>
>                 ret = sqfs_decompress(&ctxt, fragment_block, &dest_len,
> @@ -1511,7 +1509,7 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>                                       frag_entry.size);
>                 if (ret) {
>                         free(fragment_block);
> -                       goto free_fragment;
> +                       goto out;
>                 }
>
>                 for (j = offset + *actread; j < finfo.size; j++) {
> @@ -1530,17 +1528,15 @@ int sqfs_read(const char *filename, void *buf,
> loff_t offset, loff_t len,
>                 }
>         }
>
> -free_fragment:
> +out:
>         free(fragment);
> -free_buffer:
> -       if (datablk_count)
> +       if (datablk_count) {
>                 free(data_buffer);
> -free_datablk:
> -       if (datablk_count)
>                 free(datablock);
> -free_paths:
> +       }
>         free(file);
>         free(dir);
> +       free(finfo.blk_sizes);
>         sqfs_closedir(dirsp);
>
>         return ret;
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 20/28] fs/squashfs: sqfs_read: fix memory leak on finfo.blk_sizes

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:18PM +0100, Richard Genoud wrote:

> finfo.blk_sizes may not be freed in case of error in the for loop
> Setting it to null and freeing it at the end makes prevents that from
> happening.
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/9942b8e2/attachment.sig>

[PATCH v2 21/28] fs/squashfs: sqfs_probe: fix possible memory leak on error

[Richard Genoud]

If SquashFS magic number is invalid, there's a memory leak.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index d8d4584fbfd..7d6f0e88e31 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1090,8 +1090,8 @@ int sqfs_probe(struct blk_desc *fs_dev_desc, struct disk_partition *fs_partition
 	/* Make sure it has a valid SquashFS magic number*/
 	if (get_unaligned_le32(&sblk->s_magic) != SQFS_MAGIC_NUMBER) {
 		printf("Bad magic number for SquashFS image.\n");
-		ctxt.cur_dev = NULL;
-		return -EINVAL;
+		ret = -EINVAL;
+		goto error;
 	}
 
 	ctxt.sblk = sblk;
@@ -1099,12 +1099,16 @@ int sqfs_probe(struct blk_desc *fs_dev_desc, struct disk_partition *fs_partition
 	ret = sqfs_decompressor_init(&ctxt);
 
 	if (ret) {
-		ctxt.cur_dev = NULL;
-		free(ctxt.sblk);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto error;
 	}
 
 	return 0;
+error:
+	ctxt.cur_dev = NULL;
+	free(ctxt.sblk);
+	ctxt.sblk = NULL;
+	return ret;
 }
 
 static char *sqfs_basename(char *path)

[PATCH v2 21/28] fs/squashfs: sqfs_probe: fix possible memory leak on error

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> If SquashFS magic number is invalid, there's a memory leak.
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index d8d4584fbfd..7d6f0e88e31 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -1090,8 +1090,8 @@ int sqfs_probe(struct blk_desc *fs_dev_desc, struct
> disk_partition *fs_partition
>         /* Make sure it has a valid SquashFS magic number*/
>         if (get_unaligned_le32(&sblk->s_magic) != SQFS_MAGIC_NUMBER) {
>                 printf("Bad magic number for SquashFS image.\n");
> -               ctxt.cur_dev = NULL;
> -               return -EINVAL;
> +               ret = -EINVAL;
> +               goto error;
>         }
>
>         ctxt.sblk = sblk;
> @@ -1099,12 +1099,16 @@ int sqfs_probe(struct blk_desc *fs_dev_desc,
> struct disk_partition *fs_partition
>         ret = sqfs_decompressor_init(&ctxt);
>
>         if (ret) {
> -               ctxt.cur_dev = NULL;
> -               free(ctxt.sblk);
> -               return -EINVAL;
> +               ret = -EINVAL;
> +               goto error;
>         }
>
>         return 0;
> +error:
> +       ctxt.cur_dev = NULL;
> +       free(ctxt.sblk);
> +       ctxt.sblk = NULL;
> +       return ret;
>  }
>
>  static char *sqfs_basename(char *path)
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 21/28] fs/squashfs: sqfs_probe: fix possible memory leak on error

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:19PM +0100, Richard Genoud wrote:

> If SquashFS magic number is invalid, there's a memory leak.
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/b977edde/attachment.sig>

[PATCH v2 22/28] fs/squashfs: sqfs_close/sqfs_read_sblk: set ctxt.sblk to NULL after free

[Richard Genoud]

This will prevent a double free error if sqfs_close() is called twice.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 7d6f0e88e31..a46e19c75c5 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -49,6 +49,7 @@ static int sqfs_read_sblk(struct squashfs_super_block **sblk)
 
 	if (sqfs_disk_read(0, 1, *sblk) != 1) {
 		free(*sblk);
+		sblk = NULL;
 		return -EINVAL;
 	}
 
@@ -1638,6 +1639,7 @@ free_strings:
 void sqfs_close(void)
 {
 	free(ctxt.sblk);
+	ctxt.sblk = NULL;
 	ctxt.cur_dev = NULL;
 	sqfs_decompressor_cleanup(&ctxt);
 }

[PATCH v2 22/28] fs/squashfs: sqfs_close/sqfs_read_sblk: set ctxt.sblk to NULL after free

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:20PM +0100, Richard Genoud wrote:

> This will prevent a double free error if sqfs_close() is called twice.
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

This change causes the test.py squashfs tests to fail.  I am unsure if
the problem is with the tests or this exposing further problems in the
code.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/2958faf6/attachment.sig>

[PATCH v2 22/28] fs/squashfs: sqfs_close/sqfs_read_sblk: set ctxt.sblk to NULL after free

[Richard Genoud]

Le 20/11/2020 ? 02:35, Tom Rini a ?crit?:
> On Tue, Nov 03, 2020 at 12:11:20PM +0100, Richard Genoud wrote:
> 
>> This will prevent a double free error if sqfs_close() is called twice.
>>
>> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> 
> This change causes the test.py squashfs tests to fail.  I am unsure if
> the problem is with the tests or this exposing further problems in the
> code.
> 
I'll look into that.

Thanks !

[PATCH v2 23/28] fs/squashfs: sqfs_probe: reset cur_dev/cur_part_info to NULL on error

[Richard Genoud]

Resetting the context on error will prevent some checks like:
if (!ctx.cur_dev)
To pass when the probe method has failed

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index a46e19c75c5..9682ea52557 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1086,7 +1086,7 @@ int sqfs_probe(struct blk_desc *fs_dev_desc, struct disk_partition *fs_partition
 
 	ret = sqfs_read_sblk(&sblk);
 	if (ret)
-		return ret;
+		goto error;
 
 	/* Make sure it has a valid SquashFS magic number*/
 	if (get_unaligned_le32(&sblk->s_magic) != SQFS_MAGIC_NUMBER) {

[PATCH v2 23/28] fs/squashfs: sqfs_probe: reset cur_dev/cur_part_info to NULL on error

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:21PM +0100, Richard Genoud wrote:

> Resetting the context on error will prevent some checks like:
> if (!ctx.cur_dev)
> To pass when the probe method has failed
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/fb0dc921/attachment.sig>

[PATCH v2 24/28] fs/squashfs: sqfs_probe: use sqfs_decompressor_init() return value

[Richard Genoud]

sqfs_decompressor_init() returns a value, so it's better to use it than
to force the return value to EINVAL (it could be ENOMEM)

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 9682ea52557..fa3120aefd6 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1098,9 +1098,7 @@ int sqfs_probe(struct blk_desc *fs_dev_desc, struct disk_partition *fs_partition
 	ctxt.sblk = sblk;
 
 	ret = sqfs_decompressor_init(&ctxt);
-
 	if (ret) {
-		ret = -EINVAL;
 		goto error;
 	}
 

[PATCH v2 24/28] fs/squashfs: sqfs_probe: use sqfs_decompressor_init() return value

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> sqfs_decompressor_init() returns a value, so it's better to use it than
> to force the return value to EINVAL (it could be ENOMEM)
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/squashfs/sqfs.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index 9682ea52557..fa3120aefd6 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -1098,9 +1098,7 @@ int sqfs_probe(struct blk_desc *fs_dev_desc, struct
> disk_partition *fs_partition
>         ctxt.sblk = sblk;
>
>         ret = sqfs_decompressor_init(&ctxt);
> -
>         if (ret) {
> -               ret = -EINVAL;
>                 goto error;
>         }
>
>

-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 24/28] fs/squashfs: sqfs_probe: use sqfs_decompressor_init() return value

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:22PM +0100, Richard Genoud wrote:

> sqfs_decompressor_init() returns a value, so it's better to use it than
> to force the return value to EINVAL (it could be ENOMEM)
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/9828a6a2/attachment.sig>

[PATCH v2 25/28] fs/squashfs: sqfs_read: don't write beyond buffer size

[Richard Genoud]

The length of the buffer wasn't taken into account when writing to the
given buffer.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index fa3120aefd6..4710b2bd707 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1416,6 +1416,8 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 		}
 
 		finfo.size = len;
+	} else {
+		len = finfo.size;
 	}
 
 	if (datablk_count) {
@@ -1462,9 +1464,13 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 			if (ret)
 				goto out;
 
+			if ((*actread + dest_len) > len)
+				dest_len = len - *actread;
 			memcpy(buf + offset + *actread, datablock, dest_len);
 			*actread += dest_len;
 		} else {
+			if ((*actread + table_size) > len)
+				table_size = len - *actread;
 			memcpy(buf + offset + *actread, data, table_size);
 			*actread += table_size;
 		}
@@ -1472,6 +1478,8 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 		data_offset += table_size;
 		free(data_buffer);
 		data_buffer = NULL;
+		if (*actread >= len)
+			break;
 	}
 
 	/*

[PATCH v2 25/28] fs/squashfs: sqfs_read: don't write beyond buffer size

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:23PM +0100, Richard Genoud wrote:

> The length of the buffer wasn't taken into account when writing to the
> given buffer.
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/d5ccaf24/attachment.sig>

[PATCH v2 26/28] fs/squashfs: sqfs_read: remove buggy offset functionality

[Richard Genoud]

offset is the offset in the file read, not the offset in the destination
buffer.
If the offset is not null, this will lead to a memory corruption.
So, for now, we are returning an error if the offset is used.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 4710b2bd707..f63a06fd40f 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1327,6 +1327,14 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 
 	*actread = 0;
 
+	if (offset) {
+		/*
+		 * TODO: implement reading at an offset in file
+		 */
+		printf("Error: reading at a specific offset in a squashfs file is not supported yet.\n");
+		return -EINVAL;
+	}
+
 	/*
 	 * sqfs_opendir will uncompress inode and directory tables, and will
 	 * return a pointer to the directory that contains the requested file.
@@ -1466,12 +1474,12 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 
 			if ((*actread + dest_len) > len)
 				dest_len = len - *actread;
-			memcpy(buf + offset + *actread, datablock, dest_len);
+			memcpy(buf + *actread, datablock, dest_len);
 			*actread += dest_len;
 		} else {
 			if ((*actread + table_size) > len)
 				table_size = len - *actread;
-			memcpy(buf + offset + *actread, data, table_size);
+			memcpy(buf + *actread, data, table_size);
 			*actread += table_size;
 		}
 
@@ -1523,7 +1531,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 			goto out;
 		}
 
-		for (j = offset + *actread; j < finfo.size; j++) {
+		for (j = *actread; j < finfo.size; j++) {
 			memcpy(buf + j, &fragment_block[finfo.offset + j], 1);
 			(*actread)++;
 		}
@@ -1533,7 +1541,7 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 	} else if (finfo.frag && !finfo.comp) {
 		fragment_block = (void *)fragment + table_offset;
 
-		for (j = offset + *actread; j < finfo.size; j++) {
+		for (j = *actread; j < finfo.size; j++) {
 			memcpy(buf + j, &fragment_block[finfo.offset + j], 1);
 			(*actread)++;
 		}

[PATCH v2 26/28] fs/squashfs: sqfs_read: remove buggy offset functionality

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:24PM +0100, Richard Genoud wrote:

> offset is the offset in the file read, not the offset in the destination
> buffer.
> If the offset is not null, this will lead to a memory corruption.
> So, for now, we are returning an error if the offset is used.
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/2bf68295/attachment-0001.sig>

[PATCH v2 27/28] fs/squashfs: sqfs_read: fragmented files are not supported

[Richard Genoud]

The code for reading a fragmented file is not functionnal.
It's better to signal this to the user.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/squashfs/sqfs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index f63a06fd40f..a96c1d4f564 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1498,6 +1498,13 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 		goto out;
 	}
 
+	printf("Error: reading a fragmented file is not supported yet.\n");
+	ret = -EINVAL;
+	goto out;
+
+	/*
+	 * TODO: reading a fragmented file doesn't work
+	 */
 	start = frag_entry.start / ctxt.cur_dev->blksz;
 	table_size = SQFS_BLOCK_SIZE(frag_entry.size);
 	table_offset = frag_entry.start - (start * ctxt.cur_dev->blksz);

[PATCH v2 27/28] fs/squashfs: sqfs_read: fragmented files are not supported

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:25PM +0100, Richard Genoud wrote:

> The code for reading a fragmented file is not functionnal.
> It's better to signal this to the user.
> 
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

This change causes the test.py squashfs tests to fail.  I am unsure if
the problem is with the tests or this exposing further problems in the
code.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/64cba384/attachment.sig>

[PATCH v2 27/28] fs/squashfs: sqfs_read: fragmented files are not supported

[Richard Genoud]

Hi,

Le 20/11/2020 ? 02:35, Tom Rini a ?crit?:
> On Tue, Nov 03, 2020 at 12:11:25PM +0100, Richard Genoud wrote:
> 
>> The code for reading a fragmented file is not functionnal.
>> It's better to signal this to the user.
>>
>> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> 
> This change causes the test.py squashfs tests to fail.  I am unsure if
> the problem is with the tests or this exposing further problems in the
> code.
Actually, reading a fragmented file doesn't work.
The test only check if the file is read, but not it's content.

With this following patch, we'll see that the file content is not the same :


 From 68f87301c059aaae8e90e42fbec9b560aee0c6eb Mon Sep 17 00:00:00 2001
From: Richard Genoud <richard.genoud@posteo.net>
Date: Tue, 24 Nov 2020 17:45:07 +0100
Subject: [PATCH] test/py: SquashFS: Check if loaded file is corrupted

After loading the file in memory, its content should be checked for
errors.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
  test/py/tests/test_fs/test_squashfs/sqfs_common.py    | 5 ++++-
  test/py/tests/test_fs/test_squashfs/test_sqfs_load.py | 6 +++++-
  2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/test/py/tests/test_fs/test_squashfs/sqfs_common.py b/test/py/tests/test_fs/test_squashfs/sqfs_common.py
index c96f92c1d8f..a7673c73762 100644
--- a/test/py/tests/test_fs/test_squashfs/sqfs_common.py
+++ b/test/py/tests/test_fs/test_squashfs/sqfs_common.py
@@ -6,6 +6,7 @@ import os
  import random
  import string
  import subprocess
+import zlib

  def sqfs_get_random_letters(size):
      letters = []
@@ -19,12 +20,14 @@ def sqfs_generate_file(path, size):
      file = open(path, "w")
      file.write(content)
      file.close()
+    return zlib.crc32(content.encode())

  class Compression:
      def __init__(self, name, files, sizes, block_size = 4096):
          self.name = name
          self.files = files
          self.sizes = sizes
+        self.crc = []
          self.mksquashfs_opts = " -b " + str(block_size) + " -comp " + self.name

      def add_opt(self, opt):
@@ -34,7 +37,7 @@ class Compression:
          src = os.path.join(build_dir, "sqfs_src/")
          os.mkdir(src)
          for (f, s) in zip(self.files, self.sizes):
-            sqfs_generate_file(src + f, s)
+            self.crc.append(sqfs_generate_file(src + f, s))

          # the symbolic link always targets the first file
          os.symlink(self.files[0], src + "sym")
diff --git a/test/py/tests/test_fs/test_squashfs/test_sqfs_load.py b/test/py/tests/test_fs/test_squashfs/test_sqfs_load.py
index 9e900623846..2ab4660036e 100644
--- a/test/py/tests/test_fs/test_squashfs/test_sqfs_load.py
+++ b/test/py/tests/test_fs/test_squashfs/test_sqfs_load.py
@@ -4,6 +4,7 @@

  import os
  import pytest
+import zlib
  from sqfs_common import *

  @pytest.mark.boardspec('sandbox')
@@ -14,6 +15,7 @@ from sqfs_common import *
  def test_sqfs_load(u_boot_console):
      build_dir = u_boot_console.config.build_dir
      command = "sqfsload host 0 $kernel_addr_r "
+    sum_command = "crc32 -v $kernel_addr_r $filesize "

      for opt in comp_opts:
          # generate and load the squashfs image
@@ -30,10 +32,12 @@ def test_sqfs_load(u_boot_console):
          output = u_boot_console.run_command(command + "xxx")
          assert "File not found." in output

-        for (f, s) in zip(opt.files, opt.sizes):
+        for (f, s, c) in zip(opt.files, opt.sizes, opt.crc):
              try:
                  output = u_boot_console.run_command(command + f)
                  assert str(s) in output
+                output = u_boot_console.run_command(sum_command + format(c, '08x'))
+                assert not 'ERROR' in output
              except:
                  assert False
                  opt.cleanup(build_dir)

[PATCH v2 27/28] fs/squashfs: sqfs_read: fragmented files are not supported

[Simon Glass]

Hi Joao,

On Wed, 25 Nov 2020 at 01:58, Richard Genoud <richard.genoud@posteo.net> wrote:
>
> Hi,
>
> Le 20/11/2020 ? 02:35, Tom Rini a ?crit :
> > On Tue, Nov 03, 2020 at 12:11:25PM +0100, Richard Genoud wrote:
> >
> >> The code for reading a fragmented file is not functionnal.
> >> It's better to signal this to the user.
> >>
> >> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> >
> > This change causes the test.py squashfs tests to fail.  I am unsure if
> > the problem is with the tests or this exposing further problems in the
> > code.
> Actually, reading a fragmented file doesn't work.
> The test only check if the file is read, but not it's content.
>
> With this following patch, we'll see that the file content is not the same :
>
>
>  From 68f87301c059aaae8e90e42fbec9b560aee0c6eb Mon Sep 17 00:00:00 2001
> From: Richard Genoud <richard.genoud@posteo.net>
> Date: Tue, 24 Nov 2020 17:45:07 +0100
> Subject: [PATCH] test/py: SquashFS: Check if loaded file is corrupted
>
> After loading the file in memory, its content should be checked for
> errors.
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>   test/py/tests/test_fs/test_squashfs/sqfs_common.py    | 5 ++++-
>   test/py/tests/test_fs/test_squashfs/test_sqfs_load.py | 6 +++++-
>   2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/test/py/tests/test_fs/test_squashfs/sqfs_common.py b/test/py/tests/test_fs/test_squashfs/sqfs_common.py
> index c96f92c1d8f..a7673c73762 100644
> --- a/test/py/tests/test_fs/test_squashfs/sqfs_common.py
> +++ b/test/py/tests/test_fs/test_squashfs/sqfs_common.py

This test works the first time I run it but fails the second time,
since the directory already exists. This makes it necessary to disable
the test for development.

It also uses the wrong quoting style - we have settled on a single
quote by default in U-Boot.

Finally, the tests and some functions need comments about what they do
and what the arguments are.

Please can you take a look?

Thanks,
Simon

[PATCH v2 27/28] fs/squashfs: sqfs_read: fragmented files are not supported

[João Marcos Costa]

Em qua., 27 de jan. de 2021 ?s 12:15, Simon Glass <sjg@chromium.org>
escreveu:

> Hi Joao,

Hello!

>
>
> This test works the first time I run it but fails the second time,
> since the directory already exists. This makes it necessary to disable
> the test for development.
>
> It also uses the wrong quoting style - we have settled on a single
> quote by default in U-Boot.
>
> Finally, the tests and some functions need comments about what they do
> and what the arguments are.
>
> Please can you take a look?

Absolutely. Excuse me for such a late reply.

>


> Thanks,
> Simon
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 27/28] fs/squashfs: sqfs_read: fragmented files are not supported

[Simon Glass]

Hi,

On Thu, 4 Feb 2021 at 15:32, Jo?o Marcos Costa <jmcosta944@gmail.com> wrote:
>
>
>
> Em qua., 27 de jan. de 2021 ?s 12:15, Simon Glass <sjg@chromium.org> escreveu:
>>
>> Hi Joao,
>
> Hello!
>>
>>
>>
>> This test works the first time I run it but fails the second time,
>> since the directory already exists. This makes it necessary to disable
>> the test for development.
>>
>> It also uses the wrong quoting style - we have settled on a single
>> quote by default in U-Boot.
>>
>> Finally, the tests and some functions need comments about what they do
>> and what the arguments are.
>>
>> Please can you take a look?
>
> Absolutely. Excuse me for such a late reply.

Any word on this please? Have you been able to repeat this?

Regards,
Simon

[PATCH v2 27/28] fs/squashfs: sqfs_read: fragmented files are not supported

[Richard Genoud]

Hi all,

Le 08/05/2021 ? 23:51, Simon Glass a ?crit?:
> Hi,
> 
> On Thu, 4 Feb 2021 at 15:32, Jo?o Marcos Costa <jmcosta944@gmail.com> wrote:
>>
>> Em qua., 27 de jan. de 2021 ?s 12:15, Simon Glass <sjg@chromium.org> escreveu:
>>>
>>> Hi Joao,
>>
>> Hello!
>>>
>>> This test works the first time I run it but fails the second time,
>>> since the directory already exists. This makes it necessary to disable
>>> the test for development.
>>>
>>> It also uses the wrong quoting style - we have settled on a single
>>> quote by default in U-Boot.
>>>
>>> Finally, the tests and some functions need comments about what they do
>>> and what the arguments are.
>>>
>>> Please can you take a look?
>>
>> Absolutely. Excuse me for such a late reply.
> 
> Any word on this please? Have you been able to repeat this?
Yes, for me, reading fragmented files doesn't work.
The test "test_sqfs_load" is OK because it only tests the file length not its content.

I've written a patch to check if the file is corrupted or not, and it fails :
./test/py/test.py --bd sandbox --build -k test_sqfs_load -v
[...]
AssertionError: assert not 'ERROR' in 'crc32 for 01000000 ... 010013eb ==> df8e6fe2 != d1522690 ** ERROR **'
[...]

Here's the patch onto v2021.07-rc2 :
----------8<--------
Subject: [PATCH] test/py: SquashFS: Check if loaded file is corrupted

After loading the file in memory, its content should be checked for
errors.

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
  test/py/tests/test_fs/test_squashfs/sqfs_common.py    | 5 ++++-
  test/py/tests/test_fs/test_squashfs/test_sqfs_load.py | 6 +++++-
  2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/test/py/tests/test_fs/test_squashfs/sqfs_common.py b/test/py/tests/test_fs/test_squashfs/sqfs_common.py
index c96f92c1d8f..a7673c73762 100644
--- a/test/py/tests/test_fs/test_squashfs/sqfs_common.py
+++ b/test/py/tests/test_fs/test_squashfs/sqfs_common.py
@@ -6,6 +6,7 @@ import os
  import random
  import string
  import subprocess
+import zlib

  def sqfs_get_random_letters(size):
      letters = []
@@ -19,12 +20,14 @@ def sqfs_generate_file(path, size):
      file = open(path, "w")
      file.write(content)
      file.close()
+    return zlib.crc32(content.encode())

  class Compression:
      def __init__(self, name, files, sizes, block_size = 4096):
          self.name = name
          self.files = files
          self.sizes = sizes
+        self.crc = []
          self.mksquashfs_opts = " -b " + str(block_size) + " -comp " + self.name

      def add_opt(self, opt):
@@ -34,7 +37,7 @@ class Compression:
          src = os.path.join(build_dir, "sqfs_src/")
          os.mkdir(src)
          for (f, s) in zip(self.files, self.sizes):
-            sqfs_generate_file(src + f, s)
+            self.crc.append(sqfs_generate_file(src + f, s))

          # the symbolic link always targets the first file
          os.symlink(self.files[0], src + "sym")
diff --git a/test/py/tests/test_fs/test_squashfs/test_sqfs_load.py b/test/py/tests/test_fs/test_squashfs/test_sqfs_load.py
index 9e900623846..2ab4660036e 100644
--- a/test/py/tests/test_fs/test_squashfs/test_sqfs_load.py
+++ b/test/py/tests/test_fs/test_squashfs/test_sqfs_load.py
@@ -4,6 +4,7 @@

  import os
  import pytest
+import zlib
  from sqfs_common import *

  @pytest.mark.boardspec('sandbox')
@@ -14,6 +15,7 @@ from sqfs_common import *
  def test_sqfs_load(u_boot_console):
      build_dir = u_boot_console.config.build_dir
      command = "sqfsload host 0 $kernel_addr_r "
+    sum_command = "crc32 -v $kernel_addr_r $filesize "

      for opt in comp_opts:
          # generate and load the squashfs image
@@ -30,10 +32,12 @@ def test_sqfs_load(u_boot_console):
          output = u_boot_console.run_command(command + "xxx")
          assert "File not found." in output

-        for (f, s) in zip(opt.files, opt.sizes):
+        for (f, s, c) in zip(opt.files, opt.sizes, opt.crc):
              try:
                  output = u_boot_console.run_command(command + f)
                  assert str(s) in output
+                output = u_boot_console.run_command(sum_command + format(c, '08x'))
+                assert not 'ERROR' in output
              except:
                  assert False
                  opt.cleanup(build_dir)
-- 
2.20.1

[PATCH v2 27/28] fs/squashfs: sqfs_read: fragmented files are not supported

[João Marcos Costa]

Hello,

Em ter., 11 de mai. de 2021 ?s 10:04, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> Hi all,
>
> Le 08/05/2021 ? 23:51, Simon Glass a ?crit :
> > Hi,
> >
> > On Thu, 4 Feb 2021 at 15:32, Jo?o Marcos Costa <jmcosta944@gmail.com>
> wrote:
> >>
> >> Em qua., 27 de jan. de 2021 ?s 12:15, Simon Glass <sjg@chromium.org>
> escreveu:
> >>>
> >>> Hi Joao,
> >>
> >> Hello!
> >>>
> >>> This test works the first time I run it but fails the second time,
> >>> since the directory already exists. This makes it necessary to disable
> >>> the test for development.
> >>>
> >>> It also uses the wrong quoting style - we have settled on a single
> >>> quote by default in U-Boot.
> >>>
> >>> Finally, the tests and some functions need comments about what they do
> >>> and what the arguments are.
> >>>
> >>> Please can you take a look?
> >>
> >> Absolutely. Excuse me for such a late reply.
> >
> > Any word on this please? Have you been able to repeat this?
> Yes, for me, reading fragmented files doesn't work.
> The test "test_sqfs_load" is OK because it only tests the file length not
> its content.
>
> I've written a patch to check if the file is corrupted or not, and it
> fails :
> ./test/py/test.py --bd sandbox --build -k test_sqfs_load -v
> [...]
>

I finally could get back to SquashFS support today, and I fixed a few bugs
concerning the fragmented files. However, I still need to run a few more
tests before submitting the patches.

Best regards,
Joao Marcos

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 27/28] fs/squashfs: sqfs_read: fragmented files are not supported

[Tom Rini]

On Sun, May 16, 2021 at 09:44:53PM -0300, Jo?o Marcos Costa wrote:
> Hello,
> 
> Em ter., 11 de mai. de 2021 ?s 10:04, Richard Genoud <
> richard.genoud at posteo.net> escreveu:
> 
> > Hi all,
> >
> > Le 08/05/2021 ? 23:51, Simon Glass a ?crit :
> > > Hi,
> > >
> > > On Thu, 4 Feb 2021 at 15:32, Jo?o Marcos Costa <jmcosta944@gmail.com>
> > wrote:
> > >>
> > >> Em qua., 27 de jan. de 2021 ?s 12:15, Simon Glass <sjg@chromium.org>
> > escreveu:
> > >>>
> > >>> Hi Joao,
> > >>
> > >> Hello!
> > >>>
> > >>> This test works the first time I run it but fails the second time,
> > >>> since the directory already exists. This makes it necessary to disable
> > >>> the test for development.
> > >>>
> > >>> It also uses the wrong quoting style - we have settled on a single
> > >>> quote by default in U-Boot.
> > >>>
> > >>> Finally, the tests and some functions need comments about what they do
> > >>> and what the arguments are.
> > >>>
> > >>> Please can you take a look?
> > >>
> > >> Absolutely. Excuse me for such a late reply.
> > >
> > > Any word on this please? Have you been able to repeat this?
> > Yes, for me, reading fragmented files doesn't work.
> > The test "test_sqfs_load" is OK because it only tests the file length not
> > its content.
> >
> > I've written a patch to check if the file is corrupted or not, and it
> > fails :
> > ./test/py/test.py --bd sandbox --build -k test_sqfs_load -v
> > [...]
> >
> 
> I finally could get back to SquashFS support today, and I fixed a few bugs
> concerning the fragmented files. However, I still need to run a few more
> tests before submitting the patches.

Please make sure the problem with repeatedly running the tests is fixed
as well, that's what's blocking using the public gitlab CI runners,
thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210517/51061c43/attachment.sig>

[PATCH v2 28/28] fs/squashfs: implement exists() function

[Richard Genoud]

This permits to find a file and use the distro_bootcmd

Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
---
 fs/fs.c            |  2 +-
 fs/squashfs/sqfs.c | 38 ++++++++++++++++++++++++++++++++++++++
 include/squashfs.h |  1 +
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/fs/fs.c b/fs/fs.c
index fb27c910d4f..7a4020607a3 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -296,7 +296,7 @@ static struct fstype_info fstypes[] = {
 		.size = sqfs_size,
 		.close = sqfs_close,
 		.closedir = sqfs_closedir,
-		.exists = fs_exists_unsupported,
+		.exists = sqfs_exists,
 		.uuid = fs_uuid_unsupported,
 		.write = fs_write_unsupported,
 		.ln = fs_ln_unsupported,
diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index a96c1d4f564..c8e8d682cd4 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1657,6 +1657,44 @@ free_strings:
 	return ret;
 }
 
+int sqfs_exists(const char *filename)
+{
+	struct fs_dir_stream *dirsp = NULL;
+	struct squashfs_dir_stream *dirs;
+	char *dir, *file;
+	struct fs_dirent *dent;
+	int ret;
+
+	sqfs_split_path(&file, &dir, filename);
+	/*
+	 * sqfs_opendir will uncompress inode and directory tables, and will
+	 * return a pointer to the directory that contains the requested file.
+	 */
+	ret = sqfs_opendir(dir, &dirsp);
+	if (ret) {
+		ret = -EINVAL;
+		goto free_strings;
+	}
+
+	dirs = (struct squashfs_dir_stream *)dirsp;
+
+	while (!sqfs_readdir(dirsp, &dent)) {
+		ret = strcmp(dent->name, file);
+		if (!ret)
+			break;
+		free(dirs->entry);
+		dirs->entry = NULL;
+	}
+
+	sqfs_closedir(dirsp);
+
+free_strings:
+	free(dir);
+	free(file);
+
+	return ret == 0;
+}
+
 void sqfs_close(void)
 {
 	free(ctxt.sblk);
diff --git a/include/squashfs.h b/include/squashfs.h
index 819cf8c2da8..7489eefa1f2 100644
--- a/include/squashfs.h
+++ b/include/squashfs.h
@@ -19,6 +19,7 @@ int sqfs_probe(struct blk_desc *fs_dev_desc,
 int sqfs_read(const char *filename, void *buf, loff_t offset,
 	      loff_t len, loff_t *actread);
 int sqfs_size(const char *filename, loff_t *size);
+int sqfs_exists(const char *filename);
 void sqfs_close(void);
 void sqfs_closedir(struct fs_dir_stream *dirs);
 

[PATCH v2 28/28] fs/squashfs: implement exists() function

[João Marcos Costa]

Reviewed-by Joao Marcos Costa <jmcosta944@gmail.com>

Em ter., 3 de nov. de 2020 ?s 08:12, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> This permits to find a file and use the distro_bootcmd
>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>
> ---
>  fs/fs.c            |  2 +-
>  fs/squashfs/sqfs.c | 38 ++++++++++++++++++++++++++++++++++++++
>  include/squashfs.h |  1 +
>  3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fs.c b/fs/fs.c
> index fb27c910d4f..7a4020607a3 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -296,7 +296,7 @@ static struct fstype_info fstypes[] = {
>                 .size = sqfs_size,
>                 .close = sqfs_close,
>                 .closedir = sqfs_closedir,
> -               .exists = fs_exists_unsupported,
> +               .exists = sqfs_exists,
>                 .uuid = fs_uuid_unsupported,
>                 .write = fs_write_unsupported,
>                 .ln = fs_ln_unsupported,
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index a96c1d4f564..c8e8d682cd4 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -1657,6 +1657,44 @@ free_strings:
>         return ret;
>  }
>
> +int sqfs_exists(const char *filename)
> +{
> +       struct fs_dir_stream *dirsp = NULL;
> +       struct squashfs_dir_stream *dirs;
> +       char *dir, *file;
> +       struct fs_dirent *dent;
> +       int ret;
> +
> +       sqfs_split_path(&file, &dir, filename);
> +       /*
> +        * sqfs_opendir will uncompress inode and directory tables, and
> will
> +        * return a pointer to the directory that contains the requested
> file.
> +        */
> +       ret = sqfs_opendir(dir, &dirsp);
> +       if (ret) {
> +               ret = -EINVAL;
> +               goto free_strings;
> +       }
> +
> +       dirs = (struct squashfs_dir_stream *)dirsp;
> +
> +       while (!sqfs_readdir(dirsp, &dent)) {
> +               ret = strcmp(dent->name, file);
> +               if (!ret)
> +                       break;
> +               free(dirs->entry);
> +               dirs->entry = NULL;
> +       }
> +
> +       sqfs_closedir(dirsp);
> +
> +free_strings:
> +       free(dir);
> +       free(file);
> +
> +       return ret == 0;
> +}
> +
>  void sqfs_close(void)
>  {
>         free(ctxt.sblk);
> diff --git a/include/squashfs.h b/include/squashfs.h
> index 819cf8c2da8..7489eefa1f2 100644
> --- a/include/squashfs.h
> +++ b/include/squashfs.h
> @@ -19,6 +19,7 @@ int sqfs_probe(struct blk_desc *fs_dev_desc,
>  int sqfs_read(const char *filename, void *buf, loff_t offset,
>               loff_t len, loff_t *actread);
>  int sqfs_size(const char *filename, loff_t *size);
> +int sqfs_exists(const char *filename);
>  void sqfs_close(void);
>  void sqfs_closedir(struct fs_dir_stream *dirs);
>
>

-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta

[PATCH v2 28/28] fs/squashfs: implement exists() function

[Tom Rini]

On Tue, Nov 03, 2020 at 12:11:26PM +0100, Richard Genoud wrote:

> This permits to find a file and use the distro_bootcmd
> 
> Reviewed-by: Joao Marcos Costa <jmcosta944@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@posteo.net>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201119/67e8ae5e/attachment-0001.sig>

[PATCH v2 00/28] fs/squashfs: fix memory leaks and introduce exists() function

[Richard Genoud]

Le 03/11/2020 ? 12:10, Richard Genoud a ?crit?:
> This patch series fix several memory leaks, some use of dangling
> pointers (leading to cpu freeze) and finally introduce the exists()
> function for squashfs.
> This function enable testing the existence of a file, which is mandatory
> for using the distro_bootcmd
> Those fixes have been cut into several patches to be easier to review
> 
> Changes since v1:
> - patch 5: *dir = *file = NULL; is split in 2 lines
Sorry, I forgot a change:
- in patch 15, I removed a sqfs_closedir(dirsp) that I forgot in v1

> - For consistency, sqfs_frag_lookup is modified to use a single "goto
>    out"
> ( cf https://lists.denx.de/pipermail/u-boot/2020-October/429645.html )
> - more memory leak fixes in sqfs_get_abs_path, sqfs_read and sqfs_probe
> - a missing error check typo in sqfs_get_abs_path
> - some missing reseting ctxt.sblk to NULL to prevent double free
> - reset cur_dev/cur_part_info to NULL when they are freed
> - return value of sqfs_decompressor_init() wasn't used
> - use "len" in sqfs_read to prevent writing beyond buffer
> - prevent reading with an offset since it doesn't work
> - prevent reading fragmented files since it doesn't work
> 
> Richard Genoud (28):
>    fs/squashfs: fix board hang-up when calling .exists()
>    fs/squashfs: sqfs_opendir: fix some memory leaks and dangling pointers
>    fs/squashfs: sqfs_opendir: simplify error handling
>    fs/squashfs: sqfs_closedir: fix memory leak
>    fs/squashfs: sqfs_split_path: fix memory leak and dangling pointers
>    fs/squashfs: sqfs_read_directory_table: fix memory leak
>    fs/squashfs: sqfs_search_dir: fix dangling pointer
>    fs/squashfs: sqfs_search_dir: fix memory leaks
>    fs/squashfs: sqfs_read_inode_table: fix dangling pointer
>    fs/squashfs: sqfs_concat_tokens: check if malloc succeeds
>    fs/squashfs: sqfs_size: fix dangling pointer dirs->entry
>    fs/squashfs: sqfs_size: remove useless sqfs_closedir()
>    fs/squashfs: sqfs_read: fix dangling pointer dirs->entry
>    fs/squashfs: sqfs_read: remove useless sqfs_closedir()
>    fs/squashfs: sqfs_read: fix memory leak
>    fs/squashfs: sqfs_read: fix another memory leak
>    fs/squashfs: sqfs_frag_lookup: simplify error handling
>    fs/squashfs: sqfs_get_abs_path: fix error check
>    fs/squashfs: sqfs_get_abs_path: fix possible memory leak on error
>    fs/squashfs: sqfs_read: fix memory leak on finfo.blk_sizes
>    fs/squashfs: sqfs_probe: fix possible memory leak on error
>    fs/squashfs: sqfs_close/sqfs_read_sblk: set ctxt.sblk to NULL after
>      free
>    fs/squashfs: sqfs_probe: reset cur_dev/cur_part_info to NULL on error
>    fs/squashfs: sqfs_probe: use sqfs_decompressor_init() return value
>    fs/squashfs: sqfs_read: don't write beyond buffer size
>    fs/squashfs: sqfs_read: remove buggy offset functionality
>    fs/squashfs: sqfs_read: fragmented files are not supported
>    fs/squashfs: implement exists() function
> 
>   fs/fs.c            |   7 +
>   fs/squashfs/sqfs.c | 399 +++++++++++++++++++++++++++++++--------------
>   include/squashfs.h |   1 +
>   3 files changed, 286 insertions(+), 121 deletions(-)
> 

[PATCH v2 00/28] fs/squashfs: fix memory leaks and introduce exists() function

[João Marcos Costa]

I haven't been able to finish reviewing and testing everything yet, but I
thank you in advance for those fixes!

Best regards,

Em ter., 3 de nov. de 2020 ?s 08:18, Richard Genoud <
richard.genoud@posteo.net> escreveu:

> Le 03/11/2020 ? 12:10, Richard Genoud a ?crit :
> > This patch series fix several memory leaks, some use of dangling
> > pointers (leading to cpu freeze) and finally introduce the exists()
> > function for squashfs.
> > This function enable testing the existence of a file, which is mandatory
> > for using the distro_bootcmd
> > Those fixes have been cut into several patches to be easier to review
> >
> > Changes since v1:
> > - patch 5: *dir = *file = NULL; is split in 2 lines
> Sorry, I forgot a change:
> - in patch 15, I removed a sqfs_closedir(dirsp) that I forgot in v1
>
> > - For consistency, sqfs_frag_lookup is modified to use a single "goto
> >    out"
> > ( cf https://lists.denx.de/pipermail/u-boot/2020-October/429645.html )
> > - more memory leak fixes in sqfs_get_abs_path, sqfs_read and sqfs_probe
> > - a missing error check typo in sqfs_get_abs_path
> > - some missing reseting ctxt.sblk to NULL to prevent double free
> > - reset cur_dev/cur_part_info to NULL when they are freed
> > - return value of sqfs_decompressor_init() wasn't used
> > - use "len" in sqfs_read to prevent writing beyond buffer
> > - prevent reading with an offset since it doesn't work
> > - prevent reading fragmented files since it doesn't work
> >
> > Richard Genoud (28):
> >    fs/squashfs: fix board hang-up when calling .exists()
> >    fs/squashfs: sqfs_opendir: fix some memory leaks and dangling pointers
> >    fs/squashfs: sqfs_opendir: simplify error handling
> >    fs/squashfs: sqfs_closedir: fix memory leak
> >    fs/squashfs: sqfs_split_path: fix memory leak and dangling pointers
> >    fs/squashfs: sqfs_read_directory_table: fix memory leak
> >    fs/squashfs: sqfs_search_dir: fix dangling pointer
> >    fs/squashfs: sqfs_search_dir: fix memory leaks
> >    fs/squashfs: sqfs_read_inode_table: fix dangling pointer
> >    fs/squashfs: sqfs_concat_tokens: check if malloc succeeds
> >    fs/squashfs: sqfs_size: fix dangling pointer dirs->entry
> >    fs/squashfs: sqfs_size: remove useless sqfs_closedir()
> >    fs/squashfs: sqfs_read: fix dangling pointer dirs->entry
> >    fs/squashfs: sqfs_read: remove useless sqfs_closedir()
> >    fs/squashfs: sqfs_read: fix memory leak
> >    fs/squashfs: sqfs_read: fix another memory leak
> >    fs/squashfs: sqfs_frag_lookup: simplify error handling
> >    fs/squashfs: sqfs_get_abs_path: fix error check
> >    fs/squashfs: sqfs_get_abs_path: fix possible memory leak on error
> >    fs/squashfs: sqfs_read: fix memory leak on finfo.blk_sizes
> >    fs/squashfs: sqfs_probe: fix possible memory leak on error
> >    fs/squashfs: sqfs_close/sqfs_read_sblk: set ctxt.sblk to NULL after
> >      free
> >    fs/squashfs: sqfs_probe: reset cur_dev/cur_part_info to NULL on error
> >    fs/squashfs: sqfs_probe: use sqfs_decompressor_init() return value
> >    fs/squashfs: sqfs_read: don't write beyond buffer size
> >    fs/squashfs: sqfs_read: remove buggy offset functionality
> >    fs/squashfs: sqfs_read: fragmented files are not supported
> >    fs/squashfs: implement exists() function
> >
> >   fs/fs.c            |   7 +
> >   fs/squashfs/sqfs.c | 399 +++++++++++++++++++++++++++++++--------------
> >   include/squashfs.h |   1 +
> >   3 files changed, 286 insertions(+), 121 deletions(-)
> >
>


-- 
Atenciosamente,
Jo?o Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta