Re: [PATCH] MdeModulePkg/PiSmmCore: fix #PF caused by freeing read-only memory

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 3/15/2018 2:22 PM, Jian J Wang wrote:
> SMM core will add a HEADER before each allocated pool memory and clean
> up this header once it's freed. If a block of allocated pool is marked
> as read-only after allocation (EfiRuntimeServicesCode type of pool in
> SMM will always be marked as read-only), #PF exception will be triggered
> during memory pool freeing.
> 
> Normally EfiRuntimeServicesCode type of pool should not be freed in the
> real world. But some test suites will actually do memory free for all
> types of memory for the purpose of functionality and conformance test.
> So this issue should be fixed anyway.

Does DxeCore have such problem?

> 
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>   MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> index f9657f9baa..d5556eb79c 100644
> --- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> +++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> @@ -941,6 +941,7 @@ AdjustMemoryF (
>     EFI_PHYSICAL_ADDRESS  MemoryToTest;
>     UINTN                 PagesToFree;
>     UINT64                GuardBitmap;
> +  UINT64                Attributes;
>   
>     if (Memory == NULL || NumberOfPages == NULL || *NumberOfPages == 0) {
>       return;
> @@ -949,6 +950,27 @@ AdjustMemoryF (
>     Start = *Memory;
>     PagesToFree = *NumberOfPages;
>   
> +  //
> +  // In case the memory to free is marked as read-only (e.g. EfiRuntimeServicesCode).
> +  //
> +  if (mSmmMemoryAttribute != NULL) {
> +    Attributes = 0;
> +    mSmmMemoryAttribute->GetMemoryAttributes (
> +                           mSmmMemoryAttribute,
> +                           Start,
> +                           EFI_PAGES_TO_SIZE (PagesToFree),
> +                           &Attributes
> +                           );
> +    if ((Attributes & EFI_MEMORY_RO) != 0) {
> +      mSmmMemoryAttribute->ClearMemoryAttributes (
> +                             mSmmMemoryAttribute,
> +                             Start,
> +                             EFI_PAGES_TO_SIZE (PagesToFree),
> +                             EFI_MEMORY_RO
> +                             );
> +    }
> +  }
> +
>     //
>     // Head Guard must be one page before, if any.
>     //
> 


-- 
Thanks,
Ray