linux-kernel - [tip:x86/mm] x86/mm/cpa: Fix populate_pgd(): Stop trying to deallocate failed PUDs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 23 Jul 2016 00:46:26 -0700
From:	tip-bot for Andy Lutomirski <tipbot@...or.com>
To:	linux-tip-commits@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org, peterz@...radead.org,
	torvalds@...ux-foundation.org, dvlasenk@...hat.com, bp@...en8.de,
	toshi.kani@...com, jpoimboe@...hat.com, akpm@...ux-foundation.org,
	brgerst@...il.com, mcgrof@...e.com, krinkin.m.u@...il.com,
	hpa@...or.com, luto@...nel.org, mingo@...nel.org,
	tglx@...utronix.de, Valdis.Kletnieks@...edu
Subject: [tip:x86/mm] x86/mm/cpa: Fix populate_pgd(): Stop trying to
 deallocate failed PUDs

Commit-ID:  e959c4cb39feebe85e2b1191f5f666c79807631a
Gitweb:     http://git.kernel.org/tip/e959c4cb39feebe85e2b1191f5f666c79807631a
Author:     Andy Lutomirski <luto@...nel.org>
AuthorDate: Fri, 22 Jul 2016 21:58:08 -0700
Committer:  Ingo Molnar <mingo@...nel.org>
CommitDate: Sat, 23 Jul 2016 09:43:42 +0200

x86/mm/cpa: Fix populate_pgd(): Stop trying to deallocate failed PUDs

Valdis Kletnieks bisected a boot failure back to this recent commit:

  360cb4d15567 ("x86/mm/cpa: In populate_pgd(), don't set the PGD entry until it's populated")

I broke the case where a PUD table got allocated -- populate_pud()
would wander off a pgd_none entry and get lost.  I'm not sure how
this survived my testing.

Fixing this directly is difficult or impossible because of the awful
state of Linux's page table accessors.

Instead, fix the original issue in a much simpler way.  The problem
was that, if we allocated a PUD table, failed to populate it, and
freed it, another CPU could potentially keep using the PGD entry we
installed (either by copying it via vmalloc_fault or by speculatively
caching it).  There's a straightforward fix: simply leave the
top-level entry in place if this happens.  This can't waste any
significant amount of memory -- there are at most 256 entries like
this systemwide and, as a practical matter, if we hit this failure
path repeatedly, we're likely to reuse the same page anyway.

For context, this is a reversion with this hunk added in:

	if (ret < 0) {
+		/*
+		 * Leave the PUD page in place in case some other CPU or thread
+		 * already found it, but remove any useless entries we just
+		 * added to it.
+		 */
-		unmap_pgd_range(cpa->pgd, addr,
+		unmap_pud_range(pgd_entry, addr,
			        addr + (cpa->numpages << PAGE_SHIFT));
		return ret;
	}

This effectively open-codes what the now-deleted unmap_pgd_range()
function used to do except that unmap_pgd_range() used to try to
free the page as well.

Reported-by: Valdis Kletnieks <Valdis.Kletnieks@...edu>
Signed-off-by: Andy Lutomirski <luto@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Borislav Petkov <bp@...en8.de>
Cc: Brian Gerst <brgerst@...il.com>
Cc: Denys Vlasenko <dvlasenk@...hat.com>
Cc: H. Peter Anvin <hpa@...or.com>
Cc: Josh Poimboeuf <jpoimboe@...hat.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Luis R. Rodriguez <mcgrof@...e.com>
Cc: Mike Krinkin <krinkin.m.u@...il.com>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Toshi Kani <toshi.kani@...com>
Link: http://lkml.kernel.org/r/21cbc2822aa18aa812c0215f4231dbf5f65afa7f.1469249789.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@...nel.org>
---
 arch/x86/mm/pageattr.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 26c93c6..2bc6ea1 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -1082,6 +1082,8 @@ static int populate_pgd(struct cpa_data *cpa, unsigned long addr)
 		pud = (pud_t *)get_zeroed_page(GFP_KERNEL | __GFP_NOTRACK);
 		if (!pud)
 			return -1;
+
+		set_pgd(pgd_entry, __pgd(__pa(pud) | _KERNPG_TABLE));
 	}
 
 	pgprot_val(pgprot) &= ~pgprot_val(cpa->mask_clr);
@@ -1089,16 +1091,11 @@ static int populate_pgd(struct cpa_data *cpa, unsigned long addr)
 
 	ret = populate_pud(cpa, addr, pgd_entry, pgprot);
 	if (ret < 0) {
-		if (pud)
-			free_page((unsigned long)pud);
 		unmap_pud_range(pgd_entry, addr,
 				addr + (cpa->numpages << PAGE_SHIFT));
 		return ret;
 	}
 
-	if (pud)
-		set_pgd(pgd_entry, __pgd(__pa(pud) | _KERNPG_TABLE));
-
 	cpa->numpages = ret;
 	return 0;
 }