[U-Boot] [PATCH 2/3] fs: cbfs: fix locating the cbfs header

[Alexander Graf]

On 15.02.18 07:40, Andre Heider wrote:
> The value at the end of the rom is not a pointer, it is an offset
> relative to the end of rom.

Do you have any documentation pointers to this? Even just pointing to a
specific line of code in coreboot would be helpful in case anyone wants
to read it up.

> 
> Signed-off-by: Andre Heider <a.heider at gmail.com>
> ---
>  fs/cbfs/cbfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c
> index 6e1107d751..46da8f134f 100644
> --- a/fs/cbfs/cbfs.c
> +++ b/fs/cbfs/cbfs.c
> @@ -168,9 +168,9 @@ static int file_cbfs_load_header(uintptr_t end_of_rom,
>  				 struct cbfs_header *header)
>  {
>  	struct cbfs_header *header_in_rom;
> +	int32_t offset = *(u32 *)(end_of_rom - 3);

Accessing a pointer that gets subtracted by 3 looks terribly wrong.
Unfortunately it's correct, because the pointer itself is unaligned.

How about we change the logic so that we calculate an aligned pointer
(after_rom?) which we sanity check for alignment and base all
calculations on that instead?

That way the next time someone comes around he's not getting puzzled why
we're subtracting 3 and adding 1 to the pointer ;).


Alex

>  
> -	header_in_rom = (struct cbfs_header *)(uintptr_t)
> -			*(u32 *)(end_of_rom - 3);
> +	header_in_rom = (struct cbfs_header *)(end_of_rom + offset + 1);
>  	swap_header(header, header_in_rom);
>  
>  	if (header->magic != good_magic || header->offset >
>