[SeaBIOS] SeaBIOS crashes on CBFS without legacy header

[Kevin O'Connor]

On Thu, Dec 24, 2015 at 03:45:03PM -0800, Alex Gagniuc wrote:
> When SeaBIOS is run on an FMAP-based CBFS (AKA, without a CBFS master
> header, it crashes). The problem is in coreboot_cbfs_init. A
> disassembly with -Mintel shows the following:
> 
> 00000000 <coreboot_cbfs_init>:
>    ...
>    7: 8b 35 fc ff ff ff     mov    esi,DWORD PTR ds:0xfffffffc
>    d: 81 3e 4f 52 42 43     cmp    DWORD PTR [esi],0x4342524f
> 
> > 7: 8b 35 fc ff ff ff     mov    esi,DWORD PTR ds:0xfffffffc
> 
> Move the contents of memory address 0xfffffffc to esi. Since there's
> no legacy CBFS pointer, the region is all 1's. esi now contains
> 0xffffffff
> 
> > d: 81 3e 4f 52 42 43     cmp    DWORD PTR [esi],0x4342524f
> 
> 32-bit dereference esi (0xffffffff) -- causes general protection
> fault. The dereference is not because of unaligned access, but because
> the access goes over the 32-bit address space in flat mode. The way
> around this iss to sanitize the value at [0xffffffff] before
> dereferencing it. A sane check is:
> 
>  if(address_of_hdr > (0xfffffff0 - sizeof(*hdr)))
>      fail();
> This is sane check because because the x86 reset vector is resides at
> 0xfffffff0 (there are cases where this isn't true, but that's besides
> the point here).

Thanks.  Can you put together a patch, or do you want me to?

> There's also an unrelated problem with the C code that generates this assembly:
> 
> >    struct cbfs_header *hdr = *(void **)(CONFIG_CBFS_LOCATION - 4);
> 
> With CONFIG_CBFS_LOCATION = 0, this places a negative value (-4) in an
> unsigned type (pointer). The behavior of this is undefined according
> to the C standard.

Are you suggesting that an (unsigned) cast should be added?  I doubt
it would matter.

-Kevin