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

[Miquel Raynal]

Miquel Raynal <miquel.raynal at bootlin.com> wrote on Mon, 21 Dec 2020
16:14:19 +0100:

> Hi Richard,
> 
> Richard Genoud <richard.genoud at posteo.net> wrote on Mon, 21 Dec 2020
> 16:06:37 +0100:
> 
> > 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]);  
> 
> I had a look at the code, the error path is clearly not correctly
> organized.
> 
> I think the right approach would be to have real labels like,
> free_token_list, free_this, free_that and for each of them only do the
> right cleanup. Doing so should fix the issue.

Actually I remember now: I disliked your proposal of changing all
named labels to a single (and quite unclear) "goto out". This is an
example of why single labels should not be used IMHO.

Thanks,
Miquèl