linux-kernel - Re: [PATCH v2] x86-64/Xen: eliminate W+X mappings

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]

Message-ID: <20171218122825.6x33zeknoqbf3xcd@gmail.com>
Date:   Mon, 18 Dec 2017 13:28:25 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     Jan Beulich <JBeulich@...e.com>
Cc:     Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>, mingo@...e.hu,
        tglx@...utronix.de, xen-devel <xen-devel@...ts.xenproject.org>,
        linux-kernel@...r.kernel.org, hpa@...or.com,
        Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH v2] x86-64/Xen: eliminate W+X mappings


* Jan Beulich <JBeulich@...e.com> wrote:

> A few thousand such pages are usually left around due to the re-use of
> L1 tables having been provided by the hypervisor (Dom0) or tool stack
> (DomU). Set NX in the direct map variant, which needs to be done in L2
> due to the dual use of the re-used L1s.
> 
> For x86_configure_nx() to actually do what it is supposed to do, call
> get_cpu_cap() first. This was broken by commit 4763ed4d45 ("x86, mm:
> Clean up and simplify NX enablement") when switching away from the
> direct EFER read.
> 
> Signed-off-by: Jan Beulich <jbeulich@...e.com>
> ---
> v2: Adjust comment style and indentation.
> ---
> While I certainly dislike the added header inclusion to obtain the
> prototype for get_cpu_cap(), I couldn't find a better alternative. I'm
> open to suggestions.
> ---
>  arch/x86/xen/enlighten_pv.c |    3 +++
>  arch/x86/xen/mmu_pv.c       |   10 ++++++++++
>  2 files changed, 13 insertions(+)
> 
> --- 4.15-rc3/arch/x86/xen/enlighten_pv.c
> +++ 4.15-rc3-x86_64-Xen-avoid-W+X/arch/x86/xen/enlighten_pv.c
> @@ -88,6 +88,8 @@
>  #include "multicalls.h"
>  #include "pmu.h"
>  
> +#include "../kernel/cpu/cpu.h" /* get_cpu_cap() */
> +
>  void *xen_initial_gdt;
>  
>  static int xen_cpu_up_prepare_pv(unsigned int cpu);
> @@ -1258,6 +1260,7 @@ asmlinkage __visible void __init xen_sta
>  	__userpte_alloc_gfp &= ~__GFP_HIGHMEM;
>  
>  	/* Work out if we support NX */
> +	get_cpu_cap(&boot_cpu_data);
>  	x86_configure_nx();
>  
>  	/* Get mfn list */
> --- 4.15-rc4/arch/x86/xen/mmu_pv.c
> +++ 4.15-rc4-x86_64-Xen-avoid-W+X/arch/x86/xen/mmu_pv.c
> @@ -1902,6 +1902,18 @@ void __init xen_setup_kernel_pagetable(p
>  	/* Graft it onto L4[511][510] */
>  	copy_page(level2_kernel_pgt, l2);
>  
> +	/*
> +	 * Zap execute permission from the ident map. Due to the sharing of
> +	 * L1 entries we need to do this in the L2.
> +	 */
> +	if (__supported_pte_mask & _PAGE_NX)
> +		for (i = 0; i < PTRS_PER_PMD; ++i) {
> +			if (pmd_none(level2_ident_pgt[i]))
> +				continue;
> +			level2_ident_pgt[i] = pmd_set_flags(level2_ident_pgt[i],
> +							    _PAGE_NX);
> +		}
> +

This chunk has two stylistic problems:

 - Curly braces need to be added
 - Line broken in an ugly fashion: just make it long and ignore the checkpatch col80 warning

looks good otherwise.

Thanks,

	Ingo