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

[Stefan Bader]

On 30.01.19 01:14, Khaled Elmously wrote:
> 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)

At least the thread you replied to was for Bionic. There is a second submission
(which unfortunately started with Bionic in the subject but has an ACK changing
that to Cosmic and missing a second ACK. That one would be for Cosmic...

-Stefan

> 
> 
> 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
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20190130/0ecbde83/attachment.sig>