[vbox-dev] [PATCH] Fix broken SMBios checksum calculation
Martin Fleisz
martin.fleisz at thincast.com
Mon Feb 11 11:35:57 GMT 2019
Hi Michal,
exactly ... that was what I tried to explain! :-)
Gave the patch a go and it works just fine - thanks!
Cheers, Martin
On 11.02.2019 12:09, Michal Necasek wrote:
>
> Hi Martin,
>
> I think I now finally understand what's happening. The checksum
> calculation code in DevPcBios.cpp is wrong... *but* it actually
> delivers the correct result, as long as the BIOS image was run through
> 'biossums' first!
>
> Here's why: biossums calculates the checksum of the _DMI_ header and
> the _SM_ header (at that point not containing final values), and then
> calculates and updates the checksum of the entire BIOS image. Now, the
> 8-bit checksum of the _DMI_ header is 0, and the checksum of the
> entire BIOS image is also 0. Which means that when you calculate the
> checksum of the 15 _DMI_ header bytes, you'll get some value, and when
> you add the checksum of the rest of the BIOS image, the result won't
> change.
>
> You probably do not have the correct checksum of the entire BIOS
> image, and that is why the _DMI_ checksum calculation produces a bogus
> result for you. I have the checksum right, so the obviously wrong code
> still gets the right answer.
>
> I decided to throw the checksumming code in DevPcBios.cpp out
> completely and replace it with the already existing
> FwCommonPlantSmbiosAndDmiHdrs() function. That should produce the
> right checksums regardless of whether the existing checksums in the
> BIOS image are correct or not. Patch is attached, please let me know
> if it works for you.
>
>
> Cheers,
> Michal
>
>
> On 2/5/2019 5:57 PM, Martin Fleisz wrote:
>> Hi Michal,
>>
>> when you look at the source of dmidecode that is not really the case.
>> You are right that dmidecode first looks for the _SM_ header. But then
>> it will check the _SM_ checksum, the _DMI_ signature and the _DMI_
>> checksum (see
>> https://github.com/mirror/dmidecode/blob/master/dmidecode.c#L5379). If
>> any of these is wrong/missing it will fail with the error message from
>> my previous mail.
>> In both our cases the _DMI_ checksum is wrong (as you stated the code
>> obviously calculates the wrong checksum).
>> The _SM_ checksum is just wrong because of the incorrect value for the
>> _DMI_ checksum. If the _DMI_ checksum was calculated correctly, the _SM_
>> checksum is correct as it is now.
>> I hope that also makes sense ... maybe we even mean the same thing
>> anyways ... :-)
>>
>> Cheers, Martin
>>
>> On 05.02.2019 17:37, Michal Necasek wrote:
>>>
>>> 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
>>>>
>>>
>>> _______________________________________________
>>> 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
More information about the vbox-dev
mailing list