ARM/VExpressPkg: Set EFI_MEMORY_XP flag on GOP framebuffer

The framebuffer memory is set with flag
EFI_MEMORY_WC (uncached, unbuffered) which causes framebuffer memory
with eXecute bit set. Framebuffer memory having executable bit
set is a security hazard. This fix adds EFI_MEMORY_XP flag to avoid this.
Unfortunately function gDS->SetMemorySpaceAttributes() causes assertion due
to unsupported EFI_MEMORY_XP type. Therefore this fix replaces
gDS->SetMemorySpaceAttributes() with Cpu->SetMemoryAttributes().

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Girish Pathak <girish.pathak@arm.com>
Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

4 files changed, 34 insertions, 16 deletions

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
index f1c497f4b3..711f036d74 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c
@@ -17,11 +17,11 @@
 #include <Library/ArmPlatformSysConfigLib.h>

 #include <Library/PcdLib.h>

 #include <Library/DebugLib.h>

-#include <Library/DxeServicesTableLib.h>

 #include <Library/LcdPlatformLib.h>

 #include <Library/MemoryAllocationLib.h>

 #include <Library/UefiBootServicesTableLib.h>

 

+#include <Protocol/Cpu.h>

 #include <Protocol/EdidDiscovered.h>

 #include <Protocol/EdidActive.h>

 

@@ -159,6 +159,7 @@ LcdPlatformGetVram (
 {

   EFI_STATUS              Status;

   EFI_ALLOCATE_TYPE       AllocationType;

+  EFI_CPU_ARCH_PROTOCOL   *Cpu;

 

   ASSERT (VramBaseAddress != NULL);

   ASSERT (VramSize != NULL);

@@ -185,13 +186,22 @@ LcdPlatformGetVram (
     return Status;

   }

 

-  // Mark the VRAM as write-combining.

-  // The VRAM is inside the DRAM, which is cacheable.

-  Status = gDS->SetMemorySpaceAttributes (

-                  *VramBaseAddress,

-                  *VramSize,

-                  EFI_MEMORY_WC

+  // Ensure the Cpu architectural protocol is already installed

+  Status = gBS->LocateProtocol (

+                  &gEfiCpuArchProtocolGuid,

+                  NULL,

+                  (VOID **)&Cpu

                   );

+  if (!EFI_ERROR (Status)) {

+    // The VRAM is inside the DRAM, which is cacheable.

+    // Mark the VRAM as write-combining (uncached) and non-executable.

+    Status = Cpu->SetMemoryAttributes (

+                    Cpu,

+                    *VramBaseAddress,

+                    *VramSize,

+                    EFI_MEMORY_WC | EFI_MEMORY_XP

+                    );

+  }

   if (EFI_ERROR (Status)) {

     ASSERT_EFI_ERROR (Status);

     gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));

diff --git a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
index 9b0d358846..c7b1b7fae7 100644
--- a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
@@ -33,7 +33,6 @@
 [LibraryClasses]

   ArmPlatformSysConfigLib

   BaseLib

-  DxeServicesTableLib

 

 [Protocols]

   gEfiEdidDiscoveredProtocolGuid                # Produced

diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
index 50a53d3fff..bcf4f6593c 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c
@@ -17,10 +17,10 @@
 #include <Library/IoLib.h>

 #include <Library/PcdLib.h>

 #include <Library/DebugLib.h>

-#include <Library/DxeServicesTableLib.h>

 #include <Library/LcdPlatformLib.h>

 #include <Library/UefiBootServicesTableLib.h>

 

+#include <Protocol/Cpu.h>

 #include <Protocol/EdidDiscovered.h>

 #include <Protocol/EdidActive.h>

 

@@ -212,6 +212,7 @@ LcdPlatformGetVram (
   )

 {

   EFI_STATUS              Status;

+  EFI_CPU_ARCH_PROTOCOL   *Cpu;

 

   ASSERT (VramBaseAddress != NULL);

   ASSERT (VramSize != NULL);

@@ -241,13 +242,22 @@ LcdPlatformGetVram (
       return Status;

     }

 

-    // Mark the VRAM as write-combining.

-    // The VRAM is inside the DRAM, which is cacheable.

-    Status = gDS->SetMemorySpaceAttributes (

-                    *VramBaseAddress,

-                    *VramSize,

-                    EFI_MEMORY_WC

+    // Ensure the Cpu architectural protocol is already installed

+    Status = gBS->LocateProtocol (

+                    &gEfiCpuArchProtocolGuid,

+                    NULL,

+                    (VOID **)&Cpu

                     );

+    if (!EFI_ERROR (Status)) {

+      // The VRAM is inside the DRAM, which is cacheable.

+      // Mark the VRAM as write-combining (uncached) and non-executable.

+      Status = Cpu->SetMemoryAttributes (

+                      Cpu,

+                      *VramBaseAddress,

+                      *VramSize,

+                      EFI_MEMORY_WC | EFI_MEMORY_XP

+                      );

+    }

     if (EFI_ERROR (Status)) {

       ASSERT_EFI_ERROR (Status);

       gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize));

diff --git a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
index 2bf14f999e..b1fa100def 100644
--- a/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
+++ b/Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf
@@ -33,7 +33,6 @@
 [LibraryClasses]

   ArmPlatformSysConfigLib

   BaseLib

-  DxeServicesTableLib

 

 [Protocols]

   gEfiEdidDiscoveredProtocolGuid                # Produced