1. Patchew
  2. EDK2
  3. [edk2] [PATCH 0/6] Fix issues caused by NX memory protection
  4. View patch

[edk2] [PATCH 3/6] UefiCpuPkg/CpuDxe: clear NX attr for page directory

Jian J Wang posted 6 patches 6 years, 11 months ago
[edk2] [PATCH 3/6] UefiCpuPkg/CpuDxe: clear NX attr for page directory
Posted by Jian J Wang 6 years, 11 months ago 
If PcdDxeNxMemoryProtectionPolicy is set to enable protection for memory
of EfiBootServicesCode, EfiConventionalMemory and EfiReservedMemoryType,
the BIOS will hang at a page fault exception randomly.

The root cause is that the memory allocation for driver images (actually
a memory type conversion from free memory, type of EfiConventionalMemory,
to code memory, type of EfiBootServicesCode/EfiRuntimeServicesCode)
will get memory with NX set, because the CpuDxe driver will keep the NX
attribute (with free memory) in page directory during page table splitting
and then override the NX attribute of all its entries.

This patch fixes this issue by not inheriting NX attribute when turning
a page entry into a page directory during page granularity split.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index a9c9bc9d5e..1654e71103 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -528,7 +528,7 @@ SplitPage (
       for (Index = 0; Index < SIZE_4KB / sizeof(UINT64); Index++) {
         NewPageEntry[Index] = (BaseAddress + SIZE_4KB * Index) | AddressEncMask | ((*PageEntry) & PAGE_PROGATE_BITS);
       }
-      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask | ((*PageEntry) & PAGE_PROGATE_BITS);
+      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask | ((*PageEntry) & PAGE_ATTRIBUTE_BITS);
       return RETURN_SUCCESS;
     } else {
       return RETURN_UNSUPPORTED;
@@ -549,7 +549,7 @@ SplitPage (
       for (Index = 0; Index < SIZE_4KB / sizeof(UINT64); Index++) {
         NewPageEntry[Index] = (BaseAddress + SIZE_2MB * Index) | AddressEncMask | IA32_PG_PS | ((*PageEntry) & PAGE_PROGATE_BITS);
       }
-      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask | ((*PageEntry) & PAGE_PROGATE_BITS);
+      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask | ((*PageEntry) & PAGE_ATTRIBUTE_BITS);
       return RETURN_SUCCESS;
     } else {
       return RETURN_UNSUPPORTED;
@@ -979,7 +979,7 @@ RefreshGcdMemoryAttributesFromPaging (
                         );
         ASSERT_EFI_ERROR (Status);
         DEBUG ((
-          DEBUG_INFO,
+          DEBUG_VERBOSE,
           "Updated memory space attribute: [%lu] %016lx - %016lx (%016lx -> %016lx)\r\n",
           (UINT64)Index, BaseAddress, BaseAddress + Length - 1,
           MemorySpaceMap[Index].Attributes,
-- 
2.15.1.windows.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 3/6] UefiCpuPkg/CpuDxe: clear NX attr for page directory
Posted by Dong, Eric 6 years, 11 months ago 
Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jian J Wang
> Sent: Monday, January 15, 2018 4:55 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: [edk2] [PATCH 3/6] UefiCpuPkg/CpuDxe: clear NX attr for page
> directory
> 
> If PcdDxeNxMemoryProtectionPolicy is set to enable protection for memory
> of EfiBootServicesCode, EfiConventionalMemory and
> EfiReservedMemoryType, the BIOS will hang at a page fault exception
> randomly.
> 
> The root cause is that the memory allocation for driver images (actually a
> memory type conversion from free memory, type of EfiConventionalMemory,
> to code memory, type of EfiBootServicesCode/EfiRuntimeServicesCode)
> will get memory with NX set, because the CpuDxe driver will keep the NX
> attribute (with free memory) in page directory during page table splitting and
> then override the NX attribute of all its entries.
> 
> This patch fixes this issue by not inheriting NX attribute when turning a page
> entry into a page directory during page granularity split.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index a9c9bc9d5e..1654e71103 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -528,7 +528,7 @@ SplitPage (
>        for (Index = 0; Index < SIZE_4KB / sizeof(UINT64); Index++) {
>          NewPageEntry[Index] = (BaseAddress + SIZE_4KB * Index) |
> AddressEncMask | ((*PageEntry) & PAGE_PROGATE_BITS);
>        }
> -      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> ((*PageEntry) & PAGE_PROGATE_BITS);
> +      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> + ((*PageEntry) & PAGE_ATTRIBUTE_BITS);
>        return RETURN_SUCCESS;
>      } else {
>        return RETURN_UNSUPPORTED;
> @@ -549,7 +549,7 @@ SplitPage (
>        for (Index = 0; Index < SIZE_4KB / sizeof(UINT64); Index++) {
>          NewPageEntry[Index] = (BaseAddress + SIZE_2MB * Index) |
> AddressEncMask | IA32_PG_PS | ((*PageEntry) & PAGE_PROGATE_BITS);
>        }
> -      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> ((*PageEntry) & PAGE_PROGATE_BITS);
> +      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> + ((*PageEntry) & PAGE_ATTRIBUTE_BITS);
>        return RETURN_SUCCESS;
>      } else {
>        return RETURN_UNSUPPORTED;
> @@ -979,7 +979,7 @@ RefreshGcdMemoryAttributesFromPaging (
>                          );
>          ASSERT_EFI_ERROR (Status);
>          DEBUG ((
> -          DEBUG_INFO,
> +          DEBUG_VERBOSE,
>            "Updated memory space attribute: [%lu] %016lx - %016lx (%016lx -
> > %016lx)\r\n",
>            (UINT64)Index, BaseAddress, BaseAddress + Length - 1,
>            MemorySpaceMap[Index].Attributes,
> --
> 2.15.1.windows.2
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.