ACK: [SRU][Bionic][PATCH] x86/mm: Do not warn about PCI BIOS W+X mappings

[Colin Ian King]

On 28/01/2019 13:06, Juerg Haefliger wrote:
> From: Thomas Gleixner <tglx at linutronix.de>
> 
> BugLink: https://bugs.launchpad.net/bugs/1813532
> 
> PCI BIOS requires the BIOS area 0x0A0000-0x0FFFFFF to be mapped W+X for
> various legacy reasons. When CONFIG_DEBUG_WX is enabled, this triggers the
> WX warning, but this is misleading because the mapping is required and is
> not a result of an accidental oversight.
> 
> Prevent the full warning when PCI BIOS is enabled and the detected WX
> mapping is in the BIOS area. Just emit a pr_warn() which denotes the
> fact. This is partially duplicating the info which the PCI BIOS code emits
> when it maps the area as executable, but that info is not in the context of
> the WX checking output.
> 
> Remove the extra %p printout in the WARN_ONCE() while at it. %pS is enough.
> 
> Reported-by: Paul Menzel <pmenzel at molgen.mpg.de>
> Signed-off-by: Thomas Gleixner <tglx at linutronix.de>
> Tested-by: Borislav Petkov <bp at suse.de>
> Cc: Joerg Roedel <joro at 8bytes.org>
> Cc: Kees Cook <keescook at chromium.org>
> Cc: Bjorn Helgaas <bhelgaas at google.com>
> Link: https://lkml.kernel.org/r/alpine.DEB.2.21.1810082151160.2455@nanos.tec.linutronix.de
> 
> (backported from commit c200dac78fec66d87ef262cac38cfe4feabdf737)
> [juergh: Adjusted context.]
> Signed-off-by: Juerg Haefliger <juergh at canonical.com>
> ---
>  arch/x86/mm/dump_pagetables.c | 35 +++++++++++++++++++++++++++--------
>  1 file changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/mm/dump_pagetables.c b/arch/x86/mm/dump_pagetables.c
> index 2a4849e92831..c9b50a5b7942 100644
> --- a/arch/x86/mm/dump_pagetables.c
> +++ b/arch/x86/mm/dump_pagetables.c
> @@ -18,7 +18,9 @@
>  #include <linux/init.h>
>  #include <linux/sched.h>
>  #include <linux/seq_file.h>
> +#include <linux/pci.h>
>  
> +#include <asm/e820/types.h>
>  #include <asm/pgtable.h>
>  
>  /*
> @@ -225,6 +227,29 @@ static unsigned long normalize_addr(unsigned long u)
>  	return (signed long)(u << shift) >> shift;
>  }
>  
> +static void note_wx(struct pg_state *st)
> +{
> +	unsigned long npages;
> +
> +	npages = (st->current_address - st->start_address) / PAGE_SIZE;
> +
> +#ifdef CONFIG_PCI_BIOS
> +	/*
> +	 * If PCI BIOS is enabled, the PCI BIOS area is forced to WX.
> +	 * Inform about it, but avoid the warning.
> +	 */
> +	if (pcibios_enabled && st->start_address >= PAGE_OFFSET + BIOS_BEGIN &&
> +	    st->current_address <= PAGE_OFFSET + BIOS_END) {
> +		pr_warn_once("x86/mm: PCI BIOS W+X mapping %lu pages\n", npages);
> +		return;
> +	}
> +#endif
> +	/* Account the WX pages */
> +	st->wx_pages += npages;
> +	WARN_ONCE(1, "x86/mm: Found insecure W+X mapping at address %pS\n",
> +		  (void *)st->start_address);
> +}
> +
>  /*
>   * This function gets called on a break in a continuous series
>   * of PTE entries; the next one is different so we need to
> @@ -259,14 +284,8 @@ static void note_page(struct seq_file *m, struct pg_state *st,
>  		int width = sizeof(unsigned long) * 2;
>  		pgprotval_t pr = pgprot_val(st->current_prot);
>  
> -		if (st->check_wx && (pr & _PAGE_RW) && !(pr & _PAGE_NX)) {
> -			WARN_ONCE(1,
> -				  "x86/mm: Found insecure W+X mapping at address %p/%pS\n",
> -				  (void *)st->start_address,
> -				  (void *)st->start_address);
> -			st->wx_pages += (st->current_address -
> -					 st->start_address) / PAGE_SIZE;
> -		}
> +		if (st->check_wx && (pr & _PAGE_RW) && !(pr & _PAGE_NX))
> +			note_wx(st);
>  
>  		/*
>  		 * Now print the actual finished series
> 

Looks good to me.

Acked-by: Colin Ian King <colin.king at canonical.com>