[coreboot] Coreboot bug?

[ron minnich]

On Wed, Nov 11, 2009 at 3:31 PM, Myles Watson <mylesgw at gmail.com> wrote:
> On Wed, Nov 11, 2009 at 4:06 PM, Peter Stuge <peter at stuge.se> wrote:
>> Myles Watson wrote:
>>> How about this:
>>>
>>> Index: src/arch/i386/boot/coreboot_table.c
>>> ===================================================================
>>> --- src/arch/i386/boot/coreboot_table.c       (revision 4931)
>>> +++ src/arch/i386/boot/coreboot_table.c       (working copy)
>>> @@ -485,11 +485,10 @@
>>>
>>>  #if (CONFIG_HAVE_OPTION_TABLE == 1)
>>>       {
>>> -             struct lb_record *rec_dest, *rec_src;
>>> -             /* Write the option config table... */
>>> +             struct lb_record *rec_dest;
>>> +             /* Copy the option config table, it's already a lb_record... */
>>>               rec_dest = lb_new_record(head);
>>> -             rec_src = (struct lb_record *)(void *)&option_table;
>>> -             memcpy(rec_dest,  rec_src, rec_src->size);
>>> +             memcpy(rec_dest,  &option_table, sizeof(option_table));
>>
>> It is completely unclear to me why it is safe to write beyond the
>> struct lb_record
> lb_record is just the header.  The data follows it, but isn't a member
> of the struct.
>
>> (maybe it is an elaborate side-effect of the call to
>> lb_new_record()?)
> I think lb_new_record uses the size to find the next header location.
> Is that what you meant?
>
>> Acked-by: Peter Stuge <peter at stuge.se>
> Rev 4935.

I am almost certain that this change:
>>> -             memcpy(rec_dest,  rec_src, rec_src->size);
>>> +             memcpy(rec_dest,  &option_table, sizeof(option_table));

completely changes the behavior of the code and is wrong.

I'm willing to be convinced. But sizeof(option_table) is 8 and
rec_src->size is 1160. So you
were copying the whole record and now you're copying a record header.
I don't see how this can be a good idea. Am I missing something?

It seems to me to change a compiler warning you may have broken what
the code is supposed to do.

ron