Re: [PATCH v2 3/4] ArmPkg/ArmMmuLib ARM: implement memory permission control routines

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Mar 07, 2017 at 09:42:04AM +0100, Ard Biesheuvel wrote:
> Now that we have the prerequisite functionality available in ArmMmuLib,
> wire it up into ArmSetMemoryRegionNoExec, ArmClearMemoryRegionNoExec,
> ArmSetMemoryRegionReadOnly and ArmClearMemoryRegionReadOnly. This is
> used by the non-executable stack feature that is configured by DxeIpl.
> 
> NOTE: The current implementation will not combine RO and XP attributes,
>       i.e., setting/clearing a region no-exec will unconditionally
>       clear the read-only attribute, and vice versa. Currently, we
>       only use ArmSetMemoryRegionNoExec(), so for now, we should be
>       able to live with this.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> index 351b6c03a42c..b02f6d7fc590 100644
> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> @@ -37,6 +37,8 @@
>  #define ID_MMFR0_SHR_IMP_HW_COHERENT   1
>  #define ID_MMFR0_SHR_IGNORED         0xf
>  
> +#define __EFI_MEMORY_RWX               0    // no restrictions
> +
>  #define CACHE_ATTRIBUTE_MASK   (EFI_MEMORY_UC | \
>                                  EFI_MEMORY_WC | \
>                                  EFI_MEMORY_WT | \
> @@ -797,7 +799,7 @@ ArmSetMemoryRegionNoExec (
>    IN  UINT64                    Length
>    )
>  {
> -  return EFI_UNSUPPORTED;
> +  return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_XP);
>  }
>  
>  EFI_STATUS
> @@ -806,7 +808,7 @@ ArmClearMemoryRegionNoExec (
>    IN  UINT64                    Length
>    )
>  {
> -  return EFI_UNSUPPORTED;
> +  return ArmSetMemoryAttributes (BaseAddress, Length, __EFI_MEMORY_RWX);
>  }
>  
>  EFI_STATUS
> @@ -815,7 +817,7 @@ ArmSetMemoryRegionReadOnly (
>    IN  UINT64                    Length
>    )
>  {
> -  return EFI_UNSUPPORTED;
> +  return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_RO);
>  }
>  
>  EFI_STATUS
> @@ -824,7 +826,7 @@ ArmClearMemoryRegionReadOnly (
>    IN  UINT64                    Length
>    )
>  {
> -  return EFI_UNSUPPORTED;
> +  return ArmSetMemoryAttributes (BaseAddress, Length, __EFI_MEMORY_RWX);
>  }
>  
>  RETURN_STATUS
> -- 
> 2.7.4
>