[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.genoud at 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.genoud at 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.