[PATCH] fs/squashfs: fix reading of fragmented files

[João Marcos Costa]

Hello,

In fact, I really think this patch should be applied to master as soon
as possible, since the actual unsafety comes from the current code,
which may read past the fragment_block buffer size.
Besides, the patch series I sent to rewrite the test suite needs this
fix, and the current test suite is error-prone, as it was already
reported.

Best regards,


Em qua., 9 de jun. de 2021 às 14:40, Tom Rini <trini at konsulko.com> escreveu:
>
> On Wed, Jun 09, 2021 at 10:16:53AM -0300, João Marcos Costa wrote:
> > Hello, everyone
> >
> > Em qua., 26 de mai. de 2021 às 09:35, João Marcos Costa <
> > jmcosta944 at gmail.com> escreveu:
> >
> > > Hello, Miquèl
> > >
> > > Em qua., 26 de mai. de 2021 às 04:52, Miquel Raynal <
> > > miquel.raynal at bootlin.com> escreveu:
> > >
> > >> Hi Joao,
> > >>
> > >> Joao Marcos Costa <jmcosta944 at gmail.com> wrote on Mon, 17 May 2021
> > >> 18:20:38 -0300:
> > >>
> > >> > The fragmented files were not correctly read because of two issues:
> > >> >
> > >> > - The squashfs_file_info struct has a field named 'comp', which tells if
> > >> > the file's fragment is compressed or not. This field was always set to
> > >> > 'true' in sqfs_get_regfile_info and sqfs_get_lregfile_info. It should
> > >> > actually take sqfs_frag_lookup's return value. This patch addresses
> > >> > these two assignments.
> > >> >
> > >> > - In sqfs_read, the fragments (compressed or not) were copied to the
> > >> > output buffer through a for loop which was reading data at the wrong
> > >> > offset. Replace these loops by equivalent calls to memcpy, with the
> > >> > right parameters.
> > >>
> > >> Good idea to get rid of these memcpy of 1 byte :)
> > >>
> > >> > I tested this patch by comparing the MD5 checksum of a few fragmented
> > >> > files with the respective md5sum output in sandbox, considering both
> > >> > compressed and uncompressed fragments.
> > >> >
> > >> > Signed-off-by: Joao Marcos Costa <jmcosta944 at gmail.com>
> > >>
> > >> Reviewed-by: Miquel Raynal <miquel.raynal at bootlin.com>
> > >>
> > >> But next time, when you fix two issues (even if they fix the same
> > >> feature) please provide two patches ;)
> > >>
> > >> Thanks,
> > >> Miquèl
> > >>
> > >
> > >
> > Any updates on this patch review?
>
> Seems fine, but I'm also leaning on grabbing all of the squashfs patches
> for -next at this point, unless people have strong feelings about it
> being safe at this point for master, thanks.
>
> --
> Tom



-- 
Atenciosamente,
João Marcos Costa

www.linkedin.com/in/jmarcoscosta/
https://github.com/jmarcoscosta