| 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
| ||
|
Message-ID: <20151216081048.GA20560@amd>
Date: Wed, 16 Dec 2015 09:10:48 +0100
From: Pavel Machek <pavel@....cz>
To: Rusty Russell <rusty@...tcorp.com.au>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Andy Lutomirski <luto@...capital.net>,
Arjan van de Ven <arjan@...ux.intel.com>,
Borislav Petkov <bp@...en8.de>,
kernel list <linux-kernel@...r.kernel.org>,
Stephen Smalley <sds@...ho.nsa.gov>,
Brian Gerst <brgerst@...il.com>,
Denys Vlasenko <dvlasenk@...hat.com>,
Peter Anvin <hpa@...or.com>, Mike Galbraith <efault@....de>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: 4.4.-rc5: lguest causes ugly warn on: 5 W+X pages found
Hi!
> > Rusty, does the switcher need to be W+X?
> >
> > And yes, I have lguest enabled, not sure why.
>
> No. The layout is "<text page> <per-cpu-stack-pages>..." 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 <pavel@....cz>
Thanks,
Pavel
> Reported-by: Pavel Machek <pavel@....cz>
> Signed-off-by: Rusty Russell <rusty@...tcorp.com.au>
>
> 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists