Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933940AbbLPIKw (ORCPT ); Wed, 16 Dec 2015 03:10:52 -0500 Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]:40133 "EHLO atrey.karlin.mff.cuni.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932751AbbLPIKv (ORCPT ); Wed, 16 Dec 2015 03:10:51 -0500 Date: Wed, 16 Dec 2015 09:10:48 +0100 From: Pavel Machek To: Rusty Russell Cc: Linus Torvalds , Andy Lutomirski , Arjan van de Ven , Borislav Petkov , kernel list , Stephen Smalley , Brian Gerst , Denys Vlasenko , Peter Anvin , Mike Galbraith , Peter Zijlstra , Thomas Gleixner Subject: Re: 4.4.-rc5: lguest causes ugly warn on: 5 W+X pages found Message-ID: <20151216081048.GA20560@amd> References: <20151214202627.GA15104@amd> <566F3378.8070009@linux.intel.com> <20151215094015.GA3677@amd> <20151215205835.GA3522@amd> <20151215211231.GA6752@amd> <87y4cvw2oa.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87y4cvw2oa.fsf@rustcorp.com.au> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6363 Lines: 176 Hi! > > Rusty, does the switcher need to be W+X? > > > > And yes, I have lguest enabled, not sure why. > > No. The layout is "

..." and I lazily > did that as a single > map_vm_area(switcher_vma, PAGE_KERNEL_EXEC, lg_switcher_pages); > > This boots, does it solve the problem? Let me see. Note that I'm not actually using lguest. git complains about trailing whitespace in the patch. Otherwise... yes, this helps. "no W+X pages found". Tested-by: Pavel Machek Thanks, Pavel > Reported-by: Pavel Machek > Signed-off-by: Rusty Russell > > diff --git a/arch/x86/include/asm/lguest.h b/arch/x86/include/asm/lguest.h > index 3bbc07a57a31..73d0c9b92087 100644 > --- a/arch/x86/include/asm/lguest.h > +++ b/arch/x86/include/asm/lguest.h > @@ -12,7 +12,9 @@ > #define GUEST_PL 1 > > /* Page for Switcher text itself, then two pages per cpu */ > -#define TOTAL_SWITCHER_PAGES (1 + 2 * nr_cpu_ids) > +#define SWITCHER_TEXT_PAGES (1) > +#define SWITCHER_STACK_PAGES (2 * nr_cpu_ids) > +#define TOTAL_SWITCHER_PAGES (SWITCHER_TEXT_PAGES + SWITCHER_STACK_PAGES) > > /* Where we map the Switcher, in both Host and Guest. */ > extern unsigned long switcher_addr; > diff --git a/drivers/lguest/core.c b/drivers/lguest/core.c > index 312ffd3d0017..021915baef35 100644 > --- a/drivers/lguest/core.c > +++ b/drivers/lguest/core.c > @@ -22,7 +22,8 @@ > > unsigned long switcher_addr; > struct page **lg_switcher_pages; > -static struct vm_struct *switcher_vma; > +static struct vm_struct *switcher_text_vma; > +static struct vm_struct *switcher_stacks_vma; > > /* This One Big lock protects all inter-guest data structures. */ > DEFINE_MUTEX(lguest_lock); > @@ -83,54 +84,80 @@ static __init int map_switcher(void) > } > > /* > + * Copy in the compiled-in Switcher code (from x86/switcher_32.S). > + * It goes in the first page, which we map in momentarily. > + */ > + memcpy(kmap(lg_switcher_pages[0]), start_switcher_text, > + end_switcher_text - start_switcher_text); > + kunmap(lg_switcher_pages[0]); > + > + /* > * We place the Switcher underneath the fixmap area, which is the > * highest virtual address we can get. This is important, since we > * tell the Guest it can't access this memory, so we want its ceiling > * as high as possible. > */ > - switcher_addr = FIXADDR_START - (TOTAL_SWITCHER_PAGES+1)*PAGE_SIZE; > + switcher_addr = FIXADDR_START - TOTAL_SWITCHER_PAGES*PAGE_SIZE; > > /* > - * Now we reserve the "virtual memory area" we want. We might > - * not get it in theory, but in practice it's worked so far. > - * The end address needs +1 because __get_vm_area allocates an > - * extra guard page, so we need space for that. > + * Now we reserve the "virtual memory area"s we want. We might > + * not get them in theory, but in practice it's worked so far. > + * > + * We want the switcher text to be read-only and executable, and > + * the stacks to be read-write and non-executable. > */ > - switcher_vma = __get_vm_area(TOTAL_SWITCHER_PAGES * PAGE_SIZE, > - VM_ALLOC, switcher_addr, switcher_addr > - + (TOTAL_SWITCHER_PAGES+1) * PAGE_SIZE); > - if (!switcher_vma) { > + switcher_text_vma = __get_vm_area(PAGE_SIZE, VM_ALLOC|VM_NO_GUARD, > + switcher_addr, > + switcher_addr + PAGE_SIZE); > + > + if (!switcher_text_vma) { > err = -ENOMEM; > printk("lguest: could not map switcher pages high\n"); > goto free_pages; > } > > + switcher_stacks_vma = __get_vm_area(SWITCHER_STACK_PAGES * PAGE_SIZE, > + VM_ALLOC|VM_NO_GUARD, > + switcher_addr + PAGE_SIZE, > + switcher_addr + TOTAL_SWITCHER_PAGES * PAGE_SIZE); > + if (!switcher_stacks_vma) { > + err = -ENOMEM; > + printk("lguest: could not map switcher pages high\n"); > + goto free_text_vma; > + } > + > /* > * This code actually sets up the pages we've allocated to appear at > * switcher_addr. map_vm_area() takes the vma we allocated above, the > - * kind of pages we're mapping (kernel pages), and a pointer to our > - * array of struct pages. > + * kind of pages we're mapping (kernel text pages and kernel writable > + * pages respectively), and a pointer to our array of struct pages. > */ > - err = map_vm_area(switcher_vma, PAGE_KERNEL_EXEC, lg_switcher_pages); > + err = map_vm_area(switcher_text_vma, PAGE_KERNEL_RX, lg_switcher_pages); > + if (err) { > + printk("lguest: text map_vm_area failed: %i\n", err); > + goto free_vmas; > + } > + > + err = map_vm_area(switcher_stacks_vma, PAGE_KERNEL, > + lg_switcher_pages + SWITCHER_TEXT_PAGES); > if (err) { > - printk("lguest: map_vm_area failed: %i\n", err); > - goto free_vma; > + printk("lguest: stacks map_vm_area failed: %i\n", err); > + goto free_vmas; > } > > /* > * Now the Switcher is mapped at the right address, we can't fail! > - * Copy in the compiled-in Switcher code (from x86/switcher_32.S). > */ > - memcpy(switcher_vma->addr, start_switcher_text, > - end_switcher_text - start_switcher_text); > - > printk(KERN_INFO "lguest: mapped switcher at %p\n", > - switcher_vma->addr); > + switcher_text_vma->addr); > /* And we succeeded... */ > return 0; > > -free_vma: > - vunmap(switcher_vma->addr); > +free_vmas: > + /* Undoes map_vm_area and __get_vm_area */ > + vunmap(switcher_stacks_vma->addr); > +free_text_vma: > + vunmap(switcher_text_vma->addr); > free_pages: > i = TOTAL_SWITCHER_PAGES; > free_some_pages: > @@ -148,7 +175,8 @@ static void unmap_switcher(void) > unsigned int i; > > /* vunmap() undoes *both* map_vm_area() and __get_vm_area(). */ > - vunmap(switcher_vma->addr); > + vunmap(switcher_text_vma->addr); > + vunmap(switcher_stacks_vma->addr); > /* Now we just need to free the pages we copied the switcher into */ > for (i = 0; i < TOTAL_SWITCHER_PAGES; i++) > __free_pages(lg_switcher_pages[i], 0); -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/