linux-kernel - Re: [PATCH] x86: mm: Do not use set_{pud,pmd}

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20190409084031.GO4038@hirez.programming.kicks-ass.net>
Date:   Tue, 9 Apr 2019 10:40:31 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Singh, Brijesh" <brijesh.singh@....com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        Dave Hansen <dave.hansen@...el.com>,
        Dan Williams <dan.j.williams@...el.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Lendacky, Thomas" <Thomas.Lendacky@....com>
Subject: Re: [PATCH] x86: mm: Do not use set_{pud,pmd}_safe when splitting
 the large page

On Mon, Apr 08, 2019 at 07:11:21PM +0000, Singh, Brijesh wrote:
> The following commit 0a9fe8ca844d ("x86/mm: Validate kernel_physical_mapping_init()
> PTE population") triggers the below warning in the SEV guest.
> 
> WARNING: CPU: 0 PID: 0 at arch/x86/include/asm/pgalloc.h:87 phys_pmd_init+0x30d/0x386
> Call Trace:
>  kernel_physical_mapping_init+0xce/0x259
>  early_set_memory_enc_dec+0x10f/0x160
>  kvm_smp_prepare_boot_cpu+0x71/0x9d
>  start_kernel+0x1c9/0x50b
>  secondary_startup_64+0xa4/0xb0
> 
> The SEV guest calls kernel_physical_mapping_init() to clear the encryption
> mask from an existing mapping. While clearing the encryption mask
> kernel_physical_mapping_init() splits the large pages into the smaller.
> To split the page, the kernel_physical_mapping_init() allocates a new page
> and updates the existing entry. The set_{pud,pmd}_safe triggers warning
> when updating the entry with page in the present state. We should use the
> set_{pud,pmd} when updating an existing entry with the new entry.
> 
> Updating an entry will also requires a TLB flush. Currently the caller
> (early_set_memory_enc_dec()) is taking care of issuing the TLB flushes.

I'm not entirely sure I like this, this means all users of
kernel_physical_mapping_init() now need to be aware and careful.

That said; the alternative is adding an argument to the function and
propagating it through the callchain and dynamically switching between
_safe and not. Which doesn't sound ideal either.

Anybody else got clever ideas?

> Signed-off-by: Brijesh Singh <brijesh.singh@....com>
> Fixes: 0a9fe8ca844d (x86/mm: Validate kernel_physical_mapping_init() ...)
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Dave Hansen <dave.hansen@...el.com>
> Cc: Dan Williams <dan.j.williams@...el.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> Cc: Peter Zijlstra (Intel) <peterz@...radead.org>
> Cc: Andy Lutomirski <luto@...nel.org>
> Cc: Borislav Petkov <bp@...en8.de>
> Cc: H. Peter Anvin <hpa@...or.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Tom Lendacky <Thomas.Lendacky@....com>
> ---
>  arch/x86/mm/init_64.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index bccff68e3267..0a26b64a99b9 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -536,7 +536,7 @@ phys_pmd_init(pmd_t *pmd_page, unsigned long paddr, unsigned long paddr_end,
>  		paddr_last = phys_pte_init(pte, paddr, paddr_end, new_prot);
>  
>  		spin_lock(&init_mm.page_table_lock);
> -		pmd_populate_kernel_safe(&init_mm, pmd, pte);
> +		pmd_populate_kernel(&init_mm, pmd, pte);
>  		spin_unlock(&init_mm.page_table_lock);
>  	}
>  	update_page_count(PG_LEVEL_2M, pages);
> @@ -623,7 +623,7 @@ phys_pud_init(pud_t *pud_page, unsigned long paddr, unsigned long paddr_end,
>  					   page_size_mask, prot);
>  
>  		spin_lock(&init_mm.page_table_lock);
> -		pud_populate_safe(&init_mm, pud, pmd);
> +		pud_populate(&init_mm, pud, pmd);
>  		spin_unlock(&init_mm.page_table_lock);
>  	}
>  
> -- 
> 2.17.1
>