[vbox-dev] [PATCH] Fix broken SMBios checksum calculation

[Michal Necasek]

    Hi Martin,

  That's not what I'm saying. What I'm saying is that because my BIOS 
image was run through biossums, I didn't see the dmidecode problem 
because although the _DMI_ header checksum is wrong (incorrect 
calculation in DevPcBios.cpp), it does not matter in practice because 
dmidecode finds the _SM_ header first.

  In other words, for you both _SM_ and _DMI_ headers had bad checksums, 
but for me only _DMI_ did. After your fix (which is correct), the _SM_ 
header you have probably still has a bad checksum but dmidecode does not 
care because it finds the valid _DMI_ header.

  I hope that made sense.

     - Michal

On 2/5/2019 5:27 PM, Martin Fleisz wrote:
> Hi Michal,
> 
> I don't think that biossums is the way you can fix this issue because in
> DevPcBios.cpp the _DMI_ table is patched dynamically (see
> https://www.virtualbox.org/browser/vbox/trunk/src/VBox/Devices/PC/DevPcBios.cpp#L1594
> where the number and size of the DMI tables is updated) and therefore
> the checksum must be re-calculated. The _SM_ table checksum is not
> affected by these changes (as long as the _DMI_ checksum is correctly).
> 
> Cheers, Martin
> 
> On 05.02.2019 17:02, Michal Necasek wrote:
>>
>>     Hi Martin,
>>
>>   Now it's making sense. The '_DMI_' table header checksum code is
>> wrong in DevPCBios.cpp. But dmidecode normally does not get that far,
>> because it finds the '_SM_' marker first, and that one has the right
>> checksum if the BIOS image got run through the 'biossums' tool. I
>> suspect that's not happening for you, and that is something which is
>> probably missing from our makefiles.
>>
>>   I can't see into your repository on github. But you're probably right
>> and it's not doing any harm. Anyway, now I see how to reproduce the
>> problem, thanks.
>>
>>       - Michal
>>
>> On 2/5/2019 4:09 PM, Martin Fleisz wrote:
>>> Hi Michal,
>>>
>>> I compile the OSE version (with some very small modifications in
>>> VBoxBiosAlternative386.asm
>>> <https://github.com/thincast/VirtualBox-RDP-Server/commit/98e7959e484a0145e39619dc6dbc40d3b9282520#diff-9c7472aa47554e70acbb1a4298b673e4>
>>>
>>> but that shouldn't do any harm to my understanding) and when running
>>> dmidecode on Ubuntu 18.04 it reports "# No SMBIOS nor DMI entry point
>>> found, sorry.". After applying the patch everything worked just fine.
>>> I checked with VirtualBox 6.0.4 and it works just fine out of the box
>>> ... so I guess either the checksum is somehow valid by accident or there
>>> is some code missing/different in the OSE repository.
>>>
>>> Best regards, Martin
>>>
>>> On 05.02.2019 15:48, Michal Necasek wrote:
>>>>
>>>>     Hi Martin,
>>>>
>>>>    The patch looks good, but I'm struggling to reproduce the problem.
>>>> The dmidecode utility in Ubuntu 18.04.1 and 18.10 (dmidecode version
>>>> 3.1 in both) does not generate any complaints. So what exactly does?
>>>>
>>>>        Regards,
>>>>          Michal
>>>>
>>>> On 2/4/2019 12:16 PM, Martin Fleisz wrote:
>>>>> When using legacy BIOS the Intermediate Checksum of the SMBIOS Entry
>>>>> Point Structure is calculated incorrectly.
>>>>>
>>>>> The calculation starts at the wrong offset (at the beginning of
>>>>> pu8PcBios rather than pu8PcBios[i]) and creates the sum over the
>>>>> whole pu8PcBios data instead of just the next 15 bytes.
>>>>>
>>>>> The patch below fixes checksum calculation and makes applications
>>>>> like dmidecode work again in the guest.
>>>>>
>>>>> The patch is contributed under the terms of the MIT license.
>>>>>
>>>>> Cheers, Martin
>>>>>
>>>>> diff --git src/VBox/Devices/PC/DevPcBios.cpp
>>>>> src/VBox/Devices/PC/DevPcBios.cpp
>>>>> index e967adff2f..7e56cc18fe 100644
>>>>> --- src/VBox/Devices/PC/DevPcBios.cpp
>>>>> +++ src/VBox/Devices/PC/DevPcBios.cpp
>>>>> @@ -1594,9 +1594,9 @@ static DECLCALLBACK(int)
>>>>> pcbiosConstruct(PPDMDEVINS pDevIns, int iInstance, PCF
>>>>>                 *(uint16_t*)&pThis->pu8PcBios[i + 0x06] =
>>>>> RT_H2LE_U16(cbDmiTables);
>>>>>                 *(uint16_t*)&pThis->pu8PcBios[i + 0x0C] =
>>>>> RT_H2LE_U16(cNumDmiTables);
>>>>>                 uint8_t u8Sum = 0;
>>>>> -            for (unsigned j = 0; j < pThis->cbPcBios; j++)
>>>>> -                if (j != i + 0x05)
>>>>> -                    u8Sum += pThis->pu8PcBios[j];
>>>>> +            for (unsigned j = 0; j < 0x0F; j++)
>>>>> +                if (j != 0x05)
>>>>> +                    u8Sum += pThis->pu8PcBios[i + j];
>>>>>                 pThis->pu8PcBios[i + 0x05] = -u8Sum;
>>>>>                 break;
>>>>>             }
>>>>>
>>>>> _______________________________________________
>>>>> vbox-dev mailing list
>>>>> vbox-dev at virtualbox.org
>>>>> https://www.virtualbox.org/mailman/listinfo/vbox-dev
>>>>>
>>>>
>>>> _______________________________________________
>>>> vbox-dev mailing list
>>>> vbox-dev at virtualbox.org
>>>> https://www.virtualbox.org/mailman/listinfo/vbox-dev
>>>
>>>
>>> _______________________________________________
>>> vbox-dev mailing list
>>> vbox-dev at virtualbox.org
>>> https://www.virtualbox.org/mailman/listinfo/vbox-dev
>>>
>>
>> _______________________________________________
>> vbox-dev mailing list
>> vbox-dev at virtualbox.org
>> https://www.virtualbox.org/mailman/listinfo/vbox-dev
> 
> 
> _______________________________________________
> vbox-dev mailing list
> vbox-dev at virtualbox.org
> https://www.virtualbox.org/mailman/listinfo/vbox-dev
>