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

[Andre Heider]

Hi Bin,

this patch reminds me of one I sent some time ago:
http://patchwork.ozlabs.org/patch/873666/

I forgot about it, so didn't follow up on the comments, but iirc this 
fixed a 'Bad CBFS file' error for me too. Does that work for you?

Regards,
Andre

On 22/12/2018 10:58, Bin Meng wrote:
> 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
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>