Commit-ID:  e959c4cb39feebe85e2b1191f5f666c79807631a
Gitweb:     http://git.kernel.org/tip/e959c4cb39feebe85e2b1191f5f666c79807631a
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Fri, 22 Jul 2016 21:58:08 -0700
Committer:  Ingo Molnar <[email protected]>
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 <[email protected]>
Signed-off-by: Andy Lutomirski <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Brian Gerst <[email protected]>
Cc: Denys Vlasenko <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Luis R. Rodriguez <[email protected]>
Cc: Mike Krinkin <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Toshi Kani <[email protected]>
Link: 
http://lkml.kernel.org/r/21cbc2822aa18aa812c0215f4231dbf5f65afa7f.1469249789.git.l...@kernel.org
Signed-off-by: Ingo Molnar <[email protected]>
---
 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;
 }

Reply via email to