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 <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Roland McGrath <[email protected]>
Signed-off-by: Mathias Krause <[email protected]>
---
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
On Wed, Apr 5, 2017 at 1:36 PM, Mathias Krause <[email protected]> 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 <[email protected]>
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
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); \
} \