[PATCH 0/3] x86_64: Tidy up vsyscall emulation and make it optional

[PATCH 0/3] x86_64: Tidy up vsyscall emulation and make it optional

[Andy Lutomirski]

Now that arch/x86/kernel/vsyscall_64.c contains only vsyscall
emulation code, clean it up and make it optional.

Patch 1 makes vsyscall=none work be a bit more self-consistent: it
actually removes the fake vsyscall page instead of just segfaulting
anyone who tries to use it.

Patch 2 is pure cosmetic cleanup.

Patch 3 is the meat: it lets vsyscall emulation be configured out.
The config option to disable it is hidden under CONFIG_EXPERT, since
it will break legacy code.

Note that, last I checked, current userspace is unlikely to work if
the vDSO *and* vsyscalls are off.  Take it up with the glibc
maintainers.

This applies on top of tip/x86/vdso.

Andy Lutomirski (3):
  x86_64,vsyscall: Turn vsyscalls all the way off when vsyscall=none
  x86_64,vsyscall: Rewrite comment and clean up headers in vsyscall code
  x86_64,vsyscall: Make vsyscall emulation configurable

 arch/x86/Kconfig                | 18 ++++++++++++
 arch/x86/include/asm/fixmap.h   |  2 ++
 arch/x86/include/asm/page_64.h  |  4 ++-
 arch/x86/include/asm/vsyscall.h |  8 +++++
 arch/x86/kernel/Makefile        |  3 +-
 arch/x86/kernel/setup.c         |  2 --
 arch/x86/kernel/vsyscall_64.c   | 65 +++++++++++++++++------------------------
 arch/x86/xen/mmu.c              |  6 ++--
 8 files changed, 63 insertions(+), 45 deletions(-)

-- 
1.9.3

[PATCH 1/3] x86_64,vsyscall: Turn vsyscalls all the way off when vsyscall=none

[Andy Lutomirski]

I see no point in having an unusable read-only page sitting at
0xffffffffff600000 when vsyscall=none.  Instead, skip mapping it and
remove it from /proc/PID/maps.

I kept the ratelimited warning when programs try to use a vsyscall
in this mode, since it may help admins avoid confusion.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/kernel/vsyscall_64.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/vsyscall_64.c b/arch/x86/kernel/vsyscall_64.c
index 419e83b58436..2d912629c96e 100644
--- a/arch/x86/kernel/vsyscall_64.c
+++ b/arch/x86/kernel/vsyscall_64.c
@@ -307,6 +307,8 @@ struct vm_area_struct *get_gate_vma(struct mm_struct *mm)
 	if (!mm || mm->context.ia32_compat)
 		return NULL;
 #endif
+	if (vsyscall_mode == NONE)
+		return NULL;
 	return &gate_vma;
 }
 
@@ -327,7 +329,7 @@ int in_gate_area(struct mm_struct *mm, unsigned long addr)
  */
 int in_gate_area_no_mm(unsigned long addr)
 {
-	return (addr & PAGE_MASK) == VSYSCALL_ADDR;
+	return vsyscall_mode != NONE && (addr & PAGE_MASK) == VSYSCALL_ADDR;
 }
 
 void __init map_vsyscall(void)
@@ -335,10 +337,12 @@ void __init map_vsyscall(void)
 	extern char __vsyscall_page;
 	unsigned long physaddr_vsyscall = __pa_symbol(&__vsyscall_page);
 
-	__set_fixmap(VSYSCALL_PAGE, physaddr_vsyscall,
-		     vsyscall_mode == NATIVE
-		     ? PAGE_KERNEL_VSYSCALL
-		     : PAGE_KERNEL_VVAR);
+	if (vsyscall_mode != NONE)
+		__set_fixmap(VSYSCALL_PAGE, physaddr_vsyscall,
+			     vsyscall_mode == NATIVE
+			     ? PAGE_KERNEL_VSYSCALL
+			     : PAGE_KERNEL_VVAR);
+
 	BUILD_BUG_ON((unsigned long)__fix_to_virt(VSYSCALL_PAGE) !=
 		     (unsigned long)VSYSCALL_ADDR);
 }
-- 
1.9.3

[PATCH 2/3] x86_64,vsyscall: Rewrite comment and clean up headers in vsyscall code

[Andy Lutomirski]

vsyscall_64.c is just vsyscall emulation.  Tidy it up accordingly.

If my comment editing offends anyone, let me know and I can fix it.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/kernel/vsyscall_64.c | 51 +++++++++++++++----------------------------
 1 file changed, 18 insertions(+), 33 deletions(-)

diff --git a/arch/x86/kernel/vsyscall_64.c b/arch/x86/kernel/vsyscall_64.c
index 2d912629c96e..ee622f8183f3 100644
--- a/arch/x86/kernel/vsyscall_64.c
+++ b/arch/x86/kernel/vsyscall_64.c
@@ -1,52 +1,37 @@
 /*
- *  Copyright (C) 2001 Andrea Arcangeli <andrea@suse.de> SuSE
- *  Copyright 2003 Andi Kleen, SuSE Labs.
+ * Mostly copyright (c) 2012-2014 Andy Lutomirski <luto@amacapital.net>
  *
- *  [ NOTE: this mechanism is now deprecated in favor of the vDSO. ]
+ * This file implements vsyscall emulation.  vsyscalls are a legacy ABI:
+ * userspace can request certain kernel services by calling fixed
+ * addresses.  This concept is problematic:
  *
- *  Thanks to hpa@transmeta.com for some useful hint.
- *  Special thanks to Ingo Molnar for his early experience with
- *  a different vsyscall implementation for Linux/IA32 and for the name.
+ * - It interferes with ASLR.
+ * - It's awkward to write code that lives in kernel addresses but is
+ *   callable by userspace at fixed addresses.
+ * - The whole concept is impossible for 32-bit compat userspace.
+ * - UML cannot easily virtualize a vsyscall.
  *
- *  vsyscall 1 is located at -10Mbyte, vsyscall 2 is located
- *  at virtual address -10Mbyte+1024bytes etc... There are at max 4
- *  vsyscalls. One vsyscall can reserve more than 1 slot to avoid
- *  jumping out of line if necessary. We cannot add more with this
- *  mechanism because older kernels won't return -ENOSYS.
+ * As of mid-2014, I believe that there is no new userspace code that
+ * will use a vsyscall if the vDSO is present.  I hope that there will
+ * soon be no new userspace code that will ever use a vsyscall.
  *
- *  Note: the concept clashes with user mode linux.  UML users should
- *  use the vDSO.
+ * The code in this file emulates vsyscalls when notified of a page
+ * fault to a vsyscall address.
+ *
+ * The original version of this code is:
+ * Copyright (C) 2001 Andrea Arcangeli <andrea@suse.de> SuSE
+ * Copyright 2003 Andi Kleen, SuSE Labs.
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/time.h>
-#include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/timer.h>
-#include <linux/seqlock.h>
-#include <linux/jiffies.h>
-#include <linux/sysctl.h>
-#include <linux/topology.h>
-#include <linux/timekeeper_internal.h>
-#include <linux/getcpu.h>
-#include <linux/cpu.h>
-#include <linux/smp.h>
-#include <linux/notifier.h>
 #include <linux/syscalls.h>
 #include <linux/ratelimit.h>
 
 #include <asm/vsyscall.h>
-#include <asm/pgtable.h>
-#include <asm/compat.h>
-#include <asm/page.h>
 #include <asm/unistd.h>
 #include <asm/fixmap.h>
-#include <asm/errno.h>
-#include <asm/io.h>
-#include <asm/segment.h>
-#include <asm/desc.h>
-#include <asm/topology.h>
 #include <asm/traps.h>
 
 #define CREATE_TRACE_POINTS
-- 
1.9.3

[PATCH 3/3] x86_64,vsyscall: Make vsyscall emulation configurable

[Andy Lutomirski]

This adds CONFIG_X86_VSYSCALL_EMULATION, guarded by CONFIG_EXPERT.
Turning it off completely disables vsyscall emulation, saving ~3.5k
for vsyscall_64.c, 4k for vsyscall_emu_64.S (the fake vsyscall
page), some tiny amount of core mm code that supports a gate area,
and possibly 4k for a wasted pagetable.  The latter is because the
vsyscall addresses are misaligned and fit poorly in the fixmap.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/Kconfig                | 18 ++++++++++++++++++
 arch/x86/include/asm/fixmap.h   |  2 ++
 arch/x86/include/asm/page_64.h  |  4 +++-
 arch/x86/include/asm/vsyscall.h |  8 ++++++++
 arch/x86/kernel/Makefile        |  3 +--
 arch/x86/kernel/setup.c         |  2 --
 arch/x86/xen/mmu.c              |  6 ++++--
 7 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f2327e88e07c..cd10436d7d1c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -984,6 +984,24 @@ config X86_ESPFIX64
 	def_bool y
 	depends on X86_16BIT && X86_64
 
+config X86_VSYSCALL_EMULATION
+       bool "Enable vsyscall emulation" if EXPERT
+       default y
+       depends on X86_64
+       ---help---
+	 This enables emulation of the legacy vsyscall page.  Disabling
+	 it is roughly equivalent to booting with vsyscall=none, except
+	 that it will also disable the helpful warning if a program
+	 tries to use a vsyscall.  With this option set to N, offending
+	 programs will just segfault, citing addresses of the form
+	 0xffffffffff600?00.
+
+	 This option is required by many programs built before 2013, and
+	 care should be used even with newer programs if set to N.
+
+	 Disabling this option saves about 7K of kernel size and
+	 possibly 4K of additional runtime pagetable memory.
+
 config TOSHIBA
 	tristate "Toshiba Laptop support"
 	depends on X86_32
diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
index ffb1733ac91f..d8d5bcb2a0b5 100644
--- a/arch/x86/include/asm/fixmap.h
+++ b/arch/x86/include/asm/fixmap.h
@@ -69,7 +69,9 @@ enum fixed_addresses {
 #ifdef CONFIG_X86_32
 	FIX_HOLE,
 #else
+#ifdef CONFIG_X86_VSYSCALL_EMULATION
 	VSYSCALL_PAGE = (FIXADDR_TOP - VSYSCALL_ADDR) >> PAGE_SHIFT,
+#endif
 #ifdef CONFIG_PARAVIRT_CLOCK
 	PVCLOCK_FIXMAP_BEGIN,
 	PVCLOCK_FIXMAP_END = PVCLOCK_FIXMAP_BEGIN+PVCLOCK_VSYSCALL_NR_PAGES-1,
diff --git a/arch/x86/include/asm/page_64.h b/arch/x86/include/asm/page_64.h
index f408caf73430..b3bebf9e5746 100644
--- a/arch/x86/include/asm/page_64.h
+++ b/arch/x86/include/asm/page_64.h
@@ -39,6 +39,8 @@ void copy_page(void *to, void *from);
 
 #endif	/* !__ASSEMBLY__ */
 
-#define __HAVE_ARCH_GATE_AREA 1
+#ifdef CONFIG_X86_VSYSCALL_EMULATION
+# define __HAVE_ARCH_GATE_AREA 1
+#endif
 
 #endif /* _ASM_X86_PAGE_64_H */
diff --git a/arch/x86/include/asm/vsyscall.h b/arch/x86/include/asm/vsyscall.h
index 34f7d8857542..6ba66ee79710 100644
--- a/arch/x86/include/asm/vsyscall.h
+++ b/arch/x86/include/asm/vsyscall.h
@@ -4,6 +4,7 @@
 #include <linux/seqlock.h>
 #include <uapi/asm/vsyscall.h>
 
+#ifdef CONFIG_X86_VSYSCALL_EMULATION
 extern void map_vsyscall(void);
 
 /*
@@ -11,5 +12,12 @@ extern void map_vsyscall(void);
  * Returns true if handled.
  */
 extern bool emulate_vsyscall(struct pt_regs *regs, unsigned long address);
+#else
+static inline void map_vsyscall(void) {}
+static inline bool emulate_vsyscall(struct pt_regs *regs, unsigned long address)
+{
+	return false;
+}
+#endif
 
 #endif /* _ASM_X86_VSYSCALL_H */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 8f1e77440b2b..5d4502c8b983 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -28,8 +28,7 @@ obj-$(CONFIG_X86_32)	+= i386_ksyms_32.o
 obj-$(CONFIG_X86_64)	+= sys_x86_64.o x8664_ksyms_64.o
 obj-$(CONFIG_X86_64)	+= mcount_64.o
 obj-y			+= syscall_$(BITS).o vsyscall_gtod.o
-obj-$(CONFIG_X86_64)	+= vsyscall_64.o
-obj-$(CONFIG_X86_64)	+= vsyscall_emu_64.o
+obj-$(CONFIG_X86_VSYSCALL_EMULATION)	+= vsyscall_64.o vsyscall_emu_64.o
 obj-$(CONFIG_X86_ESPFIX64)	+= espfix_64.o
 obj-$(CONFIG_SYSFS)	+= ksysfs.o
 obj-y			+= bootflag.o e820.o
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 235cfd39e0d7..59a6f884fdad 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1190,9 +1190,7 @@ void __init setup_arch(char **cmdline_p)
 
 	tboot_probe();
 
-#ifdef CONFIG_X86_64
 	map_vsyscall();
-#endif
 
 	generic_apic_probe();
 
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index a8a1a3d08d4d..5046b699eec1 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1456,11 +1456,13 @@ static int xen_pgd_alloc(struct mm_struct *mm)
 		user_pgd = (pgd_t *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
 		page->private = (unsigned long)user_pgd;
 
+#ifdef CONFIG_X86_VSYSCALL_EMULATION
 		if (user_pgd != NULL) {
 			user_pgd[pgd_index(VSYSCALL_ADDR)] =
 				__pgd(__pa(level3_user_vsyscall) | _PAGE_TABLE);
 			ret = 0;
 		}
+#endif
 
 		BUG_ON(PagePinned(virt_to_page(xen_get_user_pgd(pgd))));
 	}
@@ -2021,7 +2023,7 @@ static void xen_set_fixmap(unsigned idx, phys_addr_t phys, pgprot_t prot)
 # ifdef CONFIG_HIGHMEM
 	case FIX_KMAP_BEGIN ... FIX_KMAP_END:
 # endif
-#else
+#elif defined(CONFIG_X86_VSYSCALL_EMULATION)
 	case VSYSCALL_PAGE:
 #endif
 	case FIX_TEXT_POKE0:
@@ -2060,7 +2062,7 @@ static void xen_set_fixmap(unsigned idx, phys_addr_t phys, pgprot_t prot)
 
 	__native_set_fixmap(idx, pte);
 
-#ifdef CONFIG_X86_64
+#ifdef CONFIG_X86_VSYSCALL_EMULATION
 	/* Replicate changes to map the vsyscall page into the user
 	   pagetable vsyscall mapping. */
 	if (idx == VSYSCALL_PAGE) {
-- 
1.9.3

Re: [PATCH 0/3] x86_64: Tidy up vsyscall emulation and make it optional

[josh]

On Tue, Oct 28, 2014 at 10:22:25AM -0700, Andy Lutomirski wrote:
> Now that arch/x86/kernel/vsyscall_64.c contains only vsyscall
> emulation code, clean it up and make it optional.
> 
> Patch 1 makes vsyscall=none work be a bit more self-consistent: it
> actually removes the fake vsyscall page instead of just segfaulting
> anyone who tries to use it.
> 
> Patch 2 is pure cosmetic cleanup.
> 
> Patch 3 is the meat: it lets vsyscall emulation be configured out.
> The config option to disable it is hidden under CONFIG_EXPERT, since
> it will break legacy code.
> 
> Note that, last I checked, current userspace is unlikely to work if
> the vDSO *and* vsyscalls are off.  Take it up with the glibc
> maintainers.
> 
> This applies on top of tip/x86/vdso.
> 
> Andy Lutomirski (3):
>   x86_64,vsyscall: Turn vsyscalls all the way off when vsyscall=none
>   x86_64,vsyscall: Rewrite comment and clean up headers in vsyscall code
>   x86_64,vsyscall: Make vsyscall emulation configurable

Nice!

For patches 1 and 2:
Reviewed-by: Josh Triplett <josh@joshtriplett.org>

For patch 3, I responded with a possible minor improvement, but with or
without that:
Reviewed-by: Josh Triplett <josh@joshtriplett.org>

I assume these should go through tip/x86/vdso as well?

Also, any plans to do something similar for vsyscall_gtod.c?

- Josh Triplett

Re: [PATCH 3/3] x86_64,vsyscall: Make vsyscall emulation configurable

[Josh Triplett]

On Tue, Oct 28, 2014 at 10:22:28AM -0700, Andy Lutomirski wrote:
> This adds CONFIG_X86_VSYSCALL_EMULATION, guarded by CONFIG_EXPERT.
> Turning it off completely disables vsyscall emulation, saving ~3.5k
> for vsyscall_64.c, 4k for vsyscall_emu_64.S (the fake vsyscall
> page), some tiny amount of core mm code that supports a gate area,
> and possibly 4k for a wasted pagetable.  The latter is because the
> vsyscall addresses are misaligned and fit poorly in the fixmap.
> 
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>

One minor nit below, but with or without that change,
Reviewed-by: Josh Triplett <josh@joshtriplett.org>

> --- a/arch/x86/xen/mmu.c
> +++ b/arch/x86/xen/mmu.c
> @@ -1456,11 +1456,13 @@ static int xen_pgd_alloc(struct mm_struct *mm)
>  		user_pgd = (pgd_t *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
>  		page->private = (unsigned long)user_pgd;
>  
> +#ifdef CONFIG_X86_VSYSCALL_EMULATION
>  		if (user_pgd != NULL) {
>  			user_pgd[pgd_index(VSYSCALL_ADDR)] =
>  				__pgd(__pa(level3_user_vsyscall) | _PAGE_TABLE);
>  			ret = 0;
>  		}
> +#endif

Could you instead make the if use IS_ENABLED?

		if (IS_ENABLED(CONFIG_X86_VSYSCALL_EMULATION) && user_pgd != NULL)

That has the advantage of ensuring that the code continues to compile.
(Given that you haven't removed level3_user_vsyscall, that should work.)

- Josh Triplett

Re: [PATCH 0/3] x86_64: Tidy up vsyscall emulation and make it optional

[Andy Lutomirski]

On Tue, Oct 28, 2014 at 10:46 AM,  <josh@joshtriplett.org> wrote:
> On Tue, Oct 28, 2014 at 10:22:25AM -0700, Andy Lutomirski wrote:
>> Now that arch/x86/kernel/vsyscall_64.c contains only vsyscall
>> emulation code, clean it up and make it optional.
>>
>> Patch 1 makes vsyscall=none work be a bit more self-consistent: it
>> actually removes the fake vsyscall page instead of just segfaulting
>> anyone who tries to use it.
>>
>> Patch 2 is pure cosmetic cleanup.
>>
>> Patch 3 is the meat: it lets vsyscall emulation be configured out.
>> The config option to disable it is hidden under CONFIG_EXPERT, since
>> it will break legacy code.
>>
>> Note that, last I checked, current userspace is unlikely to work if
>> the vDSO *and* vsyscalls are off.  Take it up with the glibc
>> maintainers.
>>
>> This applies on top of tip/x86/vdso.
>>
>> Andy Lutomirski (3):
>>   x86_64,vsyscall: Turn vsyscalls all the way off when vsyscall=none
>>   x86_64,vsyscall: Rewrite comment and clean up headers in vsyscall code
>>   x86_64,vsyscall: Make vsyscall emulation configurable
>
> Nice!
>
> For patches 1 and 2:
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
>
> For patch 3, I responded with a possible minor improvement, but with or
> without that:
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
>

I think that hunk may be wrong, although oddly I can't trigger the
failure to boot that I'd expect.  I'll send a v2 anyway.

> I assume these should go through tip/x86/vdso as well?

I think so.

>
> Also, any plans to do something similar for vsyscall_gtod.c?
>

I wasn't planning on it, but it could be done.

--Andy

> - Josh Triplett



-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH 3/3] x86_64,vsyscall: Make vsyscall emulation configurable

[Andy Lutomirski]

On Tue, Oct 28, 2014 at 10:57 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> On Tue, Oct 28, 2014 at 10:22:28AM -0700, Andy Lutomirski wrote:
>> This adds CONFIG_X86_VSYSCALL_EMULATION, guarded by CONFIG_EXPERT.
>> Turning it off completely disables vsyscall emulation, saving ~3.5k
>> for vsyscall_64.c, 4k for vsyscall_emu_64.S (the fake vsyscall
>> page), some tiny amount of core mm code that supports a gate area,
>> and possibly 4k for a wasted pagetable.  The latter is because the
>> vsyscall addresses are misaligned and fit poorly in the fixmap.
>>
>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>
> One minor nit below, but with or without that change,
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
>
>> --- a/arch/x86/xen/mmu.c
>> +++ b/arch/x86/xen/mmu.c
>> @@ -1456,11 +1456,13 @@ static int xen_pgd_alloc(struct mm_struct *mm)
>>               user_pgd = (pgd_t *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
>>               page->private = (unsigned long)user_pgd;
>>
>> +#ifdef CONFIG_X86_VSYSCALL_EMULATION
>>               if (user_pgd != NULL) {
>>                       user_pgd[pgd_index(VSYSCALL_ADDR)] =
>>                               __pgd(__pa(level3_user_vsyscall) | _PAGE_TABLE);
>>                       ret = 0;
>>               }
>> +#endif
>
> Could you instead make the if use IS_ENABLED?
>
>                 if (IS_ENABLED(CONFIG_X86_VSYSCALL_EMULATION) && user_pgd != NULL)
>
> That has the advantage of ensuring that the code continues to compile.
> (Given that you haven't removed level3_user_vsyscall, that should work.)

I need the ret = 0, I think, so I'll resend.

I think I'd rather use #ifdef here, since I think it would be great if
the Xen people could clean this up further.  With this change, under
some configurations, there should be no user-accessible kernel
addresses at all.  (Also, is there some PV mechanism
that I'm not thinking of that will break with this change?  I know
I've tripped over Xen pagetable and fixmap oddities before.)

--Andy

Re: [PATCH 3/3] x86_64,vsyscall: Make vsyscall emulation configurable

[Konrad Rzeszutek Wilk]

On Tue, Oct 28, 2014 at 11:09:53AM -0700, Andy Lutomirski wrote:
> On Tue, Oct 28, 2014 at 10:57 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> > On Tue, Oct 28, 2014 at 10:22:28AM -0700, Andy Lutomirski wrote:
> >> This adds CONFIG_X86_VSYSCALL_EMULATION, guarded by CONFIG_EXPERT.
> >> Turning it off completely disables vsyscall emulation, saving ~3.5k
> >> for vsyscall_64.c, 4k for vsyscall_emu_64.S (the fake vsyscall
> >> page), some tiny amount of core mm code that supports a gate area,
> >> and possibly 4k for a wasted pagetable.  The latter is because the
> >> vsyscall addresses are misaligned and fit poorly in the fixmap.
> >>
> >> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> >
> > One minor nit below, but with or without that change,
> > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> >
> >> --- a/arch/x86/xen/mmu.c
> >> +++ b/arch/x86/xen/mmu.c
> >> @@ -1456,11 +1456,13 @@ static int xen_pgd_alloc(struct mm_struct *mm)
> >>               user_pgd = (pgd_t *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> >>               page->private = (unsigned long)user_pgd;
> >>
> >> +#ifdef CONFIG_X86_VSYSCALL_EMULATION
> >>               if (user_pgd != NULL) {
> >>                       user_pgd[pgd_index(VSYSCALL_ADDR)] =
> >>                               __pgd(__pa(level3_user_vsyscall) | _PAGE_TABLE);
> >>                       ret = 0;
> >>               }
> >> +#endif
> >
> > Could you instead make the if use IS_ENABLED?
> >
> >                 if (IS_ENABLED(CONFIG_X86_VSYSCALL_EMULATION) && user_pgd != NULL)
> >
> > That has the advantage of ensuring that the code continues to compile.
> > (Given that you haven't removed level3_user_vsyscall, that should work.)
> 
> I need the ret = 0, I think, so I'll resend.
> 
> I think I'd rather use #ifdef here, since I think it would be great if
> the Xen people could clean this up further.  With this change, under
> some configurations, there should be no user-accessible kernel
> addresses at all.  (Also, is there some PV mechanism
> that I'm not thinking of that will break with this change?  I know
> I've tripped over Xen pagetable and fixmap oddities before.)

Not that I know of. The vsyscall is the only one that I know of that
does this. 

Do you have a full patchset somewhere for testing?
> 
> --Andy

Re: [PATCH 3/3] x86_64,vsyscall: Make vsyscall emulation configurable

[Andy Lutomirski]

On Oct 29, 2014 1:00 PM, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:
>
> On Tue, Oct 28, 2014 at 11:09:53AM -0700, Andy Lutomirski wrote:
> > On Tue, Oct 28, 2014 at 10:57 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> > > On Tue, Oct 28, 2014 at 10:22:28AM -0700, Andy Lutomirski wrote:
> > >> This adds CONFIG_X86_VSYSCALL_EMULATION, guarded by CONFIG_EXPERT.
> > >> Turning it off completely disables vsyscall emulation, saving ~3.5k
> > >> for vsyscall_64.c, 4k for vsyscall_emu_64.S (the fake vsyscall
> > >> page), some tiny amount of core mm code that supports a gate area,
> > >> and possibly 4k for a wasted pagetable.  The latter is because the
> > >> vsyscall addresses are misaligned and fit poorly in the fixmap.
> > >>
> > >> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> > >
> > > One minor nit below, but with or without that change,
> > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > >
> > >> --- a/arch/x86/xen/mmu.c
> > >> +++ b/arch/x86/xen/mmu.c
> > >> @@ -1456,11 +1456,13 @@ static int xen_pgd_alloc(struct mm_struct *mm)
> > >>               user_pgd = (pgd_t *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> > >>               page->private = (unsigned long)user_pgd;
> > >>
> > >> +#ifdef CONFIG_X86_VSYSCALL_EMULATION
> > >>               if (user_pgd != NULL) {
> > >>                       user_pgd[pgd_index(VSYSCALL_ADDR)] =
> > >>                               __pgd(__pa(level3_user_vsyscall) | _PAGE_TABLE);
> > >>                       ret = 0;
> > >>               }
> > >> +#endif
> > >
> > > Could you instead make the if use IS_ENABLED?
> > >
> > >                 if (IS_ENABLED(CONFIG_X86_VSYSCALL_EMULATION) && user_pgd != NULL)
> > >
> > > That has the advantage of ensuring that the code continues to compile.
> > > (Given that you haven't removed level3_user_vsyscall, that should work.)
> >
> > I need the ret = 0, I think, so I'll resend.
> >
> > I think I'd rather use #ifdef here, since I think it would be great if
> > the Xen people could clean this up further.  With this change, under
> > some configurations, there should be no user-accessible kernel
> > addresses at all.  (Also, is there some PV mechanism
> > that I'm not thinking of that will break with this change?  I know
> > I've tripped over Xen pagetable and fixmap oddities before.)
>
> Not that I know of. The vsyscall is the only one that I know of that
> does this.

There's kvm-clock, too, but that may never co-exist with Xen.

I tagged v2 here:

https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/tag/?id=optional-vsyscall-emulation-v2

and I'll send it out in a bit.

--Andy

>
> Do you have a full patchset somewhere for testing?
> >
> > --Andy

Re: [PATCH 3/3] x86_64,vsyscall: Make vsyscall emulation configurable

[Konrad Rzeszutek Wilk]

On Wed, Oct 29, 2014 at 02:30:29PM -0700, Andy Lutomirski wrote:
> On Oct 29, 2014 1:00 PM, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:
> >
> > On Tue, Oct 28, 2014 at 11:09:53AM -0700, Andy Lutomirski wrote:
> > > On Tue, Oct 28, 2014 at 10:57 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> > > > On Tue, Oct 28, 2014 at 10:22:28AM -0700, Andy Lutomirski wrote:
> > > >> This adds CONFIG_X86_VSYSCALL_EMULATION, guarded by CONFIG_EXPERT.
> > > >> Turning it off completely disables vsyscall emulation, saving ~3.5k
> > > >> for vsyscall_64.c, 4k for vsyscall_emu_64.S (the fake vsyscall
> > > >> page), some tiny amount of core mm code that supports a gate area,
> > > >> and possibly 4k for a wasted pagetable.  The latter is because the
> > > >> vsyscall addresses are misaligned and fit poorly in the fixmap.
> > > >>
> > > >> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> > > >
> > > > One minor nit below, but with or without that change,
> > > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > > >
> > > >> --- a/arch/x86/xen/mmu.c
> > > >> +++ b/arch/x86/xen/mmu.c
> > > >> @@ -1456,11 +1456,13 @@ static int xen_pgd_alloc(struct mm_struct *mm)
> > > >>               user_pgd = (pgd_t *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> > > >>               page->private = (unsigned long)user_pgd;
> > > >>
> > > >> +#ifdef CONFIG_X86_VSYSCALL_EMULATION
> > > >>               if (user_pgd != NULL) {
> > > >>                       user_pgd[pgd_index(VSYSCALL_ADDR)] =
> > > >>                               __pgd(__pa(level3_user_vsyscall) | _PAGE_TABLE);
> > > >>                       ret = 0;
> > > >>               }
> > > >> +#endif
> > > >
> > > > Could you instead make the if use IS_ENABLED?
> > > >
> > > >                 if (IS_ENABLED(CONFIG_X86_VSYSCALL_EMULATION) && user_pgd != NULL)
> > > >
> > > > That has the advantage of ensuring that the code continues to compile.
> > > > (Given that you haven't removed level3_user_vsyscall, that should work.)
> > >
> > > I need the ret = 0, I think, so I'll resend.
> > >
> > > I think I'd rather use #ifdef here, since I think it would be great if
> > > the Xen people could clean this up further.  With this change, under
> > > some configurations, there should be no user-accessible kernel
> > > addresses at all.  (Also, is there some PV mechanism

What about the vsyscall time stamp (aka kvm-clock). That is not
really VSYSCALL emulation based but normal code?

> > > that I'm not thinking of that will break with this change?  I know
> > > I've tripped over Xen pagetable and fixmap oddities before.)
> >
> > Not that I know of. The vsyscall is the only one that I know of that
> > does this.
> 
> There's kvm-clock, too, but that may never co-exist with Xen.

It will eventually.
> 
> I tagged v2 here:
> 
> https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/tag/?id=optional-vsyscall-emulation-v2
> 
> and I'll send it out in a bit.
> 
> --Andy
> 
> >
> > Do you have a full patchset somewhere for testing?
> > >
> > > --Andy

Re: [PATCH 3/3] x86_64,vsyscall: Make vsyscall emulation configurable

[Andy Lutomirski]

On Dec 1, 2014 2:08 PM, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:
>
> On Wed, Oct 29, 2014 at 02:30:29PM -0700, Andy Lutomirski wrote:
> > On Oct 29, 2014 1:00 PM, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:
> > >
> > > On Tue, Oct 28, 2014 at 11:09:53AM -0700, Andy Lutomirski wrote:
> > > > On Tue, Oct 28, 2014 at 10:57 AM, Josh Triplett <josh@joshtriplett.org> wrote:
> > > > > On Tue, Oct 28, 2014 at 10:22:28AM -0700, Andy Lutomirski wrote:
> > > > >> This adds CONFIG_X86_VSYSCALL_EMULATION, guarded by CONFIG_EXPERT.
> > > > >> Turning it off completely disables vsyscall emulation, saving ~3.5k
> > > > >> for vsyscall_64.c, 4k for vsyscall_emu_64.S (the fake vsyscall
> > > > >> page), some tiny amount of core mm code that supports a gate area,
> > > > >> and possibly 4k for a wasted pagetable.  The latter is because the
> > > > >> vsyscall addresses are misaligned and fit poorly in the fixmap.
> > > > >>
> > > > >> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> > > > >
> > > > > One minor nit below, but with or without that change,
> > > > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > > > >
> > > > >> --- a/arch/x86/xen/mmu.c
> > > > >> +++ b/arch/x86/xen/mmu.c
> > > > >> @@ -1456,11 +1456,13 @@ static int xen_pgd_alloc(struct mm_struct *mm)
> > > > >>               user_pgd = (pgd_t *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> > > > >>               page->private = (unsigned long)user_pgd;
> > > > >>
> > > > >> +#ifdef CONFIG_X86_VSYSCALL_EMULATION
> > > > >>               if (user_pgd != NULL) {
> > > > >>                       user_pgd[pgd_index(VSYSCALL_ADDR)] =
> > > > >>                               __pgd(__pa(level3_user_vsyscall) | _PAGE_TABLE);
> > > > >>                       ret = 0;
> > > > >>               }
> > > > >> +#endif
> > > > >
> > > > > Could you instead make the if use IS_ENABLED?
> > > > >
> > > > >                 if (IS_ENABLED(CONFIG_X86_VSYSCALL_EMULATION) && user_pgd != NULL)
> > > > >
> > > > > That has the advantage of ensuring that the code continues to compile.
> > > > > (Given that you haven't removed level3_user_vsyscall, that should work.)
> > > >
> > > > I need the ret = 0, I think, so I'll resend.
> > > >
> > > > I think I'd rather use #ifdef here, since I think it would be great if
> > > > the Xen people could clean this up further.  With this change, under
> > > > some configurations, there should be no user-accessible kernel
> > > > addresses at all.  (Also, is there some PV mechanism
>
> What about the vsyscall time stamp (aka kvm-clock). That is not
> really VSYSCALL emulation based but normal code?

That's entirely separate now.

>
> > > > that I'm not thinking of that will break with this change?  I know
> > > > I've tripped over Xen pagetable and fixmap oddities before.)
> > >
> > > Not that I know of. The vsyscall is the only one that I know of that
> > > does this.
> >
> > There's kvm-clock, too, but that may never co-exist with Xen.
>
> It will eventually.

Hmm. Maybe I should clean up the read code first.

--Andy

> >
> > I tagged v2 here:
> >
> > https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/tag/?id=optional-vsyscall-emulation-v2
> >
> > and I'll send it out in a bit.
> >
> > --Andy
> >
> > >
> > > Do you have a full patchset somewhere for testing?
> > > >
> > > > --Andy