[v2,5/8] selftests/x86/vsyscall: Verify that vsyscall=none blocks execution
diff mbox series

Message ID b413397c804265f8865f3e70b14b09485ea7c314.1561610354.git.luto@kernel.org
State New
Headers show
Series
  • [v2,1/8] x86/vsyscall: Remove the vsyscall=native documentation
Related show

Commit Message

Andy Lutomirski June 27, 2019, 4:45 a.m. UTC
If vsyscall=none accidentally still allowed vsyscalls, the test
wouldn't fail.  Fix it.

Cc: Kees Cook <keescook@chromium.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 tools/testing/selftests/x86/test_vsyscall.c | 76 ++++++++++++++-------
 1 file changed, 52 insertions(+), 24 deletions(-)

Comments

Kees Cook June 27, 2019, 5:29 p.m. UTC | #1
On Wed, Jun 26, 2019 at 09:45:06PM -0700, Andy Lutomirski wrote:
> If vsyscall=none accidentally still allowed vsyscalls, the test
> wouldn't fail.  Fix it.
> 
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  tools/testing/selftests/x86/test_vsyscall.c | 76 ++++++++++++++-------
>  1 file changed, 52 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/testing/selftests/x86/test_vsyscall.c b/tools/testing/selftests/x86/test_vsyscall.c
> index 4c9a8d76dba0..34a1d35995ef 100644
> --- a/tools/testing/selftests/x86/test_vsyscall.c
> +++ b/tools/testing/selftests/x86/test_vsyscall.c
> @@ -49,21 +49,21 @@ static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
>  }
>  
>  /* vsyscalls and vDSO */
> -bool should_read_vsyscall = false;
> +bool vsyscall_map_r = false, vsyscall_map_x = false;
>  
>  typedef long (*gtod_t)(struct timeval *tv, struct timezone *tz);
> -gtod_t vgtod = (gtod_t)VSYS(0xffffffffff600000);
> +const gtod_t vgtod = (gtod_t)VSYS(0xffffffffff600000);
>  gtod_t vdso_gtod;
>  
>  typedef int (*vgettime_t)(clockid_t, struct timespec *);
>  vgettime_t vdso_gettime;
>  
>  typedef long (*time_func_t)(time_t *t);
> -time_func_t vtime = (time_func_t)VSYS(0xffffffffff600400);
> +const time_func_t vtime = (time_func_t)VSYS(0xffffffffff600400);
>  time_func_t vdso_time;
>  
>  typedef long (*getcpu_t)(unsigned *, unsigned *, void *);
> -getcpu_t vgetcpu = (getcpu_t)VSYS(0xffffffffff600800);
> +const getcpu_t vgetcpu = (getcpu_t)VSYS(0xffffffffff600800);
>  getcpu_t vdso_getcpu;
>  
>  static void init_vdso(void)
> @@ -107,7 +107,7 @@ static int init_vsys(void)
>  	maps = fopen("/proc/self/maps", "r");
>  	if (!maps) {
>  		printf("[WARN]\tCould not open /proc/self/maps -- assuming vsyscall is r-x\n");
> -		should_read_vsyscall = true;
> +		vsyscall_map_r = true;
>  		return 0;
>  	}
>  
> @@ -133,12 +133,8 @@ static int init_vsys(void)
>  		}
>  
>  		printf("\tvsyscall permissions are %c-%c\n", r, x);
> -		should_read_vsyscall = (r == 'r');
> -		if (x != 'x') {
> -			vgtod = NULL;
> -			vtime = NULL;
> -			vgetcpu = NULL;
> -		}
> +		vsyscall_map_r = (r == 'r');
> +		vsyscall_map_x = (x == 'x');
>  
>  		found = true;
>  		break;
> @@ -148,10 +144,8 @@ static int init_vsys(void)
>  
>  	if (!found) {
>  		printf("\tno vsyscall map in /proc/self/maps\n");
> -		should_read_vsyscall = false;
> -		vgtod = NULL;
> -		vtime = NULL;
> -		vgetcpu = NULL;
> +		vsyscall_map_r = false;
> +		vsyscall_map_x = false;
>  	}
>  
>  	return nerrs;
> @@ -242,7 +236,7 @@ static int test_gtod(void)
>  		err(1, "syscall gettimeofday");
>  	if (vdso_gtod)
>  		ret_vdso = vdso_gtod(&tv_vdso, &tz_vdso);
> -	if (vgtod)
> +	if (vsyscall_map_x)
>  		ret_vsys = vgtod(&tv_vsys, &tz_vsys);
>  	if (sys_gtod(&tv_sys2, &tz_sys) != 0)
>  		err(1, "syscall gettimeofday");
> @@ -256,7 +250,7 @@ static int test_gtod(void)
>  		}
>  	}
>  
> -	if (vgtod) {
> +	if (vsyscall_map_x) {
>  		if (ret_vsys == 0) {
>  			nerrs += check_gtod(&tv_sys1, &tv_sys2, &tz_sys, "vsyscall", &tv_vsys, &tz_vsys);
>  		} else {
> @@ -277,7 +271,7 @@ static int test_time(void) {
>  	t_sys1 = sys_time(&t2_sys1);
>  	if (vdso_time)
>  		t_vdso = vdso_time(&t2_vdso);
> -	if (vtime)
> +	if (vsyscall_map_x)
>  		t_vsys = vtime(&t2_vsys);
>  	t_sys2 = sys_time(&t2_sys2);
>  	if (t_sys1 < 0 || t_sys1 != t2_sys1 || t_sys2 < 0 || t_sys2 != t2_sys2) {
> @@ -298,7 +292,7 @@ static int test_time(void) {
>  		}
>  	}
>  
> -	if (vtime) {
> +	if (vsyscall_map_x) {
>  		if (t_vsys < 0 || t_vsys != t2_vsys) {
>  			printf("[FAIL]\tvsyscall failed (ret:%ld output:%ld)\n", t_vsys, t2_vsys);
>  			nerrs++;
> @@ -334,7 +328,7 @@ static int test_getcpu(int cpu)
>  	ret_sys = sys_getcpu(&cpu_sys, &node_sys, 0);
>  	if (vdso_getcpu)
>  		ret_vdso = vdso_getcpu(&cpu_vdso, &node_vdso, 0);
> -	if (vgetcpu)
> +	if (vsyscall_map_x)
>  		ret_vsys = vgetcpu(&cpu_vsys, &node_vsys, 0);
>  
>  	if (ret_sys == 0) {
> @@ -373,7 +367,7 @@ static int test_getcpu(int cpu)
>  		}
>  	}
>  
> -	if (vgetcpu) {
> +	if (vsyscall_map_x) {
>  		if (ret_vsys) {
>  			printf("[FAIL]\tvsyscall getcpu() failed\n");
>  			nerrs++;
> @@ -414,10 +408,10 @@ static int test_vsys_r(void)
>  		can_read = false;
>  	}
>  
> -	if (can_read && !should_read_vsyscall) {
> +	if (can_read && !vsyscall_map_r) {
>  		printf("[FAIL]\tWe have read access, but we shouldn't\n");
>  		return 1;
> -	} else if (!can_read && should_read_vsyscall) {
> +	} else if (!can_read && vsyscall_map_r) {
>  		printf("[FAIL]\tWe don't have read access, but we should\n");
>  		return 1;
>  	} else if (can_read) {
> @@ -431,6 +425,39 @@ static int test_vsys_r(void)
>  	return 0;
>  }
>  
> +static int test_vsys_x(void)
> +{
> +#ifdef __x86_64__
> +	if (vsyscall_map_x) {
> +		/* We already tested this adequately. */
> +		return 0;
> +	}
> +
> +	printf("[RUN]\tMake sure that vsyscalls really page fault\n");
> +
> +	bool can_exec;
> +	if (sigsetjmp(jmpbuf, 1) == 0) {
> +		vgtod(NULL, NULL);
> +		can_exec = true;
> +	} else {
> +		can_exec = false;
> +	}
> +
> +	if (can_exec) {
> +		printf("[FAIL]\tExecuting the vsyscall did not page fault\n");
> +		return 1;
> +	} else if (segv_err & (1 << 4)) { /* INSTR */
> +		printf("[OK]\tExecuting the vsyscall page failed: #PF(0x%lx)\n",
> +		       segv_err);
> +	} else {
> +		printf("[FAILT]\tExecution failed with the wrong error: #PF(0x%lx)\n",
> +		       segv_err);
> +		return 1;
> +	}
> +#endif
> +
> +	return 0;
> +}
>  
>  #ifdef __x86_64__
>  #define X86_EFLAGS_TF (1UL << 8)
> @@ -462,7 +489,7 @@ static int test_emulation(void)
>  	time_t tmp;
>  	bool is_native;
>  
> -	if (!vtime)
> +	if (!vsyscall_map_x)
>  		return 0;
>  
>  	printf("[RUN]\tchecking that vsyscalls are emulated\n");
> @@ -504,6 +531,7 @@ int main(int argc, char **argv)
>  
>  	sethandler(SIGSEGV, sigsegv, 0);
>  	nerrs += test_vsys_r();
> +	nerrs += test_vsys_x();
>  
>  #ifdef __x86_64__
>  	nerrs += test_emulation();
> -- 
> 2.21.0
>

Patch
diff mbox series

diff --git a/tools/testing/selftests/x86/test_vsyscall.c b/tools/testing/selftests/x86/test_vsyscall.c
index 4c9a8d76dba0..34a1d35995ef 100644
--- a/tools/testing/selftests/x86/test_vsyscall.c
+++ b/tools/testing/selftests/x86/test_vsyscall.c
@@ -49,21 +49,21 @@  static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
 }
 
 /* vsyscalls and vDSO */
-bool should_read_vsyscall = false;
+bool vsyscall_map_r = false, vsyscall_map_x = false;
 
 typedef long (*gtod_t)(struct timeval *tv, struct timezone *tz);
-gtod_t vgtod = (gtod_t)VSYS(0xffffffffff600000);
+const gtod_t vgtod = (gtod_t)VSYS(0xffffffffff600000);
 gtod_t vdso_gtod;
 
 typedef int (*vgettime_t)(clockid_t, struct timespec *);
 vgettime_t vdso_gettime;
 
 typedef long (*time_func_t)(time_t *t);
-time_func_t vtime = (time_func_t)VSYS(0xffffffffff600400);
+const time_func_t vtime = (time_func_t)VSYS(0xffffffffff600400);
 time_func_t vdso_time;
 
 typedef long (*getcpu_t)(unsigned *, unsigned *, void *);
-getcpu_t vgetcpu = (getcpu_t)VSYS(0xffffffffff600800);
+const getcpu_t vgetcpu = (getcpu_t)VSYS(0xffffffffff600800);
 getcpu_t vdso_getcpu;
 
 static void init_vdso(void)
@@ -107,7 +107,7 @@  static int init_vsys(void)
 	maps = fopen("/proc/self/maps", "r");
 	if (!maps) {
 		printf("[WARN]\tCould not open /proc/self/maps -- assuming vsyscall is r-x\n");
-		should_read_vsyscall = true;
+		vsyscall_map_r = true;
 		return 0;
 	}
 
@@ -133,12 +133,8 @@  static int init_vsys(void)
 		}
 
 		printf("\tvsyscall permissions are %c-%c\n", r, x);
-		should_read_vsyscall = (r == 'r');
-		if (x != 'x') {
-			vgtod = NULL;
-			vtime = NULL;
-			vgetcpu = NULL;
-		}
+		vsyscall_map_r = (r == 'r');
+		vsyscall_map_x = (x == 'x');
 
 		found = true;
 		break;
@@ -148,10 +144,8 @@  static int init_vsys(void)
 
 	if (!found) {
 		printf("\tno vsyscall map in /proc/self/maps\n");
-		should_read_vsyscall = false;
-		vgtod = NULL;
-		vtime = NULL;
-		vgetcpu = NULL;
+		vsyscall_map_r = false;
+		vsyscall_map_x = false;
 	}
 
 	return nerrs;
@@ -242,7 +236,7 @@  static int test_gtod(void)
 		err(1, "syscall gettimeofday");
 	if (vdso_gtod)
 		ret_vdso = vdso_gtod(&tv_vdso, &tz_vdso);
-	if (vgtod)
+	if (vsyscall_map_x)
 		ret_vsys = vgtod(&tv_vsys, &tz_vsys);
 	if (sys_gtod(&tv_sys2, &tz_sys) != 0)
 		err(1, "syscall gettimeofday");
@@ -256,7 +250,7 @@  static int test_gtod(void)
 		}
 	}
 
-	if (vgtod) {
+	if (vsyscall_map_x) {
 		if (ret_vsys == 0) {
 			nerrs += check_gtod(&tv_sys1, &tv_sys2, &tz_sys, "vsyscall", &tv_vsys, &tz_vsys);
 		} else {
@@ -277,7 +271,7 @@  static int test_time(void) {
 	t_sys1 = sys_time(&t2_sys1);
 	if (vdso_time)
 		t_vdso = vdso_time(&t2_vdso);
-	if (vtime)
+	if (vsyscall_map_x)
 		t_vsys = vtime(&t2_vsys);
 	t_sys2 = sys_time(&t2_sys2);
 	if (t_sys1 < 0 || t_sys1 != t2_sys1 || t_sys2 < 0 || t_sys2 != t2_sys2) {
@@ -298,7 +292,7 @@  static int test_time(void) {
 		}
 	}
 
-	if (vtime) {
+	if (vsyscall_map_x) {
 		if (t_vsys < 0 || t_vsys != t2_vsys) {
 			printf("[FAIL]\tvsyscall failed (ret:%ld output:%ld)\n", t_vsys, t2_vsys);
 			nerrs++;
@@ -334,7 +328,7 @@  static int test_getcpu(int cpu)
 	ret_sys = sys_getcpu(&cpu_sys, &node_sys, 0);
 	if (vdso_getcpu)
 		ret_vdso = vdso_getcpu(&cpu_vdso, &node_vdso, 0);
-	if (vgetcpu)
+	if (vsyscall_map_x)
 		ret_vsys = vgetcpu(&cpu_vsys, &node_vsys, 0);
 
 	if (ret_sys == 0) {
@@ -373,7 +367,7 @@  static int test_getcpu(int cpu)
 		}
 	}
 
-	if (vgetcpu) {
+	if (vsyscall_map_x) {
 		if (ret_vsys) {
 			printf("[FAIL]\tvsyscall getcpu() failed\n");
 			nerrs++;
@@ -414,10 +408,10 @@  static int test_vsys_r(void)
 		can_read = false;
 	}
 
-	if (can_read && !should_read_vsyscall) {
+	if (can_read && !vsyscall_map_r) {
 		printf("[FAIL]\tWe have read access, but we shouldn't\n");
 		return 1;
-	} else if (!can_read && should_read_vsyscall) {
+	} else if (!can_read && vsyscall_map_r) {
 		printf("[FAIL]\tWe don't have read access, but we should\n");
 		return 1;
 	} else if (can_read) {
@@ -431,6 +425,39 @@  static int test_vsys_r(void)
 	return 0;
 }
 
+static int test_vsys_x(void)
+{
+#ifdef __x86_64__
+	if (vsyscall_map_x) {
+		/* We already tested this adequately. */
+		return 0;
+	}
+
+	printf("[RUN]\tMake sure that vsyscalls really page fault\n");
+
+	bool can_exec;
+	if (sigsetjmp(jmpbuf, 1) == 0) {
+		vgtod(NULL, NULL);
+		can_exec = true;
+	} else {
+		can_exec = false;
+	}
+
+	if (can_exec) {
+		printf("[FAIL]\tExecuting the vsyscall did not page fault\n");
+		return 1;
+	} else if (segv_err & (1 << 4)) { /* INSTR */
+		printf("[OK]\tExecuting the vsyscall page failed: #PF(0x%lx)\n",
+		       segv_err);
+	} else {
+		printf("[FAILT]\tExecution failed with the wrong error: #PF(0x%lx)\n",
+		       segv_err);
+		return 1;
+	}
+#endif
+
+	return 0;
+}
 
 #ifdef __x86_64__
 #define X86_EFLAGS_TF (1UL << 8)
@@ -462,7 +489,7 @@  static int test_emulation(void)
 	time_t tmp;
 	bool is_native;
 
-	if (!vtime)
+	if (!vsyscall_map_x)
 		return 0;
 
 	printf("[RUN]\tchecking that vsyscalls are emulated\n");
@@ -504,6 +531,7 @@  int main(int argc, char **argv)
 
 	sethandler(SIGSEGV, sigsegv, 0);
 	nerrs += test_vsys_r();
+	nerrs += test_vsys_x();
 
 #ifdef __x86_64__
 	nerrs += test_emulation();