Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1617356 > unrolled thread

[PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only

Back to article view | Back to linux.kernel

Contents

  [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only Mathias Krause <minipli@googlemail.com> - 2017-04-05 22:40 +0200
    Re: [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only Andy Lutomirski <luto@kernel.org> - 2017-04-06 18:10 +0200
    Re: [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values  only Thomas Gleixner <tglx@linutronix.de> - 2017-04-10 15:20 +0200
      Re: [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values  only Thomas Gleixner <tglx@linutronix.de> - 2017-04-10 15:50 +0200

#1617356 — [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only

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

[toc] | [next] | [standalone]

#1618138

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>

[toc] | [prev] | [next] | [standalone]

#1619910 — Re: [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only

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

[toc] | [prev] | [next] | [standalone]

#1619948 — Re: [PATCH] x86/vdso: ensure vdso32_enabled gets set to valid values only

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);	\
 	}								\

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web