From mboxrd@z Thu Jan 1 00:00:00 1970 From: Miquel Raynal Date: Mon, 21 Dec 2020 16:29:19 +0100 Subject: [PATCH] fs: squasfs: fix a possible NULL pointer dereference in sqfs_opendir() In-Reply-To: References: <20201218142440.21783-1-richard.genoud@posteo.net> <20201218195009.056da9d8@xps13> <20201221161419.478626f2@xps13> Message-ID: <20201221162919.2133c37f@xps13> List-Id: 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 wrote on Mon, 21 Dec 2020 16:26:00 +0100: > Le 21/12/2020 ? 16:14, Miquel Raynal a ?crit?: > > Hi Richard, > > > > Richard Genoud 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 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. > > So you're suggesting to revert this ? > commit ea1b1651c6a8 ("fs/squashfs: sqfs_opendir: simplify error handling") Yes (our e-mails crossed each other), I think it's best to have a well organized error path. Of course this error path is maybe faulty, in this case it must be fixed. But I personally prefer the revert + fix approach. Thanks, Miqu?l