ArmPkg/ArmMmuLib ARM: fix page size granularity in initial MMU setting

From what I can see this bug dates back to the commit from 2011 where
support for this was added: 2cf4b60895f8a

The first problem is that PopulateLevel2PageTable overflows the
translation table buffer because it doesn't verify that the size
actually fits within one level 2 page table.

The second problem is that the loop in FillTranslationTable doesn't
care about the PhysicalBase or the RemainLength and always substracts
one section size from RemainLength.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael Zimmermann <sigmaepsilon92@gmail.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index b02f6d7fc590e923f3860f0de43b15d817c3e240..774a7ccf59c0f92f81fa4e8ff202aac815ae589e 100644 (file)
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -128,6 +128,7 @@ PopulateLevel2PageTable (
   UINT32  SectionDescriptor;\r
   UINT32  TranslationTable;\r
   UINT32  BaseSectionAddress;\r
+  UINT32  FirstPageOffset;\r
 \r
   switch (Attributes) {\r
     case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:\r
@@ -199,9 +200,12 @@ PopulateLevel2PageTable (
         TT_DESCRIPTOR_SECTION_TYPE_PAGE_TABLE;\r
   }\r
 \r
-  PageEntry = ((UINT32 *)(TranslationTable) + ((PhysicalBase & TT_DESCRIPTOR_PAGE_INDEX_MASK) >> TT_DESCRIPTOR_PAGE_BASE_SHIFT));\r
+  FirstPageOffset = (PhysicalBase & TT_DESCRIPTOR_PAGE_INDEX_MASK) >> TT_DESCRIPTOR_PAGE_BASE_SHIFT;\r
+  PageEntry = (UINT32 *)TranslationTable + FirstPageOffset;\r
   Pages     = RemainLength / TT_DESCRIPTOR_PAGE_SIZE;\r
 \r
+  ASSERT (FirstPageOffset + Pages <= TRANSLATION_TABLE_PAGE_COUNT);\r
+\r
   for (Index = 0; Index < Pages; Index++) {\r
     *PageEntry++     =  TT_DESCRIPTOR_PAGE_BASE_ADDRESS(PhysicalBase) | PageAttributes;\r
     PhysicalBase += TT_DESCRIPTOR_PAGE_SIZE;\r
@@ -220,6 +224,7 @@ FillTranslationTable (
   UINT32  Attributes;\r
   UINT32  PhysicalBase;\r
   UINT64  RemainLength;\r
+  UINT32  PageMapLength;\r
 \r
   ASSERT(MemoryRegion->Length > 0);\r
 \r
@@ -268,30 +273,31 @@ FillTranslationTable (
   SectionEntry    = TRANSLATION_TABLE_ENTRY_FOR_VIRTUAL_ADDRESS(TranslationTable, MemoryRegion->VirtualBase);\r
 \r
   while (RemainLength != 0) {\r
-    if (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE == 0) {\r
-      if (RemainLength >= TT_DESCRIPTOR_SECTION_SIZE) {\r
-        // Case: Physical address aligned on the Section Size (1MB) && the length is greater than the Section Size\r
-        *SectionEntry++ = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(PhysicalBase) | Attributes;\r
-        PhysicalBase += TT_DESCRIPTOR_SECTION_SIZE;\r
-      } else {\r
-        // Case: Physical address aligned on the Section Size (1MB) && the length does not fill a section\r
-        PopulateLevel2PageTable (SectionEntry++, PhysicalBase, RemainLength, MemoryRegion->Attributes);\r
-\r
-        // It must be the last entry\r
-        break;\r
-      }\r
+    if (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE == 0 &&\r
+        RemainLength >= TT_DESCRIPTOR_SECTION_SIZE) {\r
+      // Case: Physical address aligned on the Section Size (1MB) && the length\r
+      // is greater than the Section Size\r
+      *SectionEntry++ = TT_DESCRIPTOR_SECTION_BASE_ADDRESS(PhysicalBase) | Attributes;\r
+      PhysicalBase += TT_DESCRIPTOR_SECTION_SIZE;\r
+      RemainLength -= TT_DESCRIPTOR_SECTION_SIZE;\r
     } else {\r
+      PageMapLength = MIN (RemainLength, TT_DESCRIPTOR_SECTION_SIZE) -\r
+                      (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE);\r
+\r
+      // Case: Physical address aligned on the Section Size (1MB) && the length\r
+      //       does not fill a section\r
       // Case: Physical address NOT aligned on the Section Size (1MB)\r
-      PopulateLevel2PageTable (SectionEntry++, PhysicalBase, RemainLength, MemoryRegion->Attributes);\r
-      // Aligned the address\r
-      PhysicalBase = (PhysicalBase + TT_DESCRIPTOR_SECTION_SIZE) & ~(TT_DESCRIPTOR_SECTION_SIZE-1);\r
+      PopulateLevel2PageTable (SectionEntry++, PhysicalBase, PageMapLength,\r
+        MemoryRegion->Attributes);\r
 \r
       // If it is the last entry\r
       if (RemainLength < TT_DESCRIPTOR_SECTION_SIZE) {\r
         break;\r
       }\r
+\r
+      PhysicalBase += PageMapLength;\r
+      RemainLength -= PageMapLength;\r
     }\r
-    RemainLength -= TT_DESCRIPTOR_SECTION_SIZE;\r
   }\r
 }\r
 \r