Remove syscall instructions at fixed addresses

Move vvars out of the vsyscall page into their own page and mark it NX.

Without this patch, an attacker who can force a daemon to call some fixed address could wait until the time contains, say, 0xCD80, and then execute the current time.

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/include/asm/fixmap.h        |    1 +
 arch/x86/include/asm/pgtable_types.h |    2 ++
 arch/x86/include/asm/vvar.h          |   22 ++++++++++------------
 arch/x86/kernel/vmlinux.lds.S        |   27 ++++++++++++++++-----------
 arch/x86/kernel/vsyscall_64.c        |    5 +++++
 5 files changed, 34 insertions(+), 23 deletions(-)

arch/x86/kernel/vmlinux.lds.S +1 -11

index 89aed99..3d89a00 100644
@@ -161,12 +161,6 @@ SECTIONS
 
 #define VVIRT_OFFSET (VSYSCALL_ADDR - __vsyscall_0)
 #define VVIRT(x) (ADDR(x) - VVIRT_OFFSET)
-#define EMIT_VVAR(x, offset) .vsyscall_var_ ## x	\
-	ADDR(.vsyscall_0) + offset		 	\
-	: AT(VLOAD(.vsyscall_var_ ## x)) {     		\
-		*(.vsyscall_var_ ## x)			\
-	}						\
-	x = VVIRT(.vsyscall_var_ ## x);
 
 	. = ALIGN(4096);
 	__vsyscall_0 = .;

@@ -192,17 +186,28 @@ SECTIONS
 		*(.vsyscall_3)
 	}
 
-#define __VVAR_KERNEL_LDS
-#include <asm/vvar.h>
-#undef __VVAR_KERNEL_LDS
-
-	. = __vsyscall_0 + PAGE_SIZE;
+	. = ALIGN(__vsyscall_0 + PAGE_SIZE, PAGE_SIZE);
 
 #undef VSYSCALL_ADDR
 #undef VLOAD_OFFSET

arch/x86/kernel/vmlinux.lds.S +8 -0

+	} :data
+
+#define __VVAR_KERNEL_LDS
+#include <asm/vvar.h>
+#undef __VVAR_KERNEL_LDS
+
+       . = ALIGN(__vvar_page + PAGE_SIZE, PAGE_SIZE);
+
 #undef EMIT_VVAR
 
 #endif /* CONFIG_X86_64 */

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

index 3e68218..3cf1cef 100644
@@ -284,9 +284,14 @@ void __init map_vsyscall(void)
 {
 	extern char __vsyscall_0;
 	unsigned long physaddr_page0 = __pa_symbol(&__vsyscall_0);
+	extern char __vvar_page;
+	unsigned long physaddr_vvar_page = __pa_symbol(&__vvar_page);
 
 	/* Note that VSYSCALL_MAPPED_PAGES must agree with the code below. */
 	__set_fixmap(VSYSCALL_FIRST_PAGE, physaddr_page0, PAGE_KERNEL_VSYSCALL);
+	__set_fixmap(VVAR_PAGE, physaddr_vvar_page, PAGE_KERNEL_VVAR);
+	BUILD_BUG_ON((unsigned long)__fix_to_virt(VVAR_PAGE) !=
+		     (unsigned long)VVAR_ADDRESS);
 }
 
 static int __init vsyscall_init(void)

Signature · 7 lines

1.7.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

This is not a security feature. It's to prevent the int 0xcc sequence from becoming ABI.

Quoted 2 lines · Andy Lutomirski

This is not a security feature.  It's to prevent the int 0xcc sequence from becoming ABI.

Hm, what kind of ABI reliance could be built on it?

arch/x86/kernel/vsyscall_64.c void __init map_vsyscall(void) · line 336

Quoted 6 lines

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

+static void __init mangle_vsyscall_movb(void *mapping,
+                                     unsigned long movb_addr, u8 initial)
+{
+     u8 *imm8;
+     BUG_ON(movb_addr >= 4095);

Please put newlines after local variable definitions.

Quoted 4 lines

 static int __init vsyscall_init(void)
 {
+     extern char __vsyscall_0;

Please don't put extern definitions in the middle of a .c file - if then it should be in a .h file. (even if only a single function uses it)

Quoted 12 lines

+     /*
+      * Randomize the magic al values for int 0xcc invocation.  This
+      * isn't really a security feature; it's to make sure that
+      * dynamic binary instrumentation tools don't start to think
+      * that the int 0xcc magic incantation is ABI.
+      */
+     vsyscall_nr_offset = get_random_int() % 3;
+     vsyscall_page = pfn_to_page(__pa_symbol(&__vsyscall_0) >> PAGE_SHIFT);
+     mapping = kmap_atomic(vsyscall_page);
+     /* It's easier to hardcode the addresses -- they're ABI. */
+     mangle_vsyscall_movb(mapping, 0, 0xcc);

what about filling it with zeroes?

arch/x86/kernel/vsyscall_64.c void __init map_vsyscall(void) · line 366

Quoted 5 lines

arch/x86/kernel/vsyscall_64.c +4 -0

+#ifndef CONFIG_UNSAFE_VSYSCALLS
+     mangle_vsyscall_movb(mapping, 1024, 0xce);
+#endif
+     mangle_vsyscall_movb(mapping, 2048, 0xf0);

Dunno, this all looks rather ugly.

Thanks,

       Ingo

We could scrap int 0xcc entirely and emulate on page fault, but that is slower and has other problems (like breaking anything that thinks it can look at a call target in a binary and dereference that address).

Here's a possibly dumb/evil idea:

Put real syscalls in the vsyscall page but mark the page NX.  Then emulate the vsyscalls on the PF_INSTR fault when userspace jumps to the correct address but send SIGSEGV for the wrong address.

Down side: it's even more complexity for the same silly case.

--Andy

Quoted 5 lines

Thanks,

       Ingo

Quoted 7 lines · Andrew Lutomirski

We could scrap int 0xcc entirely and emulate on page fault, but that is slower and has other problems (like breaking anything that thinks it can look at a call target in a binary and dereference that address).

Here's a possibly dumb/evil idea:

Quoted 5 lines

Put real syscalls in the vsyscall page but mark the page NX.  Then emulate the vsyscalls on the PF_INSTR fault when userspace jumps to the correct address but send SIGSEGV for the wrong address.

Down side: it's even more complexity for the same silly case.

Scratch that. It's incompatible with keeping time() fast for now.

Quoted 4 lines

Quoted 3 lines

 static int __init vsyscall_init(void)
 {
+     extern char __vsyscall_0;

Quoted 3 lines

Please don't put extern definitions in the middle of a .c file - if then it should be in a .h file. (even if only a single function uses it)

I thought the convention (and existing practice in vsyscall_64.c) was that if the extern reference is to a magic linker symbol then it goes in the function that uses it. But I can find it a header file.

Quoted 12 lines

Quoted 11 lines

+     /*
+      * Randomize the magic al values for int 0xcc invocation.  This
+      * isn't really a security feature; it's to make sure that
+      * dynamic binary instrumentation tools don't start to think
+      * that the int 0xcc magic incantation is ABI.
+      */
+     vsyscall_nr_offset = get_random_int() % 3;
+     vsyscall_page = pfn_to_page(__pa_symbol(&__vsyscall_0) >> PAGE_SHIFT);
+     mapping = kmap_atomic(vsyscall_page);
+     /* It's easier to hardcode the addresses -- they're ABI. */
+     mangle_vsyscall_movb(mapping, 0, 0xcc);

Quoted 1 line

what about filling it with zeroes?

Fill what with zeroes? I'm just patching one byte here.

Quoted 6 lines

arch/x86/kernel/vsyscall_64.c void __init map_vsyscall(void) · line 366

Quoted 4 lines

arch/x86/kernel/vsyscall_64.c +4 -0

+#ifndef CONFIG_UNSAFE_VSYSCALLS
+     mangle_vsyscall_movb(mapping, 1024, 0xce);
+#endif
+     mangle_vsyscall_movb(mapping, 2048, 0xf0);

Dunno, this all looks rather ugly.

Agreed. Better ideas are welcome.

We could scrap int 0xcc entirely and emulate on page fault, but that is slower and has other problems (like breaking anything that thinks it can look at a call target in a binary and dereference that address).

Here's a possibly dumb/evil idea:

Put real syscalls in the vsyscall page but mark the page NX. Then emulate the vsyscalls on the PF_INSTR fault when userspace jumps to the correct address but send SIGSEGV for the wrong address.

Down side: it's even more complexity for the same silly case.

Quoted 4 lines · Andrew Lutomirski

Quoted 4 lines

Quoted 3 lines

static int __init vsyscall_init(void)
{
+ extern char __vsyscall_0;

Quoted 8 lines

Quoted 3 lines

Please don't put extern definitions in the middle of a .c file - if then it should be in a .h file. (even if only a single function uses it)

I thought the convention (and existing practice in vsyscall_64.c) was that if the extern reference is to a magic linker symbol then it goes in the function that uses it. But I can find it a header file.

i'd suggest collecting them into a vsyscall header. The problem with externs in .c is that the moment two .c files start using it there's the danger of type divergence.

Quoted 12 lines

Quoted 12 lines

Quoted 11 lines

arch/x86/kernel/vsyscall_64.c +11 -0

+     /*
+      * Randomize the magic al values for int 0xcc invocation.  This
+      * isn't really a security feature; it's to make sure that
+      * dynamic binary instrumentation tools don't start to think
+      * that the int 0xcc magic incantation is ABI.
+      */
+     vsyscall_nr_offset = get_random_int() % 3;
+     vsyscall_page = pfn_to_page(__pa_symbol(&__vsyscall_0) >> PAGE_SHIFT);
+     mapping = kmap_atomic(vsyscall_page);
+     /* It's easier to hardcode the addresses -- they're ABI. */
+     mangle_vsyscall_movb(mapping, 0, 0xcc);

Quoted 3 lines

Quoted 1 line

what about filling it with zeroes?

Fill what with zeroes? I'm just patching one byte here.

Sigh, i suck at reading comprehension today!

Quoted 8 lines

Quoted 6 lines

arch/x86/kernel/vsyscall_64.c void __init map_vsyscall(void) · line 366

Quoted 4 lines

arch/x86/kernel/vsyscall_64.c +4 -0

+#ifndef CONFIG_UNSAFE_VSYSCALLS
+     mangle_vsyscall_movb(mapping, 1024, 0xce);
+#endif
+     mangle_vsyscall_movb(mapping, 2048, 0xf0);

Dunno, this all looks rather ugly.

Agreed. Better ideas are welcome.

None at the moment except "don't randomize it and see where the chips may fall". I'd rather live with a somewhat sticky default-off compat Kconfig switch than some permanently ugly randomization to make the transition to no-vsyscall faster.

It's not like we'll be able to remove the vsyscall altogether from the kernel - the best we can hope for is to be able to flip the default - there's binaries out there today that rely on it and binaries are sticky - a few months ago i saw someone test-running 1995 binaries ;-)

Btw., we could also make the vsyscall page vanish *runtime*, via a sysctl. That way distros only need to update their /etc/sysctl.conf.

Quoted 11 lines

We could scrap int 0xcc entirely and emulate on page fault, but that is slower and has other problems (like breaking anything that thinks it can look at a call target in a binary and dereference that address).

Here's a possibly dumb/evil idea:

Put real syscalls in the vsyscall page but mark the page NX. Then emulate the vsyscalls on the PF_INSTR fault when userspace jumps to the correct address but send SIGSEGV for the wrong address.

Quoted 1 line

Down side: it's even more complexity for the same silly case.

heh, you are good at coming up with sick ideas! ;-)

I don't think we want to add another branch to #PF, but could we turn this into #GP or perhaps an illegal instruction fault?

Should be benchmarked:

- The advantage of INT 0xCC is that it's completely isolated: it does not slow down anything else.

- doing this through #GP might be significantly slower cycle-wise. Do we know by how much?

The advantage would be that we would not waste an extra vector, it would be smaller, plus it would be rather simple to make it all a runtime toggle via a sysctl.

Documentation/feature-removal-schedule.txt Why: Superseded by the UVCIOC_CTRL_QUERY ioctl. · line 608

Quoted 5 lines

Documentation/feature-removal-schedule.txt +5 -0

+What:	CONFIG_UNSAFE_VSYSCALLS (x86_64)
+When:	When glibc 2.14 or newer is ubitquitous.  Perhaps mid-2012.
+Why:	Having user-executable code at a fixed address is a security problem.
+	Turning off CONFIG_UNSAFE_VSYSCALLS mostly removes the risk but will
+	make the time() function slower on glibc versions 2.13 and below.

I disagree with this description (and the whole idea really)

First it's time+gettimeofday+vgetcu, not just time.

A more accurate description is

"will make all x86-64 Linux programs written to the original pre vDSO syscall ABI significantly slower"

And the assumption that all world is using glibc is still as bad as it was on the first po.st

And it's still a bad idea. Especially since there's a much better alternative anyways for the "security problem" which has none of these drawbacks.

Documentation/feature-removal-schedule.txt Why: Superseded by the UVCIOC_CTRL_QUERY ioctl. · line 608

Quoted 5 lines

Documentation/feature-removal-schedule.txt +5 -0

+What:        CONFIG_UNSAFE_VSYSCALLS (x86_64)
+When:        When glibc 2.14 or newer is ubitquitous.  Perhaps mid-2012.
+Why: Having user-executable code at a fixed address is a security problem.
+     Turning off CONFIG_UNSAFE_VSYSCALLS mostly removes the risk but will
+     make the time() function slower on glibc versions 2.13 and below.

I disagree with this description (and the whole idea really)

First it's time+gettimeofday+vgetcu, not just time.

A more accurate description is

"will make all x86-64 Linux programs written to the original pre vDSO syscall ABI significantly slower"

And the assumption that all world is using glibc is still as bad as it was on the first po.st

And it's still a bad idea. Especially since there's a much better alternative anyways for the "security problem" which has none of these drawbacks.

Quoted 3 lines

And it's still a bad idea. Especially since there's a much better alternative anyways for the "security problem" which has none of these drawbacks.

What's the alternative?

[...] solution that filters the caller RIP at the generic syscall entry point and checks RCX against the 'expected' SYSCALL instruction address, which is the (per task) vdso-address + constant-offset.

Quoted 4 lines · Ingo Molnar

[...] solution that filters the caller RIP at the generic syscall entry point and checks RCX against the 'expected' SYSCALL instruction address, which is the (per task) vdso-address + constant-offset.

Note that this solution would allow the vsyscall page to be 'filtered' to the 3 allowed system calls rather efficiently, via a second level check.

This second check does not affect the fastpath, and it could be put behind a CONFIG_COMPAT_VSYSCALL deprecation define once glibc does not put vsyscall references anywhere - but we could even keep it around forever, as this way it's defanged permanently.

Thanks,

Ingo

[Sorry, possible resend.]

Quoted 12 lines · Ingo Molnar

Quoted 4 lines · Ingo Molnar

[...] solution that filters the caller RIP at the generic syscall entry point and checks RCX against the 'expected' SYSCALL instruction address, which is the (per task) vdso-address + constant-offset.

Note that this solution would allow the vsyscall page to be 'filtered' to the 3 allowed system calls rather efficiently, via a second level check.

Quoted 5 lines

This second check does not affect the fastpath, and it could be put behind a CONFIG_COMPAT_VSYSCALL deprecation define once glibc does not put vsyscall references anywhere - but we could even keep it around forever, as this way it's defanged permanently.

Are you thinking about the 32-bit vDSO? I think that 64-bit code puts syscalls instructions all over the place.

Quoted 4 lines · Ingo Molnar

[...] solution that filters the caller RIP at the generic syscall entry point and checks RCX against the 'expected' SYSCALL instruction address, which is the (per task) vdso-address + constant-offset.

Note that this solution would allow the vsyscall page to be 'filtered' to the 3 allowed system calls rather efficiently, via a second level check.

This second check does not affect the fastpath, and it could be put behind a CONFIG_COMPAT_VSYSCALL deprecation define once glibc does not put vsyscall references anywhere - but we could even keep it around forever, as this way it's defanged permanently.

Thanks,

       Ingo

Quoted 3 lines · Andi Kleen

And it's still a bad idea. Especially since there's a much better alternative anyways for the "security problem" which has none of these drawbacks.

What's the alternative?