Re: [PATCH] fs: squasfs: fix a possible NULL pointer dereference in sqfs_opendir()

[Richard Genoud]

Hi Miquel,

Le 18/12/2020 à 19:50, Miquel Raynal a écrit :

Hi Richard,

Richard Genoud <richard.gen...@posteo.net> wrote on Fri, 18 Dec 2020
15:24:40 +0100:

token_count may be != 0 and token_list not yet allocated when the out
code is reached

Wouldn't it be better to initialize token_count than adding an
(obscure) indentation level?

Well, token_count is initialized :
token_count = sqfs_count_tokens(filename);

But token_list is not yet populated. It is some lines bellow:
token_list = malloc(token_count * sizeof(char *));


But I could use something like that, maybe it's clearer :
        for (j = 0; (token_list != NULL) && (j < token_count); j++)
                free(token_list[j]);

Reported-by: Coverity CID 313547
Fixes: ea1b1651c6a8 ("fs/squashfs: sqfs_opendir: simplify error handling")
Signed-off-by: Richard Genoud <richard.gen...@posteo.net>
---
  fs/squashfs/sqfs.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 608a2bb454c..c47046b76e5 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -949,8 +949,9 @@ int sqfs_opendir(const char *filename, struct fs_dir_stream 
**dirsp)
        *dirsp = (struct fs_dir_stream *)dirs;

out:

-       for (j = 0; j < token_count; j++)
-               free(token_list[j]);
+       if (token_list)
+               for (j = 0; j < token_count; j++)
+                       free(token_list[j]);
        free(token_list);
        free(pos_list);
        free(path);

Thanks,
Miquèl

Thanks
Richard.