linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/mm/cpa: Unbreak populate_pgd(): stop trying to deallocate failed PUDs
@ 2016-07-23  4:58 Andy Lutomirski
  2016-07-23  7:46 ` [tip:x86/mm] x86/mm/cpa: Fix populate_pgd(): Stop " tip-bot for Andy Lutomirski
  2016-07-23 19:15 ` tip-bot for Andy Lutomirski
  0 siblings, 2 replies; 6+ messages in thread
From: Andy Lutomirski @ 2016-07-23  4:58 UTC (permalink / raw)
  To: x86
  Cc: Borislav Petkov, linux-kernel, Brian Gerst, Andy Lutomirski,
	Mike Krinkin

This mostly reverts commit 360cb4d15567a7eca07a5f3ade6de308bbfb4e70.

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.

Cc: Mike Krinkin <krinkin.m.u@gmail.com>
Reported-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
Signed-off-by: Andy Lutomirski <luto@kernel.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 26c93c6e04a0..2bc6ea153f76 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;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-23 20:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-23  4:58 [PATCH] x86/mm/cpa: Unbreak populate_pgd(): stop trying to deallocate failed PUDs Andy Lutomirski
2016-07-23  7:46 ` [tip:x86/mm] x86/mm/cpa: Fix populate_pgd(): Stop " tip-bot for Andy Lutomirski
2016-07-23  8:32   ` Linus Torvalds
2016-07-23 19:14     ` Ingo Molnar
2016-07-23 19:15 ` tip-bot for Andy Lutomirski
2016-07-23 20:42   ` Nicolai Stange

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).