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

[Khaled Elmously]

I made a mistake and applied this to Bionic instead of Cosmic.

It looks like this patch was generated from Bionic not from Cosmic, because it applies cleanly to B and not C. I adjusted the patch and applied it to Cosmic just now.

Juerg, should this be removed from Bionic? The bug is nominated for Bionic and the patch applied cleanly there (and I don't see another fix for Bionic on the ML).

Also could you please take a quick look at the commit that I made to Cosmic and confirm that it's good? It was straight-forward to adjust but it's your sign-off that's on the patch (git am didn't like that there was a newer #include and that the variable 'pr' was renamed to 'eff' in note_page() so I manually resolved those conflicts)


Thanks
Khaled




On 2019-01-29 01:43:02 , Khaled Elmously wrote:
> On 2019-01-28 14:06:28 , 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
> > -- 
> > 2.19.1
> > 
> > 
> > -- 
> > kernel-team mailing list
> > kernel-team at lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team