From mboxrd@z Thu Jan  1 00:00:00 1970
From: Miquel Raynal <miquel.raynal@bootlin.com>
Date: Mon, 21 Dec 2020 16:14:19 +0100
Subject: [PATCH] fs: squasfs: fix a possible NULL pointer dereference in
 sqfs_opendir()
In-Reply-To: <e9785128-0df9-e7d4-8c65-89e8b79a37d2@posteo.net>
References: <20201218142440.21783-1-richard.genoud@posteo.net>
 <20201218195009.056da9d8@xps13>
 <e9785128-0df9-e7d4-8c65-89e8b79a37d2@posteo.net>
Message-ID: <20201221161419.478626f2@xps13>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Richard,

Richard Genoud <richard.genoud@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@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.

Thanks,
Miqu?l