Re: 答复: [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add helper to valid memory addresses

[Paulo Alcantara <paulo@paulo.ac>]

Hi Jeff,

On 1/3/2018 6:42 AM, Fan Jeff wrote:
> Paul,
> 
> +  //
> +  // Calculate physical address of PML4E
> +  //
> +  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
> +
> 
> Should not pass VS build, instead you could use LShiftU64/RShiftU64 to 
> do 64bit shift operation as below:
> 
>    PhysicalAddress = (UINT64)Cr3 & LShiftU64 (LShiftU64 (1, 
> MaxPhyAddrBits) - 1, 12);
> 
>    PhysicalAddress |= LShiftU64 (RShiftU64((UINT64)LinearAddress, 39) & 
> 0x1FF), 3);

OK - I'll fix them up and then build-test it with IA32/X64 MSFT toolchain.

Thanks
Paulo

> 
> Jeff
> 
> *发件人: *Paulo Alcantara <mailto:paulo@paulo.ac>
> *发送时间: *2017年12月29日12:41
> *收件人: *edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>
> *抄送: *Laszlo Ersek <mailto:lersek@redhat.com>; Eric Dong 
> <mailto:eric.dong@intel.com>
> *主题: *[edk2] [RFC v4 4/6] UefiCpuPkg/CpuExceptionHandlerLib: Add 
> helper to valid memory addresses
> 
> Introduce IsLinearAddressValid() function that will be used for
> validating memory addresses that would get dereferenced during stack
> traces in IA32 and X64 CPU exceptions.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Requested-by: Brian Johnson <brian.johnson@hpe.com>
> Requested-by: Jiewen Yao <jiewen.yao@intel.com>
> Signed-off-by: Paulo Alcantara <paulo@paulo.ac>
> ---
>   UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c | 382 
> ++++++++++++++++++++
>   UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h |  16 +
>   2 files changed, 398 insertions(+)
> 
> diff --git 
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c 
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> index 867c5c01d6..52b3eb1463 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> @@ -14,6 +14,9 @@
> 
>   #include "CpuExceptionCommon.h"
> 
> +#include <Register/Msr.h>
> +#include <Library/DebugLib.h>
> +
>   //
>   // Error code flag indicating whether or not an error code will be
>   // pushed on the stack if an exception occurs.
> @@ -194,3 +197,382 @@ GetPdbFileName (
>       }
>     }
>   }
> +
> +/**
> +  Check if a linear address is valid by walking the page tables in 4-level
> +  paging mode.
> +
> +  @param[in]  Cr3             CR3 control register.
> +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
> +  @param[in]  LinearAddress   Linear address to be checked.
> +**/
> +STATIC
> +BOOLEAN
> +Do4LevelPagingModeCheck (
> +  IN UINTN            Cr3,
> +  IN UINT8            MaxPhyAddrBits,
> +  IN UINTN            LinearAddress
> +  )
> +{
> +  UINT64 PhysicalAddress;
> +  UINT64 *Pml4TableEntry;
> +  UINT64 *PageDirPtrTableEntry;
> +  UINT64 *PageDirEntry;
> +  UINT64 *PageTableEntry;
> +
> +  //
> +  // In 4-level paging mode, linear addresses are 48 bits wide
> +  //
> +  if ((UINT64)LinearAddress > (1ULL << 48) - 1) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Calculate physical address of PML4E
> +  //
> +  PhysicalAddress = (UINT64)Cr3 & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 39) & 0x1FF) << 3;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*Pml4TableEntry) - 1)) == 0);
> +
> +  Pml4TableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if a PDPTE is present
> +  //
> +  if ((*Pml4TableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Calculate physical address of PDPTE
> +  //
> +  PhysicalAddress = *Pml4TableEntry & (((1ULL << MaxPhyAddrBits) - 1) 
> << 12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 30) & 0x1FF) << 3;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirPtrTableEntry) - 1)) == 0);
> +
> +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check whether a PDPTE or 1GiB page entry is present
> +  //
> +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if PDPTE maps an 1GiB page
> +  //
> +  if ((*PageDirPtrTableEntry & BIT7) != 0) {
> +    return TRUE;
> +  }
> +
> +  //
> +  // Calculate physical address of PDE
> +  //
> +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) 
> - 1) <<
> +                                             12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 21) & 0x1FF) << 3;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> +
> +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check whether a PDE or a 2MiB page entry is present
> +  //
> +  if ((*PageDirEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if PDE maps a 2MiB page
> +  //
> +  if ((*PageDirEntry & BIT7) != 0) {
> +    return TRUE;
> +  }
> +
> +  //
> +  // Calculate physical address of PTE
> +  //
> +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> +  PhysicalAddress |= (((UINT64)LinearAddress >> 12) & 0x1FF) << 3;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> +
> +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if PTE maps a 4KiB page
> +  //
> +  if ((*PageTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
> +/**
> +  Check if a linear address is valid by walking the page tables in 
> 32-bit paging
> +  mode.
> +
> +  @param[in]  Cr3             CR3 control register.
> +  @param[in]  Cr4             CR4 control register.
> +  @param[in]  LinearAddress   Linear address to be checked.
> +**/
> +STATIC
> +BOOLEAN
> +Do32BitPagingModeCheck (
> +  IN UINTN            Cr3,
> +  IN UINTN            Cr4,
> +  IN UINTN            LinearAddress
> +  )
> +{
> +  UINT64 PhysicalAddress;
> +  UINT32 *PageDirEntry;
> +  UINT32 *PageTableEntry;
> +
> +  if (LinearAddress > MAX_UINT32) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Calculate physical address of PDE
> +  //
> +  PhysicalAddress = (UINT32)Cr3 & (((1ULL << 20) - 1) << 12);
> +  PhysicalAddress |= (((UINT32)LinearAddress >> 22) & 0x3FF) << 2;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> +
> +  PageDirEntry = (UINT32 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check whether a PTE or a 4MiB page is present
> +  //
> +  if ((*PageDirEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if PDE maps a 4MiB page
> +  //
> +  if ((Cr4 & BIT4) != 0 && (*PageDirEntry & BIT7) != 0) {
> +    return TRUE;
> +  }
> +
> +  //
> +  // Calculate physical address of PTE
> +  //
> +  PhysicalAddress = *PageDirEntry & (((1ULL << 20) - 1) << 12);
> +  PhysicalAddress |= (((UINT32)LinearAddress >> 12) & 0x3FF) << 2;
> +
> +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> +
> +  PageTableEntry = (UINT32 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if PTE maps a 4KiB page
> +  //
> +  if ((*PageTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
> +/**
> +  Check if a linear address is valid by walking the page tables in PAE 
> paging
> +  mode.
> +
> +  @param[in]  Cr3             CR3 control register.
> +  @param[in]  MaxPhyAddrBits  MAXPHYADDRBITS bits.
> +  @param[in]  LinearAddress   Linear address to be checked.
> +**/
> +STATIC
> +BOOLEAN
> +DoPAEPagingModeCheck (
> +  IN UINTN            Cr3,
> +  IN UINT8            MaxPhyAddrBits,
> +  IN UINTN            LinearAddress
> +  )
> +{
> +  UINT64 PhysicalAddress;
> +  UINT64 *PageDirPtrTableEntry;
> +  UINT64 *PageDirEntry;
> +  UINT64 *PageTableEntry;
> +
> +  if (LinearAddress > MAX_UINT32) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Calculate physical address of PDPTE
> +  //
> +  PhysicalAddress = (UINT32)Cr3 >> 5;
> +
> +  //
> +  // Select PDPTE register
> +  //
> +  PhysicalAddress +=
> +    ((UINT32)LinearAddress >> 30) * sizeof (*PageDirPtrTableEntry);
> +
> +  PageDirPtrTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if PDE is present
> +  //
> +  if ((*PageDirPtrTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  PhysicalAddress = *PageDirPtrTableEntry & (((1ULL << MaxPhyAddrBits) 
> - 1) <<
> +                                             12);
> +  PhysicalAddress |= ((LinearAddress >> 21) & 0x1FF) << 3;
> +  ASSERT ((PhysicalAddress & (sizeof (*PageDirEntry) - 1)) == 0);
> +
> +  PageDirEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check whether a PTE or a 2MiB page is present
> +  //
> +  if ((*PageDirEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if PDE maps a 2MiB page
> +  //
> +  if ((*PageDirEntry & BIT7) != 0) {
> +    return TRUE;
> +  }
> +
> +  //
> +  // Calculate physical address of PTE
> +  //
> +  PhysicalAddress = *PageDirEntry & (((1ULL << MaxPhyAddrBits) - 1) << 12);
> +  PhysicalAddress |= ((LinearAddress >> 12) & 0x1FF) << 3;
> +  ASSERT ((PhysicalAddress & (sizeof (*PageTableEntry) - 1)) == 0);
> +
> +  PageTableEntry = (UINT64 *)(UINTN)PhysicalAddress;
> +
> +  //
> +  // Check if PTE maps a 4KiB page
> +  //
> +  if ((*PageTableEntry & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
> +/**
> +  Check if a linear address is valid.
> +
> +  @param[in]  Cr0            CR0 control register.
> +  @param[in]  Cr3            CR3 control register.
> +  @param[in]  Cr4            CR4 control register.
> +  @param[in]  LinearAddress  Linear address to be checked.
> +**/
> +BOOLEAN
> +IsLinearAddressValid (
> +  IN  UINTN              Cr0,
> +  IN  UINTN              Cr3,
> +  IN  UINTN              Cr4,
> +  IN  UINTN              LinearAddress
> +  )
> +{
> +  UINT32                  Eax;
> +  UINT32                  Edx;
> +  UINT8                   MaxPhyAddrBits;
> +  MSR_IA32_EFER_REGISTER  Msr;
> +  BOOLEAN                 AddressValid;
> +
> +  //
> +  // Check for valid input parameters
> +  //
> +  if (Cr0 == 0 || Cr4 == 0 || LinearAddress == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Check if paging is disabled
> +  //
> +  if ((Cr0 & BIT31) == 0) {
> +    //
> +    // If CR4.PAE bit is set, then the linear (or physical) address 
> supports
> +    // only up to 36 bits.
> +    //
> +    if (((Cr4 & BIT5) != 0 && (UINT64)LinearAddress > 0xFFFFFFFFFULL) ||
> +        LinearAddress > 0xFFFFFFFF) {
> +      return FALSE;
> +    }
> +
> +    return TRUE;
> +  }
> +
> +  //
> +  // Paging can be enabled only if CR0.PE bit is set
> +  //
> +  if ((Cr0 & BIT0) == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // CR3 register cannot be zero if paging is enabled
> +  //
> +  if (Cr3 == 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Get MAXPHYADDR bits
> +  //
> +  AsmCpuid (0x80000000, &Eax, NULL, NULL, NULL);
> +  if (Eax >= 0x80000008) {
> +    AsmCpuid (0x80000008, &Eax, NULL, NULL, NULL);
> +    MaxPhyAddrBits = (UINT8)Eax;
> +  } else {
> +    AsmCpuid (1, NULL, NULL, NULL, &Edx);
> +    if ((Edx & BIT6) != 0) {
> +      MaxPhyAddrBits = 36;
> +    } else {
> +      MaxPhyAddrBits = 32;
> +    }
> +  }
> +
> +  ASSERT (MaxPhyAddrBits > 0);
> +
> +  AddressValid = FALSE;
> +
> +  //
> +  // check if CR4.PAE bit is not set
> +  //
> +  if ((Cr4 & BIT5) == 0) {
> +    //
> +    // Check if linear address is valid in 32-bit paging mode
> +    //
> +    AddressValid = Do32BitPagingModeCheck (Cr3, Cr4, LinearAddress);
> +  } else {
> +    if (MaxPhyAddrBits > 52) {
> +      return FALSE;
> +    }
> +
> +    Msr.Uint64 = AsmReadMsr64 (MSR_IA32_EFER);
> +
> +    if (Msr.Bits.LME == 0) {
> +      //
> +      // Check if linear address is valid in PAE paging mode
> +      //
> +      AddressValid = DoPAEPagingModeCheck (Cr3, MaxPhyAddrBits, 
> LinearAddress);
> +    } else {
> +      //
> +      // Check if linear address is valid in 4-level paging mode
> +      //
> +      AddressValid = Do4LevelPagingModeCheck (Cr3, MaxPhyAddrBits,
> +                                              LinearAddress);
> +    }
> +  }
> +
> +  return AddressValid;
> +}
> diff --git 
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h 
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> index ec46c2d9d3..1b51034c25 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> @@ -330,5 +330,21 @@ GetPdbFileName (
>     OUT CHAR8    **PdbFileName
>     );
> 
> +/**
> +  Check if a linear address is valid.
> +
> +  @param[in]  Cr0            CR0 control register.
> +  @param[in]  Cr3            CR3 control register.
> +  @param[in]  Cr4            CR4 control register.
> +  @param[in]  LinearAddress  Linear address to be checked.
> +**/
> +BOOLEAN
> +IsLinearAddressValid (
> +  IN  UINTN              Cr0,
> +  IN  UINTN              Cr3,
> +  IN  UINTN              Cr4,
> +  IN  UINTN              LinearAddress
> +  );
> +
>   #endif
> 
> -- 
> 2.14.3
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>