x86-64: vsyscall emulation cleanups
Subsystems (2)
cover x86-64: vsyscall emulation cleanups
[cc list trimmed from the previous patches to include only people who seem
like they would care about this round of changes. My apologies if I missed anyone.]
This series applies to the x86/vdso branch of the -tip tree.
Patch 1/3 deletes some very outdated comments in vsyscall_64.c.
Patch 2/3 simplifies the vsyscall emulation int 0xcc mechanism and removes some ret instructions that are (as pointed out by the PaX team) possibly dangerous.
Patch 3/3 fixes what is arguably the most serious problem of all: people thought that CONFIG_COMPAT_VSYSCALLS controlled binary compatibility. It changes the Kconfig short description, help text, and feature removal entry to make it a lot more clear what's going on.
Andy Lutomirski (3):
x86-64: Fix outdated comments in vsyscall_64.c
x86-64: Clean up vsyscall emulation and remove fixed-address ret
x86-64: Clarify CONFIG_COMPAT_VSYSCALLS text
Documentation/feature-removal-schedule.txt | 11 ++- arch/x86/Kconfig | 27 +++++---- arch/x86/include/asm/vsyscall.h | 10 +++- arch/x86/kernel/vsyscall_64.c | 85 +++++++++------------------- arch/x86/kernel/vsyscall_emu_64.S | 17 +----- 5 files changed, 58 insertions(+), 92 deletions(-)
Signature · 1 line
1.7.5.2
PATCH 1/3 x86-64: Fix outdated comments in vsyscall_64.c
--- arch/x86/kernel/vsyscall_64.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
arch/x86/kernel/vsyscall_64.c +4 -3
@@ -11,10 +11,11 @@* 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. -* If we want more than four we need a vDSO. * -* Note: the concept clashes with user mode linux. If you use UML and -* want per guest time just set the kernel.vsyscall64 sysctl to 0. +* This mechanism is deprecated in favor of the vDSO. +* +* Note: the concept clashes with user mode linux. UML users should +* use the vDSO. */ /* Disable profiling for userspace code: */
Signature · 1 line
1.7.5.2
[tip:x86/vdso] x86-64: Fix outdated comments in vsyscall_64.c
Commit-ID: 8d6316596441de42e3f30c8e536579f6292b46a0 Gitweb: http://git.kernel.org/tip/8d6316596441de42e3f30c8e536579f6292b46a0 Author: Andy Lutomirski <[email protected]> AuthorDate: Mon, 6 Jun 2011 13:27:23 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 6 Jun 2011 19:47:20 +0200
x86-64: Fix outdated comments in vsyscall_64.c
--- arch/x86/kernel/vsyscall_64.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
arch/x86/kernel/vsyscall_64.c +4 -3
@@ -11,10 +11,11 @@* 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. -* If we want more than four we need a vDSO. * -* Note: the concept clashes with user mode linux. If you use UML and -* want per guest time just set the kernel.vsyscall64 sysctl to 0. +* This mechanism is deprecated in favor of the vDSO. +* +* Note: the concept clashes with user mode linux. UML users should +* use the vDSO. */ /* Disable profiling for userspace code: */
PATCH 2/3 x86-64: Clean up vsyscall emulation and remove fixed-address ret
In reply to Andy LutomirskiRemove the fixed-address ret in the vsyscall emulation stubs. As a result, int 0xcc no longer works if executed from user address space (which is OK -- nothing sensible should do that).
Removing support for int 0xcc in user address space cleans up the code considerably.
--- arch/x86/include/asm/vsyscall.h | 10 ++++- arch/x86/kernel/vsyscall_64.c | 78 +++++++++++-------------------------- arch/x86/kernel/vsyscall_emu_64.S | 17 +-------- 3 files changed, 32 insertions(+), 73 deletions(-)
arch/x86/include/asm/vsyscall.h +8 -2
@@ -32,9 +32,15 @@ extern struct timezone sys_tz;extern void map_vsyscall (void );/* Emulation */ -static inline bool in_vsyscall_page (unsigned long addr) + +static inline bool is_vsyscall_entry (unsigned long addr) +{ +return (addr & ~0xC00UL ) ==VSYSCALL_START ; +} + +static inline int vsyscall_entry_nr (unsigned long addr) { -return (addr & ~(PAGE_SIZE -1 )) ==VSYSCALL_START ; +return (addr &0xC00UL ) >>10 ; }#endif /* __KERNEL__ */
arch/x86/kernel/vsyscall_64.c +23 -55
138 lines not on this page · raw message
arch/x86/kernel/vsyscall_emu_64.S +1 -16
@@ -7,36 +7,21 @@ #include <linux/linkage.h> #include <asm/irq_vectors.h> -/* - * These magic incantations are chosen so that they fault if entered anywhere - * other than an instruction boundary. The movb instruction is two bytes, and - * the int imm8 instruction is also two bytes, so the only misaligned places - * to enter are the immediate values for the two instructions. 0xcc is int3 - * (always faults), 0xce is into (faults on x64-64, and 32-bit code can't get - * here), and 0xf0 is lock (lock int is invalid). - * - * The unused parts of the page are filled with 0xcc by the linker script. - */ +/* The unused parts of the page are filled with 0xcc by the linker script. */ .section .vsyscall_0, "a" ENTRY(vsyscall_0) - movb $0xcc, %al int $VSYSCALL_EMU_VECTOR - ret END(vsyscall_0) #ifndef CONFIG_COMPAT_VSYSCALLS .section .vsyscall_1, "a" ENTRY(vsyscall_1) - movb $0xce, %al int $VSYSCALL_EMU_VECTOR - ret END(vsyscall_1) #endif .section .vsyscall_2, "a" ENTRY(vsyscall_2) - movb $0xf0, %al int $VSYSCALL_EMU_VECTOR - ret END(vsyscall_2)
Signature · 1 line
1.7.5.2
Re: [PATCH 2/3] x86-64: Clean up vsyscall emulation and remove fixed-address ret
Quoted 7 lines · Andy Lutomirski
Remove the fixed-address ret in the vsyscall emulation stubs. As a result, int
0xccno longer works if executed from user address space (which is OK -- nothing sensible should do that).Removing support for int
0xccin user address space cleans up the code considerably.
Quoted 6 lines
Signed-off-by Andy Lutomirski <[email protected]>--- arch/x86/include/asm/vsyscall.h | 10 ++++- arch/x86/kernel/vsyscall_64.c | 78 +++++++++++-------------------------- arch/x86/kernel/vsyscall_emu_64.S | 17 +-------- 3 files changed, 32 insertions(+), 73 deletions(-)
Ok, this makes the whole series a lot more palatable!
Can i propagate the rename suggested by hpa into patch #3:
CONFIG_COMPAT_VSYSCALL => CONFIG_LEGACY_VSYSCALL
? 'legacy' is probably the best name for this and it thus won't be confused with the 32-bit compat facilities we have.
Thanks,
Ingo
Re: [PATCH 2/3] x86-64: Clean up vsyscall emulation and remove fixed-address ret
Quoted 17 lines · Ingo Molnar
Quoted 7 lines · Andy Lutomirski
Remove the fixed-address ret in the vsyscall emulation stubs. As a result, int
0xccno longer works if executed from user address space (which is OK -- nothing sensible should do that).Removing support for int
0xccin user address space cleans up the code considerably.Quoted 6 lines
Signed-off-by Andy Lutomirski <[email protected]>---
arch/x86/include/asm/vsyscall.h | 10 ++++-arch/x86/kernel/vsyscall_64.c| 78 +++++++++++--------------------------arch/x86/kernel/vsyscall_emu_64.S| 17 +--------3 files changed, 32 insertions(+), 73 deletions(-)
Quoted 5 lines
Ok, this makes the whole series a lot more palatable!
Can i propagate the rename suggested by hpa into patch #3:
CONFIG_COMPAT_VSYSCALL =>
CONFIG_LEGACY_VSYSCALL
Certainly! CONFIG_LEGACY_VTIME might be even better, but I'm fine with any renaming you'd like to do.
--Andy
Quoted 8 lines
? 'legacy' is probably the best name for this and it thus won't be confused with the 32-bit compat facilities we have.
Thanks,
Ingo
Re: [PATCH 2/3] x86-64: Clean up vsyscall emulation and remove fixed-address ret
Quoted 5 lines · Andrew Lutomirski
Quoted 5 lines
Quoted 4 lines
arch/x86/include/asm/vsyscall.h | 10 ++++-
arch/x86/kernel/vsyscall_64.c| 78 +++++++++++--------------------------arch/x86/kernel/vsyscall_emu_64.S| 17 +--------3 files changed, 32 insertions(+), 73 deletions(-)
Quoted 8 lines
Quoted 5 lines
Ok, this makes the whole series a lot more palatable!
Can i propagate the rename suggested by hpa into patch #3:
CONFIG_COMPAT_VSYSCALL =>
CONFIG_LEGACY_VSYSCALLCertainly!
CONFIG_LEGACY_VTIMEmight be even better, but I'm fine with any renaming you'd like to do.
Good point, i changed it to CONFIG_LEGACY_VTIME - this correctly implies that it's only about vtime(), not about any other vsyscall.
Thanks,
Ingo
[tip:x86/vdso] x86-64: Clean up vsyscall emulation and remove fixed-address ret
In reply to Andy LutomirskiCommit-ID: 7dc0452808b75615d1dc4d5160b26aaabc6a7300 Gitweb: http://git.kernel.org/tip/7dc0452808b75615d1dc4d5160b26aaabc6a7300 Author: Andy Lutomirski <[email protected]> AuthorDate: Mon, 6 Jun 2011 13:27:24 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 6 Jun 2011 19:47:20 +0200
x86-64: Clean up vsyscall emulation and remove fixed-address ret
Remove the fixed-address ret in the vsyscall emulation stubs. As a result, int 0xcc no longer works if executed from user address space (which is OK -- nothing sensible should do that).
Removing support for int 0xcc in user address space cleans up the code considerably.
--- arch/x86/include/asm/vsyscall.h | 10 ++++- arch/x86/kernel/vsyscall_64.c | 78 +++++++++++-------------------------- arch/x86/kernel/vsyscall_emu_64.S | 17 +-------- 3 files changed, 32 insertions(+), 73 deletions(-)
arch/x86/include/asm/vsyscall.h +8 -2
@@ -32,9 +32,15 @@ extern struct timezone sys_tz;extern void map_vsyscall (void );/* Emulation */ -static inline bool in_vsyscall_page (unsigned long addr) + +static inline bool is_vsyscall_entry (unsigned long addr) +{ +return (addr & ~0xC00UL ) ==VSYSCALL_START ; +} + +static inline int vsyscall_entry_nr (unsigned long addr) { -return (addr & ~(PAGE_SIZE -1 )) ==VSYSCALL_START ; +return (addr &0xC00UL ) >>10 ; }#endif /* __KERNEL__ */
arch/x86/kernel/vsyscall_64.c +23 -55
138 lines not on this page · raw message
arch/x86/kernel/vsyscall_emu_64.S +1 -16
@@ -7,36 +7,21 @@ #include <linux/linkage.h> #include <asm/irq_vectors.h> -/* - * These magic incantations are chosen so that they fault if entered anywhere - * other than an instruction boundary. The movb instruction is two bytes, and - * the int imm8 instruction is also two bytes, so the only misaligned places - * to enter are the immediate values for the two instructions. 0xcc is int3 - * (always faults), 0xce is into (faults on x64-64, and 32-bit code can't get - * here), and 0xf0 is lock (lock int is invalid). - * - * The unused parts of the page are filled with 0xcc by the linker script. - */ +/* The unused parts of the page are filled with 0xcc by the linker script. */ .section .vsyscall_0, "a" ENTRY(vsyscall_0) - movb $0xcc, %al int $VSYSCALL_EMU_VECTOR - ret END(vsyscall_0) #ifndef CONFIG_COMPAT_VSYSCALLS .section .vsyscall_1, "a" ENTRY(vsyscall_1) - movb $0xce, %al int $VSYSCALL_EMU_VECTOR - ret END(vsyscall_1) #endif .section .vsyscall_2, "a" ENTRY(vsyscall_2) - movb $0xf0, %al int $VSYSCALL_EMU_VECTOR - ret END(vsyscall_2)
PATCH 3/3 x86-64: Clarify CONFIG_COMPAT_VSYSCALLS text
In reply to Andy LutomirskiEnough people are confused about whether CONFIG_COMPAT_VSYSCALLS=n breaks binary compatibility that we should make it harder to be confused. So make it more clear what's going on.
The new text is a slight lie in that CONFIG_COMPAT_VSYSCALLS could make it slightly harder to exploit *kernel* bugs once the kernel address layout is randomized, but I mentioning that in the help text might just cause more confusion
--- Documentation/feature-removal-schedule.txt | 11 +++++++---- arch/x86/Kconfig | 27 +++++++++++++++------------ 2 files changed, 22 insertions(+), 16 deletions(-)
Documentation/feature-removal-schedule.txt +7 -4
@@ -602,11 +602,14 @@ Who: Laurent Pinchart <[email protected]> ---------------------------- What: CONFIG_COMPAT_VSYSCALLS (x86_64) -When: When glibc 2.14 or newer is ubitquitous. Perhaps mid-2012. +When: When glibc 2.14 or newer is ubiquitous. Perhaps 2013. Why: Having user-executable syscall invoking code at a fixed addresses makes - it easier for attackers to exploit security holes. - Turning off CONFIG_COMPAT_VSYSCALLS mostly removes the risk but will - make the time() function slower on glibc versions 2.13 and below. + it easier for attackers to exploit security holes. Turning off + CONFIG_COMPAT_VSYSCALLS reduces the risk without breaking binary + compatibility but will make the time() function slower on glibc + versions 2.13 and below. + + We may flip the default setting to N before 2013. Who: Andy Lutomirski <[email protected]> ----------------------------
arch/x86/Kconfig +15 -12
@@ -1648,24 +1648,27 @@ config COMPAT_VDSO config COMPAT_VSYSCALLS def_bool y - prompt "Fixed address legacy vsyscalls" + prompt "Fast legacy sys_time() vsyscall" depends on X86_64 ---help--- - Legacy user code expects to be able to issue three syscalls - by calling a fixed addresses. If you say N, then the kernel - traps and emulates these calls. If you say Y, then there is - actual executable code at a fixed address to implement time() - efficiently. + Glibc 2.13 and older, statically linked binaries, and a few + other things use a legacy ABI to implement time(). - On a system with recent enough glibc (probably 2.14 or - newer) and no static binaries, you can say N without a - performance penalty to improve security: having no fixed - address userspace-executable syscall invoking code makes - it harder for both remote and local attackers to exploit - security holes. + If you say N here, the kernel will emulate that interface in + order to make certain types of userspace bugs more difficult + to exploit. This will cause some legacy software to run + slightly more slowly. + + If you say Y here, then the kernel will provide native code to + allow legacy programs to run without any performance impact. + This could make it easier to exploit certain types of + userspace bugs. If unsure, say Y. + NOTE: disabling this option will not break any ABI; the kernel + will be fully compatible with all binaries either way. + config CMDLINE_BOOL bool "Built-in kernel command line" ---help---
Signature · 1 line
1.7.5.2
[tip:x86/vdso] x86-64: Rename COMPAT_VSYSCALLS to LEGACY_VTIME and clarify documentation
Commit-ID: feba7e97df8c463331071b79fba2164ead6aa14b Gitweb: http://git.kernel.org/tip/feba7e97df8c463331071b79fba2164ead6aa14b Author: Andy Lutomirski <[email protected]> AuthorDate: Mon, 6 Jun 2011 13:27:25 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 6 Jun 2011 19:49:31 +0200
x86-64: Rename COMPAT_VSYSCALLS to LEGACY_VTIME and clarify documentation
Rename COMPAT_VSYSCALLS to LEGACY_VTIME to make sure it's not confused with the 32-bit compat facilities we have on x86-64.
Also, in the discussions enough people were confused about whether LEGACY_VTIME=n breaks binary compatibility that we should make it harder to be confused. So make it more clear what's going on.
[ The new text is slightly inaccurate in that LEGACY_VTIME
could make it slightly harder to exploit *kernel* bugs once the kernel address layout is randomized, but me mentioning that in the help text might just cause more confusion. ]
--- Documentation/feature-removal-schedule.txt | 13 +++++++---- arch/x86/Kconfig | 29 +++++++++++++++------------ arch/x86/kernel/vsyscall_64.c | 6 ++-- arch/x86/kernel/vsyscall_emu_64.S | 2 +- 4 files changed, 28 insertions(+), 22 deletions(-)
Documentation/feature-removal-schedule.txt +8 -5
@@ -601,12 +601,15 @@ Who: Laurent Pinchart <[email protected]> ---------------------------- -What: CONFIG_COMPAT_VSYSCALLS (x86_64) -When: When glibc 2.14 or newer is ubitquitous. Perhaps mid-2012. +What: CONFIG_LEGACY_VTIME (x86_64) +When: When glibc 2.14 or newer is ubiquitous. Perhaps 2013. Why: Having user-executable syscall invoking code at a fixed addresses makes - it easier for attackers to exploit security holes. - Turning off CONFIG_COMPAT_VSYSCALLS mostly removes the risk but will - make the time() function slower on glibc versions 2.13 and below. + it easier for attackers to exploit security holes. Turning off + CONFIG_LEGACY_VTIME reduces the risk without breaking binary + compatibility but will make the time() function slightly slower on + glibc versions 2.13 and below. + + We may flip the default setting to N before 2013. Who: Andy Lutomirski <[email protected]> ----------------------------
arch/x86/Kconfig +16 -13
@@ -1646,26 +1646,29 @@ config COMPAT_VDSO If unsure, say Y. -config COMPAT_VSYSCALLS +config LEGACY_VTIME def_bool y - prompt "Fixed address legacy vsyscalls" + prompt "Fast legacy sys_time() vsyscall" depends on X86_64 ---help--- - Legacy user code expects to be able to issue three syscalls - by calling a fixed addresses. If you say N, then the kernel - traps and emulates these calls. If you say Y, then there is - actual executable code at a fixed address to implement time() - efficiently. + Glibc 2.13 and older, statically linked binaries, and a few + other things use a legacy ABI to implement time(). - On a system with recent enough glibc (probably 2.14 or - newer) and no static binaries, you can say N without a - performance penalty to improve security: having no fixed - address userspace-executable syscall invoking code makes - it harder for both remote and local attackers to exploit - security holes. + If you say N here, the kernel will emulate that interface in + order to make certain types of userspace bugs more difficult + to exploit. This will cause some legacy software to run + slightly more slowly. + + If you say Y here, then the kernel will provide native code to + allow legacy programs to run without any performance impact. + This could make it easier to exploit certain types of + userspace bugs. If unsure, say Y. + NOTE: disabling this option will not break any ABI; the kernel + will be fully compatible with all binaries either way. + config CMDLINE_BOOL bool "Built-in kernel command line" ---help---
arch/x86/kernel/vsyscall_64.c +3 -3
@@ -102,7 +102,7 @@ static void warn_bad_vsyscall(const char *level, struct pt_regs *regs, regs->ip - 2, regs->sp, regs->ax, regs->si, regs->di); } -#ifdef CONFIG_COMPAT_VSYSCALLS +#ifdef CONFIG_LEGACY_VTIME /* This will break when the xtime seconds get inaccurate, but that is * unlikely */
@@ -124,7 +124,7 @@ vtime(time_t *t)return result; } -#endif /* CONFIG_COMPAT_VSYSCALLS */ +#endif /* CONFIG_LEGACY_VTIME */ void dotraplinkage do_emulate_vsyscall(struct pt_regs *regs, long error_code) {
@@ -169,7 +169,7 @@ void dotraplinkage do_emulate_vsyscall(struct pt_regs *regs, long error_code) (struct timezone __user *)regs->si);break ; -#ifndef CONFIG_COMPAT_VSYSCALLS +#ifndef CONFIG_LEGACY_VTIME case 1 : vsyscall_name ="time" ; ret =sys_time ((time_t __user *)regs->di);
arch/x86/kernel/vsyscall_emu_64.S +1 -1
@@ -14,7 +14,7 @@ ENTRY(vsyscall_0) int $VSYSCALL_EMU_VECTOR END(vsyscall_0) -#ifndef CONFIG_COMPAT_VSYSCALLS +#ifndef CONFIG_LEGACY_VTIME .section .vsyscall_1, "a" ENTRY(vsyscall_1) int $VSYSCALL_EMU_VECTOR