[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1469182720-6207-1-git-send-email-krinkin.m.u@gmail.com>
Date: Fri, 22 Jul 2016 13:18:40 +0300
From: Mike Krinkin <krinkin.m.u@...il.com>
To: luto@...nel.org,
Valdis.Kletnieks@...edu,
kernel-hardening@...ts.openwall.com
Cc: linux-arch@...r.kernel.org,
bp@...en8.de,
nadav.amit@...il.com,
keescook@...omium.org,
brgerst@...il.com,
torvalds@...ux-foundation.org,
jpoimboe@...hat.com,
jann@...jh.net,
heiko.carstens@...ibm.com,
Mike Krinkin <krinkin.m.u@...il.com>
Subject: Re: [PATCH v5 03/32] x86/cpa: In populate_pgd, don't set the pgd entry until it's populated
Hi, the patch solves problem for me, but i have one question below
> --Andy
> From 6589ddf69a1369e1ecb95f0af489d90b980e256e Mon Sep 17 00:00:00 2001
> Message-Id: <6589ddf69a1369e1ecb95f0af489d90b980e256e.1469165371.git.luto@...nel.org>
> From: Andy Lutomirski <luto@...nel.org>
> Date: Thu, 21 Jul 2016 22:22:02 -0700
> Subject: [PATCH] x86/mm: Fix populate_pgd()
>
> I make an obvious error in populate_pgd() -- it would fail to correctly
> populate the page tables when it allocated a new pud page.
>
> Fixes: 360cb4d15567 ("x86/mm/cpa: In populate_pgd(), don't set the PGD entry until it's populated")
> Reported-by: Valdis Kletnieks <Valdis.Kletnieks@...edu>
> Signed-off-by: Andy Lutomirski <luto@...nel.org>
> ---
> arch/x86/mm/pageattr.c | 28 +++++++++++++++-------------
> 1 file changed, 15 insertions(+), 13 deletions(-)
> diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
> index 26c93c6e04a0..5ee7d1c794a4 100644
> --- a/arch/x86/mm/pageattr.c
> +++ b/arch/x86/mm/pageattr.c
> @@ -984,8 +984,8 @@ static int populate_pmd(struct cpa_data *cpa,
> return num_pages;
> }
>
> -static int populate_pud(struct cpa_data *cpa, unsigned long start, pgd_t *pgd,
> - pgprot_t pgprot)
> +static int populate_pud(struct cpa_data *cpa, unsigned long start,
> + pud_t *pud_page, pgprot_t pgprot)
> {
> pud_t *pud;
> unsigned long end;
> @@ -1006,7 +1006,7 @@ static int populate_pud(struct cpa_data *cpa, unsigned long start, pgd_t *pgd,
> cur_pages = (pre_end - start) >> PAGE_SHIFT;
> cur_pages = min_t(int, (int)cpa->numpages, cur_pages);
>
> - pud = pud_offset(pgd, start);
> + pud = pud_page + pud_index(start);
>
> /*
> * Need a PMD page?
> @@ -1027,7 +1027,7 @@ static int populate_pud(struct cpa_data *cpa, unsigned long start, pgd_t *pgd,
> if (cpa->numpages == cur_pages)
> return cur_pages;
>
> - pud = pud_offset(pgd, start);
> + pud = pud_page + pud_index(start);
> pud_pgprot = pgprot_4k_2_large(pgprot);
>
> /*
> @@ -1047,7 +1047,7 @@ static int populate_pud(struct cpa_data *cpa, unsigned long start, pgd_t *pgd,
> if (start < end) {
> int tmp;
>
> - pud = pud_offset(pgd, start);
> + pud = pud_page + pud_index(start);
> if (pud_none(*pud))
> if (alloc_pmd_page(pud))
> return -1;
> @@ -1069,7 +1069,7 @@ static int populate_pud(struct cpa_data *cpa, unsigned long start, pgd_t *pgd,
> static int populate_pgd(struct cpa_data *cpa, unsigned long addr)
> {
> pgprot_t pgprot = __pgprot(_KERNPG_TABLE);
> - pud_t *pud = NULL; /* shut up gcc */
> + pud_t *pud_page = NULL; /* shut up gcc */
> pgd_t *pgd_entry;
> int ret;
>
> @@ -1079,25 +1079,27 @@ static int populate_pgd(struct cpa_data *cpa, unsigned long addr)
> * Allocate a PUD page and hand it down for mapping.
> */
> if (pgd_none(*pgd_entry)) {
> - pud = (pud_t *)get_zeroed_page(GFP_KERNEL | __GFP_NOTRACK);
> - if (!pud)
> + pud_page = (pud_t *)get_zeroed_page(GFP_KERNEL | __GFP_NOTRACK);
> + if (!pud_page)
> return -1;
> }
>
> pgprot_val(pgprot) &= ~pgprot_val(cpa->mask_clr);
> pgprot_val(pgprot) |= pgprot_val(cpa->mask_set);
>
> - ret = populate_pud(cpa, addr, pgd_entry, pgprot);
> + ret = populate_pud(cpa, addr,
> + pud_page ?: (pud_t *)pgd_page_vaddr(*pgd_entry),
> + pgprot);
> if (ret < 0) {
> - if (pud)
> - free_page((unsigned long)pud);
> + if (pud_page)
> + free_page((unsigned long)pud_page);
> unmap_pud_range(pgd_entry, addr,
> addr + (cpa->numpages << PAGE_SHIFT));
is it safe to call unmap_pud_range on pgd_entry if it's empty (pgd_none returned true)?
> return ret;
> }
>
> - if (pud)
> - set_pgd(pgd_entry, __pgd(__pa(pud) | _KERNPG_TABLE));
> + if (pud_page)
> + set_pgd(pgd_entry, __pgd(__pa(pud_page) | _KERNPG_TABLE));
>
> cpa->numpages = ret;
> return 0;
> --
> 2.7.4
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
