[edk2,4/5] ArmPkg/ArmMmuLib ARM: implement memory permission control routines

Message ID 1488385903-30267-5-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • ArmPkg, ArmVirtPkg ARM: enable non-executable stack
Related show

Commit Message

Ard Biesheuvel March 1, 2017, 4:31 p.m.
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>

---
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm March 6, 2017, 4:11 p.m. | #1
On Wed, Mar 01, 2017 at 04:31:42PM +0000, 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>

> ---

>  ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 16 ++++++++--------

>  1 file changed, 8 insertions(+), 8 deletions(-)

> 

> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c

> index 1112660b434e..55601328d93e 100644

> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c

> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c

> @@ -761,40 +761,40 @@ ArmSetMemoryAttributes (

>    return Status;

>  }

>  

> -RETURN_STATUS

> +EFI_STATUS


Could these RETURN_->EFI_ fixes be folded into 1/5 instead (if you've
not already pushed it by the time you get here)?

>  ArmSetMemoryRegionNoExec (

>    IN  EFI_PHYSICAL_ADDRESS      BaseAddress,

>    IN  UINT64                    Length

>    )

>  {

> -  return RETURN_UNSUPPORTED;

> +  return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_XP);

>  }

>  

> -RETURN_STATUS

> +EFI_STATUS

>  ArmClearMemoryRegionNoExec (

>    IN  EFI_PHYSICAL_ADDRESS      BaseAddress,

>    IN  UINT64                    Length

>    )

>  {

> -  return RETURN_UNSUPPORTED;

> +  return ArmSetMemoryAttributes (BaseAddress, Length, 0);


I'd be slightly happier if there was a #define for that 0, throughout.
Alternatively, replace with a macro called ArmClearMemoryAttributes().

/
    Leif

>  }

>  

> -RETURN_STATUS

> +EFI_STATUS

>  ArmSetMemoryRegionReadOnly (

>    IN  EFI_PHYSICAL_ADDRESS      BaseAddress,

>    IN  UINT64                    Length

>    )

>  {

> -  return RETURN_UNSUPPORTED;

> +  return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_RO);

>  }

>  

> -RETURN_STATUS

> +EFI_STATUS

>  ArmClearMemoryRegionReadOnly (

>    IN  EFI_PHYSICAL_ADDRESS      BaseAddress,

>    IN  UINT64                    Length

>    )

>  {

> -  return RETURN_UNSUPPORTED;

> +  return ArmSetMemoryAttributes (BaseAddress, Length, 0);

>  }

>  

>  RETURN_STATUS

> -- 

> 2.7.4

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch hide | download patch | download mbox

diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index 1112660b434e..55601328d93e 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -761,40 +761,40 @@  ArmSetMemoryAttributes (
   return Status;
 }
 
-RETURN_STATUS
+EFI_STATUS
 ArmSetMemoryRegionNoExec (
   IN  EFI_PHYSICAL_ADDRESS      BaseAddress,
   IN  UINT64                    Length
   )
 {
-  return RETURN_UNSUPPORTED;
+  return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_XP);
 }
 
-RETURN_STATUS
+EFI_STATUS
 ArmClearMemoryRegionNoExec (
   IN  EFI_PHYSICAL_ADDRESS      BaseAddress,
   IN  UINT64                    Length
   )
 {
-  return RETURN_UNSUPPORTED;
+  return ArmSetMemoryAttributes (BaseAddress, Length, 0);
 }
 
-RETURN_STATUS
+EFI_STATUS
 ArmSetMemoryRegionReadOnly (
   IN  EFI_PHYSICAL_ADDRESS      BaseAddress,
   IN  UINT64                    Length
   )
 {
-  return RETURN_UNSUPPORTED;
+  return ArmSetMemoryAttributes (BaseAddress, Length, EFI_MEMORY_RO);
 }
 
-RETURN_STATUS
+EFI_STATUS
 ArmClearMemoryRegionReadOnly (
   IN  EFI_PHYSICAL_ADDRESS      BaseAddress,
   IN  UINT64                    Length
   )
 {
-  return RETURN_UNSUPPORTED;
+  return ArmSetMemoryAttributes (BaseAddress, Length, 0);
 }
 
 RETURN_STATUS