Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Yuntao Wang <ytcoode@gmail.com>
To:     Dave Hansen <dave.hansen@linux.intel.com>,
        Andy Lutomirski <luto@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>
Cc:     "H. Peter Anvin" <hpa@zytor.com>, Baoquan He <bhe@redhat.com>,
        "Kirill A. Shutemov" <kirill@shutemov.name>, x86@kernel.org,
        linux-kernel@vger.kernel.org, Yuntao Wang <ytcoode@gmail.com>,
        stable@vger.kernel.org
Subject: [PATCH] x86/mm: Fix possible index overflow when creating page table mapping
Date:   Thu, 16 Jun 2022 21:55:10 +0800
Message-Id: <20220616135510.1784995-1-ytcoode@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

There are two issues in phys_p4d_init():

- The __kernel_physical_mapping_init() does not do boundary-checking for
  paddr_end and passes it directly to phys_p4d_init(), phys_p4d_init() does
  not do bounds checking either, so if the physical memory to be mapped is
  large enough, 'p4d_page + p4d_index(vaddr)' will wrap around to the
  beginning entry of the P4D table and its data will be overwritten.

- The for loop body will be executed only when 'vaddr < vaddr_end'
  evaluates to true, but if that condition is true, 'paddr >= paddr_end'
  will evaluate to false, thus the 'if (paddr >= paddr_end) {}' block will
  never be executed and become dead code.

To fix these issues, use 'i < PTRS_PER_P4D' instead of 'vaddr < vaddr_end'
as the for loop condition, this also make it more consistent with the logic
of the phys_{pud,pmt,pte}_init() functions.

Fixes: 432c833218dd ("x86/mm: Handle physical-virtual alignment mismatch in phys_p4d_init()")
Cc: stable@vger.kernel.org
Signed-off-by: Yuntao Wang <ytcoode@gmail.com>
---
 arch/x86/mm/init_64.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 8779d6be6a49..e718c9b3f539 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -674,18 +674,18 @@ static unsigned long __meminit
 phys_p4d_init(p4d_t *p4d_page, unsigned long paddr, unsigned long paddr_end,
 	      unsigned long page_size_mask, pgprot_t prot, bool init)
 {
-	unsigned long vaddr, vaddr_end, vaddr_next, paddr_next, paddr_last;
-
-	paddr_last = paddr_end;
-	vaddr = (unsigned long)__va(paddr);
-	vaddr_end = (unsigned long)__va(paddr_end);
+	unsigned long vaddr, vaddr_next, paddr_next, paddr_last;
+	int i;
 
 	if (!pgtable_l5_enabled())
 		return phys_pud_init((pud_t *) p4d_page, paddr, paddr_end,
 				     page_size_mask, prot, init);
 
-	for (; vaddr < vaddr_end; vaddr = vaddr_next) {
-		p4d_t *p4d = p4d_page + p4d_index(vaddr);
+	paddr_last = paddr_end;
+	vaddr = (unsigned long)__va(paddr);
+
+	for (i = p4d_index(vaddr); i < PTRS_PER_P4D; i++, vaddr = vaddr_next) {
+		p4d_t *p4d = p4d_page + i;
 		pud_t *pud;
 
 		vaddr_next = (vaddr & P4D_MASK) + P4D_SIZE;
@@ -704,13 +704,13 @@ phys_p4d_init(p4d_t *p4d_page, unsigned long paddr, unsigned long paddr_end,
 
 		if (!p4d_none(*p4d)) {
 			pud = pud_offset(p4d, 0);
-			paddr_last = phys_pud_init(pud, paddr, __pa(vaddr_end),
+			paddr_last = phys_pud_init(pud, paddr, paddr_end,
 					page_size_mask, prot, init);
 			continue;
 		}
 
 		pud = alloc_low_page();
-		paddr_last = phys_pud_init(pud, paddr, __pa(vaddr_end),
+		paddr_last = phys_pud_init(pud, paddr, paddr_end,
 					   page_size_mask, prot, init);
 
 		spin_lock(&init_mm.page_table_lock);
-- 
2.36.0