Re: [edk2-devel] [PATCH] UefiCpuPkg:fix issue when splitting paging entry

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Friday, May 17, 2024 5:45 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>;
> Gerd Hoffmann <kraxel@redhat.com>; Wu, Jiaxin <jiaxin.wu@intel.com>;
> Zhou, Jianfeng <jianfeng.zhou@intel.com>
> Subject: [PATCH] UefiCpuPkg:fix issue when splitting paging entry
> 
> This patch is to fix issue when splitting leaf paging
> entry in CpuPageTableLib code.
> 
> In previous code, before we assign the new child paging
> structure address to the content of splitted paging entry,
> PageTableLibSetPnle() is called to make sure the bit7 is
> set to 0, which indicate the previous leaf entry is
> changed to non-leaf entry now. There is a gap between
> we change the bit7 and we assign the new child paging
> structure address to the content of the splitted paging
> entry. If the address of code execution or data access
> happens to be in the range covered by the splitted paging
> entry, this gap may cause issue.
> 
> In this patch, we prepare the new paging entry content
> value in a local variable and assign the value to the
> splitted paging entry at once. The volatile keyword
> is used to ensure that no optimization will occur in
> compilation.
> 
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Zhou Jianfeng <jianfeng.zhou@intel.com>
> ---
>  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index b10a3008e4..bdc411338f 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -342,6 +342,7 @@ PageTableLibMapInLevel (
>    UINT64              PhysicalAddrInAttr;
>    IA32_PAGING_ENTRY   OriginalParentPagingEntry;
>    IA32_PAGING_ENTRY   OriginalCurrentPagingEntry;
> +  IA32_PAGING_ENTRY   TempPagingEntry;
> 
>    ASSERT (Level != 0);
>    ASSERT ((Attribute != NULL) && (Mask != NULL));
> @@ -359,6 +360,8 @@ PageTableLibMapInLevel (
> 
>    OriginalParentPagingEntry.Uint64 = ParentPagingEntry->Uint64;
>    OneOfPagingEntry.Uint64          = 0;
> +  TempPagingEntry.Uint64           = 0;
> +
>    //
>    // RegionLength: 256T (1 << 48) 512G (1 << 39), 1G (1 << 30), 2M (1 << 21)
> or 4K (1 << 12).
>    //
> @@ -441,8 +444,10 @@ PageTableLibMapInLevel (
>        // Non-leaf entry doesn't have PAT bit. So use
> ~IA32_PE_BASE_ADDRESS_MASK_40 is to make sure PAT bit
>        // (bit12) in original big-leaf entry is not assigned to PageTableBaseAddress
> field of non-leaf entry.
>        //
> -      PageTableLibSetPnle (&ParentPagingEntry->Pnle, &NopAttribute,
> &AllOneMask);
> -      ParentPagingEntry->Uint64 = ((UINTN)(VOID *)PagingEntry) |
> (ParentPagingEntry->Uint64 & (~IA32_PE_BASE_ADDRESS_MASK_40));
> +      TempPagingEntry.Uint64 = ParentPagingEntry->Uint64;
> +      PageTableLibSetPnle (&TempPagingEntry.Pnle, &NopAttribute,
> &AllOneMask);
> +      TempPagingEntry.Uint64                           = ((UINTN)(VOID *)PagingEntry) |
> (TempPagingEntry.Uint64 & (~IA32_PE_BASE_ADDRESS_MASK_40));
> +      *(volatile UINT64 *)&(ParentPagingEntry->Uint64) =
> TempPagingEntry.Uint64;
>      }
>    } else {
>      //
> --
> 2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119183): https://edk2.groups.io/g/devel/message/119183
Mute This Topic: https://groups.io/mt/106150750/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-