[SeaBIOS] SeaBIOS crashes on CBFS without legacy header

Alex G. mr.nuke.me at gmail.com
Tue Dec 29 01:09:22 CET 2015


On 12/28/2015 09:31 AM, Kevin O'Connor wrote:
> On Thu, Dec 24, 2015 at 03:45:03PM -0800, Alex Gagniuc wrote:
>>  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?

I won't be able to test on actual hardware until next year, so feel free
to do so.

>> 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.

I'm suggesting to not rely on (0 - 4) to produce 0xfffffffc. The
(unsigned) cast does not solve the C issue of writing a negative value
into an unsigned type. Personally, I would refactor to something like
(casts purchased separately):

#define CBFS_MASTER_PTR_LOC 0xfffffffc

if (CONFIG_CBFS_LOCATION)
	*hdr = (CONFIG_CBFS_LOCATION - 4);
else
	*hdr = CBFS_MASTER_PTR_LOC;

It's a little more code, but it avoids the trap of blaming things on
undefined behavior, a trap which I fell into when debugging the crash.

Alex



More information about the SeaBIOS mailing list