Re: [PATCH1/1]sqfs: sqfs_tokenize() should fill the tokens list instead of free items

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hello,

trini@konsulko.com wrote on Tue, 26 Oct 2021 15:25:56 -0400:

> On Sat, Oct 16, 2021 at 10:19:48AM +0800, Jincheng Wang wrote:
> 
> > We can delete two lines of code to avoid double free bug, but still a wild
> > pointers bug.
> > 
> > A test for wild pointers:
> >     sqfsls host 0  1//2/3//4/5
> > 
> > Fill the tokens list can solve it well.
> > 
> > 
> > Signed-off-by: Jincheng Wang <jc.w4ng@gmail.com>
> > ---
> >  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 e2d91c654c..50d3f8b71e 100644
> > --- a/fs/squashfs/sqfs.c
> > +++ b/fs/squashfs/sqfs.c
> > @@ -303,8 +303,9 @@ static int sqfs_tokenize(char **tokens, int count,
> > const char *str)
> >   aux = strtok(!j ? strc : NULL, "/");
> >   tokens[j] = strdup(aux);
> >   if (!tokens[j]) {
> > - for (i = 0; i < j; i++)
> > - free(tokens[i]);

Where is the double free here? Why do you stop freeing the tokens list?

> > + /* fill tokens list to avoid wild pointers being freed*/
> > + for (i = j + 1; i < count; i++)
> > + tokens[i] = 0;

This does not feel right. Perhaps it's just me not getting through the
lack of spacing successfully but this deserves more explanations.

> >   ret = -ENOMEM;
> >   goto free_strc;  
> 
> Aside from the whitespace having been destroyed, any comments from the
> maintainers / reviewers?  Thanks!
> 

Thanks,
Miquèl