[edk2-devel] [PATCH v5 2/9] MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore (CVE-2019-11098)

[Laszlo Ersek]

On 07/13/20 08:38, Wang, Jian J wrote:
>> +VOID
>> +ConvertStatusCodeCallbacks (
>> +  IN  UINTN                   OrgFvHandle,
>> +  IN  UINTN                   FvHandle,
>> +  IN  UINTN                   FvSize
>> +  )
>> +{
>> +  EFI_PEI_HOB_POINTERS    Hob;
>> +  UINTN                   *NumberOfEntries;
>> +  UINTN                   *CallbackEntry;
>> +  UINTN                   Index;
>> +
>> +  Hob.Raw  = GetFirstGuidHob (&gStatusCodeCallbackGuid);
>> +  while (Hob.Raw != NULL) {
>> +    NumberOfEntries = GET_GUID_HOB_DATA (Hob);
>> +    CallbackEntry   = NumberOfEntries + 1;
>> +    for (Index = 0; Index < *NumberOfEntries; Index++) {
>> +      if (((VOID *) CallbackEntry[Index]) != NULL) {
>> +        if ((CallbackEntry[Index] >= OrgFvHandle) && (CallbackEntry[Index] <
>> (OrgFvHandle + FvSize))) {
>> +          DEBUG ((DEBUG_INFO, "Migrating CallbackEntry[%d] from 0x%08X to ",
>> Index, CallbackEntry[Index]));
> CallbackEntry is defined as pointer to UINTN, which is 4-byte with 32-bit PEI.
> Using %08X might be not a good idea. Suggest to use %p instead.
> 

For portability between 32-bit (such as IA32 and ARM) and 64-bit (such
as X64 and AARCH64), UINTN values should be printed as follows:

- cast them to UINT64
- print them with %Lx or %Lu

In the above message, we have two UINTN objects, Index and
CallbackEntry[Index]. Therefore, all of %d, %X, and %p are wrong. The
proper way is this:

          DEBUG ((
            DEBUG_INFO,
            "Migrating CallbackEntry[%Lu] from 0x%016Lx to ",
            (UINT64)Index,
            (UINT64)CallbackEntry[Index]
            ));

If you want to zero-pad to 8 nibbles only (not 16) on ARM and IA32,
that's possible too. Replace the constant field width "16" with "*", and
pass the field width explicitly, as a parameter:

          DEBUG ((
            DEBUG_INFO,
            "Migrating CallbackEntry[%Lu] from 0x%0*Lx to ",
            (UINT64)Index,
            (sizeof CallbackEntry[Index]) * 2,
            (UINT64)CallbackEntry[Index]
            ));

The field width specifier "*" takes an extra parameter, which needs to
be of type UINTN in edk2 (see BasePrintLibSPrintMarker()). The sizeof
operator produces an UINTN.

The following example:

  UINTN Foo;

  Foo = 0xABCD;
  DEBUG ((DEBUG_INFO, "Foo=0x%0*Lx\n", (sizeof Foo) * 2, (UINT64)Foo));

produces the following output on IA32:

  Foo=0x0000ABCD

and on X64:

  Foo=0x000000000000ABCD

Thanks,
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#63160): https://edk2.groups.io/g/devel/message/63160
Mute This Topic: https://groups.io/mt/75390174/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-