* [patch 0/3] x86/vdso: Fix enable inconsistencies and cleanup @ 2017-04-10 15:14 Thomas Gleixner 2017-04-10 15:14 ` [patch 1/3] x86/vdso: Ensure vdso32_enabled gets set to valid values only Thomas Gleixner ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Thomas Gleixner @ 2017-04-10 15:14 UTC (permalink / raw) To: LKML; +Cc: x86, Peter Zijlstra, Andy Lutomirski, Mathias Krause The following series is a collection of Mathias enable fix, the plug of the sysctl race and a cleanup of the vdso*_enable handling. Thanks, tglx ^ permalink raw reply [flat|nested] 13+ messages in thread
* [patch 1/3] x86/vdso: Ensure vdso32_enabled gets set to valid values only 2017-04-10 15:14 [patch 0/3] x86/vdso: Fix enable inconsistencies and cleanup Thomas Gleixner @ 2017-04-10 15:14 ` Thomas Gleixner 2017-04-10 16:34 ` [tip:x86/urgent] " tip-bot for Mathias Krause 2017-04-10 15:14 ` [patch 2/3] x86/vdso: Plug race between mapping and ELF header setup Thomas Gleixner 2017-04-10 15:14 ` [patch 3/3] x86/vdso: Sanitize vdso*_enabled handling Thomas Gleixner 2 siblings, 1 reply; 13+ messages in thread From: Thomas Gleixner @ 2017-04-10 15:14 UTC (permalink / raw) To: LKML Cc: x86, Peter Zijlstra, Andy Lutomirski, Mathias Krause, Roland McGrath, stable [-- Attachment #1: x86vdso_Ensure_vdso32_enabled_gets_set_to_valid_values_only.patch --] [-- Type: text/plain, Size: 1870 bytes --] From: Mathias Krause <minipli@googlemail.com> vdso_enabled can be set to arbitrary integer values via the kernel command line 'vdso32=' parameter or via 'sysctl abi.vsyscall32'. load_vdso32() only maps VDSO if vdso_enabled == 1, but ARCH_DLINFO_IA32 merily checks for vdso_enabled != 0. As a consequence the AT_SYSINFO_EHDR auxiliary vector for the VDSO_ENTRY is emitted with a NULL pointer which causes a segfault when the application tries to use the VDSO. Restrict the valid arguments on the command line and the sysctl to 0 and 1. Fixes: b0b49f2673f0 ("x86, vdso: Remove compat vdso support") Signed-off-by: Mathias Krause <minipli@googlemail.com> Acked-by: Andy Lutomirski <luto@amacapital.net> Cc: Roland McGrath <roland@redhat.com> Cc: stable@vger.kernel.org Link: http://lkml.kernel.org/r/1491424561-7187-1-git-send-email-minipli@googlemail.com Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/entry/vdso/vdso32-setup.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) --- a/arch/x86/entry/vdso/vdso32-setup.c +++ b/arch/x86/entry/vdso/vdso32-setup.c @@ -30,8 +30,10 @@ static int __init vdso32_setup(char *s) { vdso32_enabled = simple_strtoul(s, NULL, 0); - if (vdso32_enabled > 1) + if (vdso32_enabled > 1) { pr_warn("vdso32 values other than 0 and 1 are no longer allowed; vdso disabled\n"); + vdso32_enabled = 0; + } return 1; } @@ -62,13 +64,18 @@ subsys_initcall(sysenter_setup); /* Register vsyscall32 into the ABI table */ #include <linux/sysctl.h> +static const int zero; +static const int one = 1; + static struct ctl_table abi_table2[] = { { .procname = "vsyscall32", .data = &vdso32_enabled, .maxlen = sizeof(int), .mode = 0644, - .proc_handler = proc_dointvec + .proc_handler = proc_dointvec_minmax, + .extra1 = (int *)&zero, + .extra2 = (int *)&one, }, {} }; ^ permalink raw reply [flat|nested] 13+ messages in thread
* [tip:x86/urgent] x86/vdso: Ensure vdso32_enabled gets set to valid values only 2017-04-10 15:14 ` [patch 1/3] x86/vdso: Ensure vdso32_enabled gets set to valid values only Thomas Gleixner @ 2017-04-10 16:34 ` tip-bot for Mathias Krause 0 siblings, 0 replies; 13+ messages in thread From: tip-bot for Mathias Krause @ 2017-04-10 16:34 UTC (permalink / raw) To: linux-tip-commits Cc: minipli, linux-kernel, peterz, mingo, luto, hpa, tglx, roland Commit-ID: c06989da39cdb10604d572c8c7ea8c8c97f3c483 Gitweb: http://git.kernel.org/tip/c06989da39cdb10604d572c8c7ea8c8c97f3c483 Author: Mathias Krause <minipli@googlemail.com> AuthorDate: Mon, 10 Apr 2017 17:14:27 +0200 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Mon, 10 Apr 2017 18:31:41 +0200 x86/vdso: Ensure vdso32_enabled gets set to valid values only vdso_enabled can be set to arbitrary integer values via the kernel command line 'vdso32=' parameter or via 'sysctl abi.vsyscall32'. load_vdso32() only maps VDSO if vdso_enabled == 1, but ARCH_DLINFO_IA32 merily checks for vdso_enabled != 0. As a consequence the AT_SYSINFO_EHDR auxiliary vector for the VDSO_ENTRY is emitted with a NULL pointer which causes a segfault when the application tries to use the VDSO. Restrict the valid arguments on the command line and the sysctl to 0 and 1. Fixes: b0b49f2673f0 ("x86, vdso: Remove compat vdso support") Signed-off-by: Mathias Krause <minipli@googlemail.com> Acked-by: Andy Lutomirski <luto@amacapital.net> Cc: Peter Zijlstra <peterz@infradead.org> Cc: stable@vger.kernel.org Cc: Roland McGrath <roland@redhat.com> Link: http://lkml.kernel.org/r/1491424561-7187-1-git-send-email-minipli@googlemail.com Link: http://lkml.kernel.org/r/20170410151723.518412863@linutronix.de Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/entry/vdso/vdso32-setup.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/arch/x86/entry/vdso/vdso32-setup.c b/arch/x86/entry/vdso/vdso32-setup.c index 7853b53..3f9d1a8 100644 --- a/arch/x86/entry/vdso/vdso32-setup.c +++ b/arch/x86/entry/vdso/vdso32-setup.c @@ -30,8 +30,10 @@ static int __init vdso32_setup(char *s) { vdso32_enabled = simple_strtoul(s, NULL, 0); - if (vdso32_enabled > 1) + if (vdso32_enabled > 1) { pr_warn("vdso32 values other than 0 and 1 are no longer allowed; vdso disabled\n"); + vdso32_enabled = 0; + } return 1; } @@ -62,13 +64,18 @@ subsys_initcall(sysenter_setup); /* Register vsyscall32 into the ABI table */ #include <linux/sysctl.h> +static const int zero; +static const int one = 1; + static struct ctl_table abi_table2[] = { { .procname = "vsyscall32", .data = &vdso32_enabled, .maxlen = sizeof(int), .mode = 0644, - .proc_handler = proc_dointvec + .proc_handler = proc_dointvec_minmax, + .extra1 = (int *)&zero, + .extra2 = (int *)&one, }, {} }; ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [patch 2/3] x86/vdso: Plug race between mapping and ELF header setup 2017-04-10 15:14 [patch 0/3] x86/vdso: Fix enable inconsistencies and cleanup Thomas Gleixner 2017-04-10 15:14 ` [patch 1/3] x86/vdso: Ensure vdso32_enabled gets set to valid values only Thomas Gleixner @ 2017-04-10 15:14 ` Thomas Gleixner 2017-04-10 15:56 ` Andy Lutomirski 2017-04-10 16:34 ` [tip:x86/urgent] " tip-bot for Thomas Gleixner 2017-04-10 15:14 ` [patch 3/3] x86/vdso: Sanitize vdso*_enabled handling Thomas Gleixner 2 siblings, 2 replies; 13+ messages in thread From: Thomas Gleixner @ 2017-04-10 15:14 UTC (permalink / raw) To: LKML; +Cc: x86, Peter Zijlstra, Andy Lutomirski, Mathias Krause, stable [-- Attachment #1: x86-vdso--Plug-race-between-mapping-and-ELF-header-setup.patch --] [-- Type: text/plain, Size: 1131 bytes --] The vsyscall32 sysctl can racy against a concurrent fork when it switches from disabled to enabled: arch_setup_additional_pages() if (vdso32_enabled) --> No mapping sysctl.vsysscall32() --> vdso32_enabled = true create_elf_tables() ARCH_DLINFO_IA32 if (vdso32_enabled) { --> Add VDSO entry with NULL pointer Make ARCH_DLINFO_IA32 check whether the VDSO mapping has been set up for the newly forked process or not. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: Andy Lutomirski <luto@amacapital.net> Cc: Mathias Krause <minipli@googlemail.com> Cc: stable@vger.kernel.org --- arch/x86/include/asm/elf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/arch/x86/include/asm/elf.h +++ b/arch/x86/include/asm/elf.h @@ -287,7 +287,7 @@ struct task_struct; #define ARCH_DLINFO_IA32 \ do { \ - if (vdso32_enabled) { \ + if (VDSO_CURRENT_BASE) { \ NEW_AUX_ENT(AT_SYSINFO, VDSO_ENTRY); \ NEW_AUX_ENT(AT_SYSINFO_EHDR, VDSO_CURRENT_BASE); \ } \ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch 2/3] x86/vdso: Plug race between mapping and ELF header setup 2017-04-10 15:14 ` [patch 2/3] x86/vdso: Plug race between mapping and ELF header setup Thomas Gleixner @ 2017-04-10 15:56 ` Andy Lutomirski 2017-04-10 16:34 ` [tip:x86/urgent] " tip-bot for Thomas Gleixner 1 sibling, 0 replies; 13+ messages in thread From: Andy Lutomirski @ 2017-04-10 15:56 UTC (permalink / raw) To: Thomas Gleixner; +Cc: LKML, X86 ML, Peter Zijlstra, Mathias Krause, stable On Mon, Apr 10, 2017 at 8:14 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > The vsyscall32 sysctl can racy against a concurrent fork when it switches > from disabled to enabled: > > arch_setup_additional_pages() > if (vdso32_enabled) > --> No mapping > sysctl.vsysscall32() > --> vdso32_enabled = true > create_elf_tables() > ARCH_DLINFO_IA32 > if (vdso32_enabled) { > --> Add VDSO entry with NULL pointer > > Make ARCH_DLINFO_IA32 check whether the VDSO mapping has been set up for > the newly forked process or not. Acked-by: Andy Lutomirski <luto@kernel.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [tip:x86/urgent] x86/vdso: Plug race between mapping and ELF header setup 2017-04-10 15:14 ` [patch 2/3] x86/vdso: Plug race between mapping and ELF header setup Thomas Gleixner 2017-04-10 15:56 ` Andy Lutomirski @ 2017-04-10 16:34 ` tip-bot for Thomas Gleixner 1 sibling, 0 replies; 13+ messages in thread From: tip-bot for Thomas Gleixner @ 2017-04-10 16:34 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, peterz, minipli, hpa, luto, tglx, mingo Commit-ID: 6fdc6dd90272ce7e75d744f71535cfbd8d77da81 Gitweb: http://git.kernel.org/tip/6fdc6dd90272ce7e75d744f71535cfbd8d77da81 Author: Thomas Gleixner <tglx@linutronix.de> AuthorDate: Mon, 10 Apr 2017 17:14:28 +0200 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Mon, 10 Apr 2017 18:31:41 +0200 x86/vdso: Plug race between mapping and ELF header setup The vsyscall32 sysctl can racy against a concurrent fork when it switches from disabled to enabled: arch_setup_additional_pages() if (vdso32_enabled) --> No mapping sysctl.vsysscall32() --> vdso32_enabled = true create_elf_tables() ARCH_DLINFO_IA32 if (vdso32_enabled) { --> Add VDSO entry with NULL pointer Make ARCH_DLINFO_IA32 check whether the VDSO mapping has been set up for the newly forked process or not. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Acked-by: Andy Lutomirski <luto@amacapital.net> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Mathias Krause <minipli@googlemail.com> Cc: stable@vger.kernel.org Link: http://lkml.kernel.org/r/20170410151723.602367196@linutronix.de Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/include/asm/elf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h index 9d49c18..3762536 100644 --- a/arch/x86/include/asm/elf.h +++ b/arch/x86/include/asm/elf.h @@ -287,7 +287,7 @@ struct task_struct; #define ARCH_DLINFO_IA32 \ do { \ - if (vdso32_enabled) { \ + if (VDSO_CURRENT_BASE) { \ NEW_AUX_ENT(AT_SYSINFO, VDSO_ENTRY); \ NEW_AUX_ENT(AT_SYSINFO_EHDR, VDSO_CURRENT_BASE); \ } \ ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [patch 3/3] x86/vdso: Sanitize vdso*_enabled handling 2017-04-10 15:14 [patch 0/3] x86/vdso: Fix enable inconsistencies and cleanup Thomas Gleixner 2017-04-10 15:14 ` [patch 1/3] x86/vdso: Ensure vdso32_enabled gets set to valid values only Thomas Gleixner 2017-04-10 15:14 ` [patch 2/3] x86/vdso: Plug race between mapping and ELF header setup Thomas Gleixner @ 2017-04-10 15:14 ` Thomas Gleixner 2017-04-10 15:55 ` Andy Lutomirski 2 siblings, 1 reply; 13+ messages in thread From: Thomas Gleixner @ 2017-04-10 15:14 UTC (permalink / raw) To: LKML; +Cc: x86, Peter Zijlstra, Andy Lutomirski, Mathias Krause [-- Attachment #1: x86-vdso--Sanitize-vdso*_enabled-handling.patch --] [-- Type: text/plain, Size: 4587 bytes --] vdso*_enabled is conceptionally a boolean. Though there are leftovers from the original implementation which required an int. That's just confusing and error prone. Convert evrything to boolean and correct stale comments. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: Andy Lutomirski <luto@amacapital.net> Cc: Mathias Krause <minipli@googlemail.com> --- arch/x86/entry/vdso/vdso32-setup.c | 40 +++++++++++++++++++++---------------- arch/x86/entry/vdso/vma.c | 12 +++++++---- arch/x86/include/asm/elf.h | 4 +-- arch/x86/um/vdso/vma.c | 4 +-- 4 files changed, 35 insertions(+), 25 deletions(-) --- a/arch/x86/entry/vdso/vdso32-setup.c +++ b/arch/x86/entry/vdso/vdso32-setup.c @@ -24,27 +24,19 @@ * Should the kernel map a VDSO page into processes and pass its * address down to glibc upon exec()? */ -unsigned int __read_mostly vdso32_enabled = VDSO_DEFAULT; +bool __read_mostly vdso32_enabled = VDSO_DEFAULT; static int __init vdso32_setup(char *s) { - vdso32_enabled = simple_strtoul(s, NULL, 0); - - if (vdso32_enabled > 1) { - pr_warn("vdso32 values other than 0 and 1 are no longer allowed; vdso disabled\n"); - vdso32_enabled = 0; - } + unsigned int ena = simple_strtoul(s, NULL, 0); + if (ena > 1) + pr_warn("vdso32=%u out of range [0-1], disabling vdso32\n", ena); + vdso32_enabled = ena == 1; return 1; } - -/* - * For consistency, the argument vdso32=[012] affects the 32-bit vDSO - * behavior on both 64-bit and 32-bit kernels. - * On 32-bit kernels, vdso=[012] means the same thing. - */ +/* The vdso32 option is valid on 64bit and 32bit kernels */ __setup("vdso32=", vdso32_setup); - #ifdef CONFIG_X86_32 __setup_param("vdso=", vdso_setup, vdso32_setup, 0); #endif @@ -66,14 +58,27 @@ subsys_initcall(sysenter_setup); static const int zero; static const int one = 1; +static int vdso32_sysctl; + +static int vdso_update_handler(struct ctl_table *table, int write, + void __user *buffer, size_t *lenp, + loff_t *ppos) +{ + int ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); + + if (ret || !write) + return ret; + vdso32_enabled = !!vdso32_sysctl; + return 0; +} static struct ctl_table abi_table2[] = { { .procname = "vsyscall32", - .data = &vdso32_enabled, - .maxlen = sizeof(int), + .data = &vdso32_sysctl, + .maxlen = sizeof(bool), .mode = 0644, - .proc_handler = proc_dointvec_minmax, + .proc_handler = vdso_update_handler, .extra1 = (int *)&zero, .extra2 = (int *)&one, }, @@ -91,6 +96,7 @@ static struct ctl_table abi_root_table2[ static __init int ia32_binfmt_init(void) { + vdso32_sysctl = vdso32_enabled; register_sysctl_table(abi_root_table2); return 0; } --- a/arch/x86/entry/vdso/vma.c +++ b/arch/x86/entry/vdso/vma.c @@ -24,7 +24,7 @@ #include <asm/cpufeature.h> #if defined(CONFIG_X86_64) -unsigned int __read_mostly vdso64_enabled = 1; +bool __read_mostly vdso64_enabled = true; #endif void __init init_vdso_image(const struct vdso_image *image) @@ -279,7 +279,7 @@ int map_vdso_once(const struct vdso_imag #if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION) static int load_vdso32(void) { - if (vdso32_enabled != 1) /* Other values all mean "disabled" */ + if (!vdso32_enabled) return 0; return map_vdso(&vdso_image_32, 0); @@ -323,8 +323,12 @@ int arch_setup_additional_pages(struct l #ifdef CONFIG_X86_64 static __init int vdso_setup(char *s) { - vdso64_enabled = simple_strtoul(s, NULL, 0); - return 0; + unsigned int ena = simple_strtoul(s, NULL, 0); + + if (ena > 1) + pr_warn("vdso=%u out of range [0-1], disabling vdso\n", ena); + vdso64_enabled = ena == 1; + return 1; } __setup("vdso=", vdso_setup); #endif --- a/arch/x86/include/asm/elf.h +++ b/arch/x86/include/asm/elf.h @@ -76,10 +76,10 @@ typedef struct user_fxsr_struct elf_fpxr #include <asm/vdso.h> #ifdef CONFIG_X86_64 -extern unsigned int vdso64_enabled; +extern bool vdso64_enabled; #endif #if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION) -extern unsigned int vdso32_enabled; +extern bool vdso32_enabled; #endif /* --- a/arch/x86/um/vdso/vma.c +++ b/arch/x86/um/vdso/vma.c @@ -13,7 +13,7 @@ #include <asm/elf.h> #include <linux/init.h> -static unsigned int __read_mostly vdso_enabled = 1; +static bool __read_mostly vdso_enabled = true; unsigned long um_vdso_addr; extern unsigned long task_size; @@ -47,7 +47,7 @@ static int __init init_vdso(void) oom: printk(KERN_ERR "Cannot allocate vdso\n"); - vdso_enabled = 0; + vdso_enabled = false; return -ENOMEM; } ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch 3/3] x86/vdso: Sanitize vdso*_enabled handling 2017-04-10 15:14 ` [patch 3/3] x86/vdso: Sanitize vdso*_enabled handling Thomas Gleixner @ 2017-04-10 15:55 ` Andy Lutomirski 2017-04-10 16:25 ` Thomas Gleixner 0 siblings, 1 reply; 13+ messages in thread From: Andy Lutomirski @ 2017-04-10 15:55 UTC (permalink / raw) To: Thomas Gleixner; +Cc: LKML, X86 ML, Peter Zijlstra, Mathias Krause On Mon, Apr 10, 2017 at 8:14 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > +static int vdso32_sysctl; > + > +static int vdso_update_handler(struct ctl_table *table, int write, > + void __user *buffer, size_t *lenp, > + loff_t *ppos) > +{ > + int ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); Using a global for the temporary is gross and mildly racy. I would just open-code the conversion. Or, even better, add proc_dobool(). I'm not convinced that the current patch is a cleanup. --Andy ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch 3/3] x86/vdso: Sanitize vdso*_enabled handling 2017-04-10 15:55 ` Andy Lutomirski @ 2017-04-10 16:25 ` Thomas Gleixner 0 siblings, 0 replies; 13+ messages in thread From: Thomas Gleixner @ 2017-04-10 16:25 UTC (permalink / raw) To: Andy Lutomirski; +Cc: LKML, X86 ML, Peter Zijlstra, Mathias Krause On Mon, 10 Apr 2017, Andy Lutomirski wrote: > On Mon, Apr 10, 2017 at 8:14 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > > +static int vdso32_sysctl; > > + > > +static int vdso_update_handler(struct ctl_table *table, int write, > > + void __user *buffer, size_t *lenp, > > + loff_t *ppos) > > +{ > > + int ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); > > Using a global for the temporary is gross and mildly racy. I would > just open-code the conversion. Or, even better, add proc_dobool(). Dammit, thought about it and then got lazy. :( Thanks, tglx ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only @ 2017-04-05 20:36 Mathias Krause 2017-04-06 15:59 ` Andy Lutomirski 2017-04-10 13:13 ` Thomas Gleixner 0 siblings, 2 replies; 13+ messages in thread From: Mathias Krause @ 2017-04-05 20:36 UTC (permalink / raw) To: x86 Cc: Mathias Krause, linux-kernel, Andy Lutomirski, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Roland McGrath If either via kernel command line 'vdso32=' or via 'sysctl abi.vsyscall32' vdso32_enabled gets set to a value below 0 or above 1, load_vdso32() won't map the vDSO but ARCH_DLINFO_IA32 would still pass an AT_SYSINFO_EHDR auxiliary vector, however with a NULL pointer. That'll make any program trying to make use of it fail with a segmentation fault. At least musl makes use of it if the kernel provides it. Ensure vdso32_enabled gets set to valid values only to fix this corner case. Fixes: b0b49f2673f0 ("x86, vdso: Remove compat vdso support") Cc: Andy Lutomirski <luto@amacapital.net> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Roland McGrath <roland@redhat.com> Signed-off-by: Mathias Krause <minipli@googlemail.com> --- arch/x86/entry/vdso/vdso32-setup.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/arch/x86/entry/vdso/vdso32-setup.c b/arch/x86/entry/vdso/vdso32-setup.c index 7853b53959cd..ca312c174d6f 100644 --- a/arch/x86/entry/vdso/vdso32-setup.c +++ b/arch/x86/entry/vdso/vdso32-setup.c @@ -30,8 +30,10 @@ static int __init vdso32_setup(char *s) { vdso32_enabled = simple_strtoul(s, NULL, 0); - if (vdso32_enabled > 1) + if (vdso32_enabled > 1) { pr_warn("vdso32 values other than 0 and 1 are no longer allowed; vdso disabled\n"); + vdso32_enabled = 0; + } return 1; } @@ -62,13 +64,18 @@ int __init sysenter_setup(void) /* Register vsyscall32 into the ABI table */ #include <linux/sysctl.h> +static const int zero; +static const int one = 1; + static struct ctl_table abi_table2[] = { { .procname = "vsyscall32", .data = &vdso32_enabled, .maxlen = sizeof(int), .mode = 0644, - .proc_handler = proc_dointvec + .proc_handler = proc_dointvec_minmax, + .extra1 = (int *)&zero, + .extra2 = (int *)&one, }, {} }; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only 2017-04-05 20:36 [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only Mathias Krause @ 2017-04-06 15:59 ` Andy Lutomirski 2017-04-10 13:13 ` Thomas Gleixner 1 sibling, 0 replies; 13+ messages in thread From: Andy Lutomirski @ 2017-04-06 15:59 UTC (permalink / raw) To: Mathias Krause Cc: X86 ML, linux-kernel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Roland McGrath On Wed, Apr 5, 2017 at 1:36 PM, Mathias Krause <minipli@googlemail.com> wrote: > If either via kernel command line 'vdso32=' or via 'sysctl abi.vsyscall32' > vdso32_enabled gets set to a value below 0 or above 1, load_vdso32() won't > map the vDSO but ARCH_DLINFO_IA32 would still pass an AT_SYSINFO_EHDR > auxiliary vector, however with a NULL pointer. That'll make any program > trying to make use of it fail with a segmentation fault. At least musl > makes use of it if the kernel provides it. > > Ensure vdso32_enabled gets set to valid values only to fix this corner > case. Acked-by: Andy Lutomirski <luto@kernel.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only 2017-04-05 20:36 [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only Mathias Krause 2017-04-06 15:59 ` Andy Lutomirski @ 2017-04-10 13:13 ` Thomas Gleixner 2017-04-10 13:41 ` Thomas Gleixner 1 sibling, 1 reply; 13+ messages in thread From: Thomas Gleixner @ 2017-04-10 13:13 UTC (permalink / raw) To: Mathias Krause Cc: x86, linux-kernel, Andy Lutomirski, Ingo Molnar, H. Peter Anvin, Roland McGrath On Wed, 5 Apr 2017, Mathias Krause wrote: > @@ -62,13 +64,18 @@ int __init sysenter_setup(void) > /* Register vsyscall32 into the ABI table */ > #include <linux/sysctl.h> > > +static const int zero; > +static const int one = 1; > + > static struct ctl_table abi_table2[] = { > { > .procname = "vsyscall32", > .data = &vdso32_enabled, > .maxlen = sizeof(int), > .mode = 0644, > - .proc_handler = proc_dointvec > + .proc_handler = proc_dointvec_minmax, > + .extra1 = (int *)&zero, > + .extra2 = (int *)&one, This is still bustable. Let's start with: vdso32_enabled = false arch_setup_additional_pages() --> No mapping sysctl.vsysscall32() --> vdso32_enabled = true create_elf_tables() if (vdso32_enabled) { --> Add VDSO entry with NULL pointer The vdso map code needs to store a flag in current which can be checked in ARCH_DLINFO_IA32. Thanks, tglx ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only 2017-04-10 13:13 ` Thomas Gleixner @ 2017-04-10 13:41 ` Thomas Gleixner 0 siblings, 0 replies; 13+ messages in thread From: Thomas Gleixner @ 2017-04-10 13:41 UTC (permalink / raw) To: Mathias Krause Cc: x86, linux-kernel, Andy Lutomirski, Ingo Molnar, H. Peter Anvin, Roland McGrath On Mon, 10 Apr 2017, Thomas Gleixner wrote: > On Wed, 5 Apr 2017, Mathias Krause wrote: > > @@ -62,13 +64,18 @@ int __init sysenter_setup(void) > > /* Register vsyscall32 into the ABI table */ > > #include <linux/sysctl.h> > > > > +static const int zero; > > +static const int one = 1; > > + > > static struct ctl_table abi_table2[] = { > > { > > .procname = "vsyscall32", > > .data = &vdso32_enabled, > > .maxlen = sizeof(int), > > .mode = 0644, > > - .proc_handler = proc_dointvec > > + .proc_handler = proc_dointvec_minmax, > > + .extra1 = (int *)&zero, > > + .extra2 = (int *)&one, > > This is still bustable. Let's start with: vdso32_enabled = false > > arch_setup_additional_pages() > --> No mapping > > sysctl.vsysscall32() > --> vdso32_enabled = true > > create_elf_tables() > if (vdso32_enabled) { > --> Add VDSO entry with NULL pointer > > The vdso map code needs to store a flag in current which can be checked in > ARCH_DLINFO_IA32. It's ways simpler. Patch below. Thanks, tglx --- a/arch/x86/include/asm/elf.h +++ b/arch/x86/include/asm/elf.h @@ -287,7 +287,7 @@ struct task_struct; #define ARCH_DLINFO_IA32 \ do { \ - if (vdso32_enabled) { \ + if (VDSO_CURRENT_BASE) { \ NEW_AUX_ENT(AT_SYSINFO, VDSO_ENTRY); \ NEW_AUX_ENT(AT_SYSINFO_EHDR, VDSO_CURRENT_BASE); \ } \ ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-04-10 17:58 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-10 15:14 [patch 0/3] x86/vdso: Fix enable inconsistencies and cleanup Thomas Gleixner 2017-04-10 15:14 ` [patch 1/3] x86/vdso: Ensure vdso32_enabled gets set to valid values only Thomas Gleixner 2017-04-10 16:34 ` [tip:x86/urgent] " tip-bot for Mathias Krause 2017-04-10 15:14 ` [patch 2/3] x86/vdso: Plug race between mapping and ELF header setup Thomas Gleixner 2017-04-10 15:56 ` Andy Lutomirski 2017-04-10 16:34 ` [tip:x86/urgent] " tip-bot for Thomas Gleixner 2017-04-10 15:14 ` [patch 3/3] x86/vdso: Sanitize vdso*_enabled handling Thomas Gleixner 2017-04-10 15:55 ` Andy Lutomirski 2017-04-10 16:25 ` Thomas Gleixner -- strict thread matches above, loose matches on Subject: below -- 2017-04-05 20:36 [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only Mathias Krause 2017-04-06 15:59 ` Andy Lutomirski 2017-04-10 13:13 ` Thomas Gleixner 2017-04-10 13:41 ` Thomas Gleixner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.