[U-Boot] [PATCH] fs: cbfs: remove wrong header validation

[Bin Meng]

Hi Christian,

On Tue, Dec 18, 2018 at 4:57 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Christian,
>
> On Tue, Dec 18, 2018 at 4:18 PM Christian Gmeiner
> <christian.gmeiner at gmail.com> wrote:
> >
> > Am Mi., 12. Dez. 2018 um 15:27 Uhr schrieb Christian Gmeiner
> > <christian.gmeiner at gmail.com>:
> > >
> > > Hi Bin,
> > >
> > > Finally I have some time to look deeper into this issue.
> > >
> > > >
> > > > On Thu, Sep 20, 2018 at 10:47 PM Christian Gmeiner
> > > > <christian.gmeiner at gmail.com> wrote:
> > > > >
> > > > > Coreboot does not contain such a check:
> > > > > https://github.com/coreboot/coreboot/blob/eeb4e20b2f6d786c92fe3efb30817e90389a2bfe/src/commonlib/cbfs.c#L64
> > > > >
> > > > > Before this change cbfsinit failed with 'Bad CBFS file'. After this change all cbfs commands
> > > > > are working as expected.
> > > > >
> > > > > Signed-off-by: Christian Gmeiner <christian.gmeiner at gmail.com>
> > > > > ---
> > > > >  fs/cbfs/cbfs.c | 6 +-----
> > > > >  1 file changed, 1 insertion(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c
> > > > > index 0dce639b49..2a581f0c18 100644
> > > > > --- a/fs/cbfs/cbfs.c
> > > > > +++ b/fs/cbfs/cbfs.c
> > > > > @@ -96,11 +96,7 @@ static int file_cbfs_next_file(u8 *start, u32 size, u32 align,
> > > > >                 }
> > > > >
> > > > >                 swap_file_header(&header, fileHeader);
> > > > > -               if (header.offset < sizeof(struct cbfs_fileheader) ||
> > > > > -                   header.offset > header.len) {
> > > > > -                       file_cbfs_result = CBFS_BAD_FILE;
> > > > > -                       return -1;
> > > > > -               }
> > > >
> > > > It looks to me the existing codes were doing some sanity checks. Can
> > > > you elaborate why this is failing on your board? In your coreboot
> > > > reference, I don't see exactly how U-Boot codes are connected to the
> > > > coreboot one.
> > > >
> > >
> > > This has nothing to do with my board at all - I can easily reproduce
> > > this issue under qemu:
> > >

[snip]

> > >
> > >
> > > What is needed to get further with this patch?
> > >
> >
> > ping
>
> Sorry, I meant to take some time to have a look at this. Will do this week.
>

I have looked at this patch. It looks we just need remove the
"header.offset > header.len" check. The other check is still valid.

During the investigation, I've noticed some other issues in the cbfs
codes. I've included your patch in my series [1]. Please have a look.

[1] http://patchwork.ozlabs.org/project/uboot/list/?series=83355

Regards,
Bin