The section-to-page attribute conversion takes the shareability and
execute-never attributes into account, whereas the page-to-section
counterpart does not. The result is that GetMemoryRegionPage () -which
takes a section attribute argument (via *RegionAttributes) that is
ostensibly based on the first page in the range, but differs from the
actual page attributes when converted back- may return with a
RegionLength of zero. This is incorrect, and confuses code that scans a
region by calling GetMemoryRegion () in sequence.

So fix the conversion, and ASSERT () on a non-zero region length.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmPkg/Drivers/CpuDxe/Arm/Mmu.c   | 3 +++
 ArmPkg/Include/Chipset/ArmV7Mmu.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
index ea856f5cdd26..8eb1f71395f5 100644
--- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
@@ -480,6 +480,8 @@ GetMemoryRegion (
 
     PageAttributes    = PageTable[PageTableIndex] & TT_DESCRIPTOR_PAGE_ATTRIBUTE_MASK;
     *RegionAttributes = TT_DESCRIPTOR_CONVERT_TO_SECTION_CACHE_POLICY (PageAttributes) |
+                        TT_DESCRIPTOR_CONVERT_TO_SECTION_S (PageAttributes) |
+                        TT_DESCRIPTOR_CONVERT_TO_SECTION_XN (PageAttributes) |
                         TT_DESCRIPTOR_CONVERT_TO_SECTION_AP (PageAttributes);
   }
 
@@ -494,6 +496,7 @@ GetMemoryRegion (
 
       // Scan the page table to find the end of the region.
       Status = GetMemoryRegionPage (PageTable, BaseAddress, RegionLength, RegionAttributes);
+      ASSERT (*RegionLength > 0);
 
       // If we have found the end of the region (Status == EFI_SUCCESS) then we exit the for-loop
       if (Status == EFI_SUCCESS) {
diff --git a/ArmPkg/Include/Chipset/ArmV7Mmu.h b/ArmPkg/Include/Chipset/ArmV7Mmu.h
index 6a2584ceb303..e0219747df86 100644
--- a/ArmPkg/Include/Chipset/ArmV7Mmu.h
+++ b/ArmPkg/Include/Chipset/ArmV7Mmu.h
@@ -128,6 +128,8 @@
 #define TT_DESCRIPTOR_CONVERT_TO_PAGE_CACHE_POLICY(Desc)  ((((Desc) & (0x3 << 12)) >> 6) | (Desc & (0x3 << 2)))
 
 #define TT_DESCRIPTOR_CONVERT_TO_SECTION_AP(Desc)            ((((Desc) & TT_DESCRIPTOR_PAGE_AP_MASK) << 6) & TT_DESCRIPTOR_SECTION_AP_MASK)
+#define TT_DESCRIPTOR_CONVERT_TO_SECTION_S(Desc)             ((((Desc) & TT_DESCRIPTOR_PAGE_S_MASK) << 6) & TT_DESCRIPTOR_SECTION_S_MASK)
+#define TT_DESCRIPTOR_CONVERT_TO_SECTION_XN(Desc)            ((((Desc) & TT_DESCRIPTOR_PAGE_XN_MASK) << 4) & TT_DESCRIPTOR_SECTION_XN_MASK)
 #define TT_DESCRIPTOR_CONVERT_TO_SECTION_CACHE_POLICY(Desc)  ((((Desc) & (0x3 << 6)) << 6) | (Desc & (0x3 << 2)))
 
 #define TT_DESCRIPTOR_SECTION_ATTRIBUTE_MASK  (TT_DESCRIPTOR_SECTION_NS_MASK | TT_DESCRIPTOR_SECTION_NG_MASK |               \
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101106): https://edk2.groups.io/g/devel/message/101106
Mute This Topic: https://groups.io/mt/97585983/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-