x86_64: Tidy up vsyscall emulation and make it optional

PATCH Superseded Andy Lutomirski 3 participants 17 messages Oct 28, 2014 – Dec 1, 2014

Subsystems (2)

x86 Xen hypervisor x86

lore

Share & tools

lore

Andy Lutomirski

author

2014-10-28 17:22 UTC #0 lore raw

cover x86_64: Tidy up vsyscall emulation and make it optional

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

Signature · 1 line

1.9.3

Andy Lutomirski

author

2014-10-28 17:22 UTC #1 lore raw

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

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 <[email protected]>

---
 arch/x86/kernel/vsyscall_64.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

arch/x86/kernel/vsyscall_64.c +9 -5

index 419e83b58436..2d912629c96e 100644
@@ -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);
 }

Signature · 1 line

1.9.3

Andy Lutomirski

author

2014-10-28 17:22 UTC #2 lore raw

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

In reply to 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 <[email protected]>

---
 arch/x86/kernel/vsyscall_64.c | 51 +++++++++++++++----------------------------
 1 file changed, 18 insertions(+), 33 deletions(-)

arch/x86/kernel/vsyscall_64.c +18 ~~-33~~

index 2d912629c96e..ee622f8183f3 100644
@@ -1,52 +1,37 @@
 /*
- *  Copyright (C) 2001 Andrea Arcangeli <[email protected]> SuSE
- *  Copyright 2003 Andi Kleen, SuSE Labs.
+ * Mostly copyright (c) 2012-2014 Andy Lutomirski <[email protected]>
  *
- *  [ 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 [email protected] 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 <[email protected]> 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

Signature · 1 line

1.9.3

Andy Lutomirski

author

2014-10-28 17:22 UTC #3 lore raw

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

In reply to 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 <[email protected]>

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

arch/x86/Kconfig +18 -0

index f2327e88e07c..cd10436d7d1c 100644
@@ -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

arch/x86/include/asm/fixmap.h +2 -0

index ffb1733ac91f..d8d5bcb2a0b5 100644
@@ -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,

arch/x86/include/asm/page_64.h +3 -1

index f408caf73430..b3bebf9e5746 100644
@@ -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 */

arch/x86/include/asm/vsyscall.h +8 -0

index 34f7d8857542..6ba66ee79710 100644
@@ -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 */

arch/x86/kernel/Makefile +1 -2

index 8f1e77440b2b..5d4502c8b983 100644
@@ -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

arch/x86/kernel/setup.c +0 -2

index 235cfd39e0d7..59a6f884fdad 100644
@@ -1190,9 +1190,7 @@ void __init setup_arch(char **cmdline_p)
 
 	tboot_probe();
 
-#ifdef CONFIG_X86_64
 	map_vsyscall();
-#endif
 
 	generic_apic_probe();

arch/x86/xen/mmu.c +4 -2

index a8a1a3d08d4d..5046b699eec1 100644
@@ -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) {

Signature · 1 line

1.9.3

Josh Triplett

Reviewed-by on v1 3/3

2014-10-28 17:57 UTC #4 lore raw

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

Quoted 8 lines · 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 <[email protected]>

One minor nit below, but with or without that change,

Reviewed-by Josh Triplett <[email protected]>

arch/x86/xen/mmu.c static int xen_pgd_alloc(struct mm_struct *mm) · line 1465

Quoted 13 lines

arch/x86/xen/mmu.c +2 -0

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

Andy Lutomirski

author

2014-10-28 18:10 UTC #5 lore raw

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

Quoted 29 lines · Josh Triplett

Quoted 8 lines · 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 <[email protected]>
One minor nit below, but with or without that change,
Reviewed-by Josh Triplett <[email protected]>
arch/x86/xen/mmu.c static int xen_pgd_alloc(struct mm_struct *mm) · line 1465
Quoted 13 lines
arch/x86/xen/mmu.c +2 -0
@@ -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?

Quoted 4 lines

                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

Andy Lutomirski

author

2014-10-28 18:10 UTC #6 lore raw

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

In reply to Josh Triplett

Quoted 29 lines · Josh Triplett

Quoted 8 lines · 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 <[email protected]>
One minor nit below, but with or without that change,
Reviewed-by Josh Triplett <[email protected]>
arch/x86/xen/mmu.c static int xen_pgd_alloc(struct mm_struct *mm) · line 1465
Quoted 13 lines
arch/x86/xen/mmu.c +2 -0
@@ -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?

Quoted 4 lines

                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.

--Andy

Konrad Rzeszutek Wilk

2014-10-29 20:00 UTC #7 lore raw

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

Quoted 35 lines · Andy Lutomirski

Quoted 29 lines · Josh Triplett
Quoted 8 lines · 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 <[email protected]>
One minor nit below, but with or without that change,
Reviewed-by Josh Triplett <[email protected]>
arch/x86/xen/mmu.c static int xen_pgd_alloc(struct mm_struct *mm) · line 1465
Quoted 13 lines
arch/x86/xen/mmu.c +2 -0
@@ -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?
Quoted 4 lines
                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.)

Quoted 8 lines

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?

Quoted 2 lines

--Andy

Konrad Rzeszutek Wilk

2014-10-29 20:00 UTC #8 lore raw

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

In reply to Andy Lutomirski

Quoted 35 lines · Andy Lutomirski

Quoted 29 lines · Josh Triplett
Quoted 8 lines · 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 <[email protected]>
One minor nit below, but with or without that change,
Reviewed-by Josh Triplett <[email protected]>
arch/x86/xen/mmu.c static int xen_pgd_alloc(struct mm_struct *mm) · line 1465
Quoted 13 lines
arch/x86/xen/mmu.c +2 -0
@@ -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?
Quoted 4 lines
                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.)

Quoted 8 lines

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?

Quoted 2 lines

--Andy

Andy Lutomirski

author

2014-10-29 21:30 UTC #9 lore raw

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

Quoted 46 lines · Konrad Rzeszutek Wilk

Quoted 35 lines · Andy Lutomirski
Quoted 29 lines · Josh Triplett
Quoted 8 lines · 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 <[email protected]>
One minor nit below, but with or without that change,
Reviewed-by Josh Triplett <[email protected]>
arch/x86/xen/mmu.c static int xen_pgd_alloc(struct mm_struct *mm) · line 1465
Quoted 13 lines
arch/x86/xen/mmu.c +2 -0
@@ -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?
Quoted 4 lines
                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.)
Quoted 8 lines
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.)

Quoted 2 lines

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

Quoted 4 lines

Do you have a full patchset somewhere for testing?
Quoted 2 lines
--Andy

Konrad Rzeszutek Wilk

2014-12-01 22:08 UTC #10 lore raw

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

Quoted 38 lines · Andy Lutomirski

Quoted 37 lines · Konrad Rzeszutek Wilk
Quoted 35 lines · Andy Lutomirski
Quoted 33 lines · Josh Triplett
Quoted 8 lines · 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 <[email protected]>
One minor nit below, but with or without that change,
Reviewed-by Josh Triplett <[email protected]>
arch/x86/xen/mmu.c static int xen_pgd_alloc(struct mm_struct *mm) · line 1465
Quoted 13 lines
arch/x86/xen/mmu.c +2 -0
@@ -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.)

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
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?

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
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.

Quoted 13 lines

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
Quoted 4 lines
Do you have a full patchset somewhere for testing?
Quoted 2 lines
--Andy

Konrad Rzeszutek Wilk

2014-12-01 22:08 UTC #11 lore raw

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

In reply to Andy Lutomirski

Quoted 38 lines · Andy Lutomirski

Quoted 37 lines · Konrad Rzeszutek Wilk
Quoted 35 lines · Andy Lutomirski
Quoted 33 lines · Josh Triplett
Quoted 8 lines · 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 <[email protected]>
One minor nit below, but with or without that change,
Reviewed-by Josh Triplett <[email protected]>
arch/x86/xen/mmu.c static int xen_pgd_alloc(struct mm_struct *mm) · line 1465
Quoted 13 lines
arch/x86/xen/mmu.c +2 -0
@@ -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.)

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
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?

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
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.

Quoted 13 lines

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
Quoted 4 lines
Do you have a full patchset somewhere for testing?
Quoted 2 lines
--Andy

Andy Lutomirski

author

2014-12-01 23:18 UTC #12 lore raw

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

Quoted 42 lines · Konrad Rzeszutek Wilk

Quoted 40 lines · Andy Lutomirski
Quoted 39 lines · Konrad Rzeszutek Wilk
Quoted 37 lines · Andy Lutomirski
Quoted 29 lines · Josh Triplett
Quoted 8 lines · 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 <[email protected]>
One minor nit below, but with or without that change,
Reviewed-by Josh Triplett <[email protected]>
arch/x86/xen/mmu.c static int xen_pgd_alloc(struct mm_struct *mm) · line 1465
Quoted 13 lines
arch/x86/xen/mmu.c +2 -0
@@ -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?
Quoted 4 lines
                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.

Quoted 7 lines

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
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.

Quoted 10 lines

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
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

Quoted 13 lines

Quoted 13 lines
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
Quoted 4 lines
Do you have a full patchset somewhere for testing?
Quoted 2 lines
--Andy

Andy Lutomirski

author

2014-12-01 23:18 UTC #13 lore raw

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

In reply to Konrad Rzeszutek Wilk

Quoted 42 lines · Konrad Rzeszutek Wilk

Quoted 40 lines · Andy Lutomirski
Quoted 39 lines · Konrad Rzeszutek Wilk
Quoted 37 lines · Andy Lutomirski
Quoted 29 lines · Josh Triplett
Quoted 8 lines · 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 <[email protected]>
One minor nit below, but with or without that change,
Reviewed-by Josh Triplett <[email protected]>
arch/x86/xen/mmu.c static int xen_pgd_alloc(struct mm_struct *mm) · line 1465
Quoted 13 lines
arch/x86/xen/mmu.c +2 -0
@@ -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?
Quoted 4 lines
                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.

Quoted 7 lines

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
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.

Quoted 10 lines

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
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

Quoted 13 lines

Quoted 13 lines
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
Quoted 4 lines
Do you have a full patchset somewhere for testing?
Quoted 2 lines
--Andy

Andy Lutomirski

author

2014-10-29 21:30 UTC #14 lore raw

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

In reply to Konrad Rzeszutek Wilk

Quoted 46 lines · Konrad Rzeszutek Wilk

Quoted 35 lines · Andy Lutomirski
Quoted 29 lines · Josh Triplett
Quoted 8 lines · 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 <[email protected]>
One minor nit below, but with or without that change,
Reviewed-by Josh Triplett <[email protected]>
arch/x86/xen/mmu.c static int xen_pgd_alloc(struct mm_struct *mm) · line 1465
Quoted 13 lines
arch/x86/xen/mmu.c +2 -0
@@ -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?
Quoted 4 lines
                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.)
Quoted 8 lines
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.)

Quoted 2 lines

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

Quoted 4 lines

Do you have a full patchset somewhere for testing?
Quoted 2 lines
--Andy

[email protected]

Reviewed-by Reviewed-by on the v1 series

2014-10-28 17:46 UTC #15 lore raw

In reply to Andy Lutomirski

Quoted 17 lines · 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.

Quoted 6 lines

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 <[email protected]>

For patch 3, I responded with a possible minor improvement, but with or without that:

Reviewed-by Josh Triplett <[email protected]>

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

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

- Josh Triplett

Andy Lutomirski

author

2014-10-28 18:05 UTC #16 lore raw

Quoted 25 lines · On Tue, Oct 28, 2014 at 10:46 AM, <[email protected]> wrote:

Quoted 17 lines · 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.
Quoted 6 lines
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

Quoted 9 lines

Nice!
For patches 1 and 2:
Reviewed-by Josh Triplett <[email protected]>
For patch 3, I responded with a possible minor improvement, but with or without that:
Reviewed-by Josh Triplett <[email protected]>

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.

Quoted 1 line

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

I think so.

Quoted 3 lines

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

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

--Andy

Quoted 1 line

- Josh Triplett

Signature · 2 lines

Andy Lutomirski
AMA Capital Management, LLC