From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c0c::241; helo=mail-wr0-x241.google.com; envelope-from=sigmaepsilon92@gmail.com; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x241.google.com (mail-wr0-x241.google.com [IPv6:2a00:1450:400c:c0c::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id F111821A1098A for ; Wed, 20 Dec 2017 11:24:14 -0800 (PST) Received: by mail-wr0-x241.google.com with SMTP id v21so12517172wrc.0 for ; Wed, 20 Dec 2017 11:29:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=GUe/q4PmQ5qaju2GbjwhRi2wpvO7RUuLC+UKh1mN5/Q=; b=USa2FJQr7O7fGXNW+vRmrLuwusLnHEUgzWjnrOnjlHLsB/LiEBp761trgAGh1G26UM nrDgNBHRdhpaB7LcXzB4ofeHjlKX+QbwTqdvHAQoVfUSd/Qaibahjvxai0vioSW1pvxq JUWAZHBtFXohuleXXK7/+wvKCUBqzDLr0YpYEJWO+3OmiK+xW2JkSnvjoYdQMcjw5PHZ tS0mJbooRXquXgNCY6YQg0TFoT3fuBnRidTMJv0e9UPSKxyAXAe9DtyBoydKCDt0atJ9 /R+BbseZpBQoPHire0y6TMm5H9A3MSjTR6vmk0yGzJJoxQgkh6PILxmJCNPq2/H3izAn cFcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=GUe/q4PmQ5qaju2GbjwhRi2wpvO7RUuLC+UKh1mN5/Q=; b=jbmt13cid2NpMTaAOiO3VJ8tyaue4fBWveORaHMS1adgXyp3HUF2Eowv9XXqcYghhv irV6rxptDVr+X2IfEFuWsh+dPZ/i66/DnpLaOzGoPFKrsZL0stWjMHhak2+xzVs4rGBU 49DMQs3lFZSKecwcoev8mx5f5qUvvuFTcKoxGUEgKXR59Pk4fPUI0/V6wlmAoTyB9a/3 15G5ZFZSvYFPmqLqiBlNjPWuKokfnMD8LGsgzBwIXyx1f1o2kwf05fdyONXKYFLBz+qN hoIzx4N9U/UWSV63lZRMZDysOdDldrJgTHNWawGPYUGtivbUTyJRxvt7zyzZtKANOIoe Q8/w== X-Gm-Message-State: AKGB3mKx/mrdf/X+8yxRE6wuELVtIJdzIlI2xo2s2/U2sinmkEkW+AcY J2LJ8of6m3TYLRBg36g8l+aUJUFP X-Google-Smtp-Source: ACJfBosLlg76CHW85yD5D8y8xtNNnhQqVGroft/6KVYS6nTL9WVapORqP7PMsYghjqdNCCfD/yNWjw== X-Received: by 10.223.193.4 with SMTP id r4mr7649598wre.151.1513798140774; Wed, 20 Dec 2017 11:29:00 -0800 (PST) Received: from localhost.localdomain ([2a02:908:5a9:8400:5ec8:3210:9b68:c91c]) by smtp.gmail.com with ESMTPSA id p6sm8826433wrd.78.2017.12.20.11.28.59 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 Dec 2017 11:28:59 -0800 (PST) From: M1cha To: edk2-devel@lists.01.org Cc: Ard Biesheuvel , Leif Lindholm Date: Wed, 20 Dec 2017 20:28:58 +0100 Message-Id: <20171220192858.1901-1-sigmaepsilon92@gmail.com> X-Mailer: git-send-email 2.15.1 Subject: [PATCH v2] ArmPkg/ArmMmuLib ARM: fix page size granularity in initial MMU setting X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Dec 2017 19:24:15 -0000 >>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 --- 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