public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2] ArmPkg/ArmMmuLib ARM: fix page size granularity in initial MMU setting
@ 2017-12-20 19:28 M1cha
  2017-12-20 21:31 ` Ard Biesheuvel
  0 siblings, 1 reply; 2+ messages in thread
From: M1cha @ 2017-12-20 19:28 UTC (permalink / raw)
  To: edk2-devel; +Cc: Ard Biesheuvel, Leif Lindholm

>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>
---

v2: implement changes as requested by Ard.

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

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



^ permalink raw reply related	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-12-20 21:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-20 19:28 [PATCH v2] ArmPkg/ArmMmuLib ARM: fix page size granularity in initial MMU setting M1cha
2017-12-20 21:31 ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox