Re: [PATCH] x86/asm/entry/32, selftests: Add test_syscall_vdso test
On Wed, 10 Jun 2015 22:15:58 +0200
Denys Vlasenko wrote:
> On 06/10/2015 10:00 PM, Kees Cook wrote:
> >> + printf("[SKIP]\tAT_SYSINFO not supplied, can't test\n");
> >> + exit(0); /* this is not a test failure */
> >
> > Why is that not a test failure? It would mean it didn't actually test
> > anything, which seems like a failure to me.
>
> Are you objecting to comment wording, or to exiting with 0?
>
> I exit with 0 because no bug was detected.
I believe there's a way to exit with "not supported". That's what the
ftrace tests do.
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/asm/entry/32, selftests: Add test_syscall_vdso test
On Wed, Jun 10, 2015 at 1:15 PM, Denys Vlasenko wrote:
> On 06/10/2015 10:00 PM, Kees Cook wrote:
>>> + printf("[SKIP]\tAT_SYSINFO not supplied, can't test\n");
>>> + exit(0); /* this is not a test failure */
>>
>> Why is that not a test failure? It would mean it didn't actually test
>> anything, which seems like a failure to me.
>
> Are you objecting to comment wording, or to exiting with 0?
>
> I exit with 0 because no bug was detected.
It seemed like a test failure to me: you're failing open ("couldn't
configure test, I guess everything is okay") instead of failing closed
("couldn't configure test, something is terribly wrong").
If you can't locate how to make a syscall, then the test should fail,
IMO, since it was not possible to perform the test, so you don't know
if flags are being correctly handled across syscalls.
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/asm/entry/32, selftests: Add test_syscall_vdso test
On Wed, Jun 10, 2015 at 9:59 AM, Denys Vlasenko wrote:
> The test is fairly simplistic: it checks that all registers
> are preserved across 32-bit syscall via VDSO.
>
> Run-tested.
>
> Signed-off-by: Denys Vlasenko
> CC: Linus Torvalds
> CC: Steven Rostedt
> CC: Ingo Molnar
> CC: Borislav Petkov
> CC: "H. Peter Anvin"
> CC: Andy Lutomirski
> CC: Oleg Nesterov
> CC: Frederic Weisbecker
> CC: Alexei Starovoitov
> CC: Will Drewry
> CC: Kees Cook
> CC: x...@kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
> tools/testing/selftests/x86/Makefile| 2 +-
> tools/testing/selftests/x86/test_syscall_vdso.c | 198
>
> 2 files changed, 199 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/x86/test_syscall_vdso.c
I love tests! Very nice. :)
Reviewed-by: Kees Cook
> diff --git a/tools/testing/selftests/x86/Makefile
> b/tools/testing/selftests/x86/Makefile
> index caa60d5..84effa6 100644
> --- a/tools/testing/selftests/x86/Makefile
> +++ b/tools/testing/selftests/x86/Makefile
> @@ -5,7 +5,7 @@ include ../lib.mk
> .PHONY: all all_32 all_64 warn_32bit_failure clean
>
> TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs
> -TARGETS_C_32BIT_ONLY := entry_from_vm86
> +TARGETS_C_32BIT_ONLY := entry_from_vm86 test_syscall_vdso
>
> TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
> BINARIES_32 := $(TARGETS_C_32BIT_ALL:%=%_32)
> diff --git a/tools/testing/selftests/x86/test_syscall_vdso.c
> b/tools/testing/selftests/x86/test_syscall_vdso.c
> new file mode 100644
> index 000..78e6efb
> --- /dev/null
> +++ b/tools/testing/selftests/x86/test_syscall_vdso.c
> @@ -0,0 +1,198 @@
> +/*
> + * 32-bit syscall ABI conformance test.
> + *
> + * Copyright (c) 2015 Denys Vlasenko
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + */
> +#undef _GNU_SOURCE
> +#define _GNU_SOURCE 1
> +#undef __USE_GNU
> +#define __USE_GNU 1
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#if !defined(__i386__)
> +int main(int argc, char **argv, char **envp)
> +{
> + printf("[SKIP] Not a 32-bit x86\n");
> + return 0;
> +}
> +#else
> +
> +//TODO: check r8..15 for info leaks.
> +//TODO: run this test twice, second time under ptrace.
Minor nit-pick: I would recommend using /* */-style comments, not
//-style. And checkpatch.pl would yell too. :)
> +
> +long syscall_addr;
> +long get_syscall(char **envp)
> +{
> + Elf32_auxv_t *auxv;
> + while (*envp++ != NULL)
> + continue;
> + for (auxv = (void *)envp; auxv->a_type != AT_NULL; auxv++)
> + if( auxv->a_type == AT_SYSINFO)
> + return auxv->a_un.a_val;
> + printf("[SKIP]\tAT_SYSINFO not supplied, can't test\n");
> + exit(0); /* this is not a test failure */
Why is that not a test failure? It would mean it didn't actually test
anything, which seems like a failure to me.
> +}
> +
> +int nfds;
> +fd_set rfds;
> +fd_set wfds;
> +fd_set efds;
> +struct timespec timeout;
> +sigset_t sigmask;
> +struct {
> + sigset_t *sp;
> + int sz;
> +} sigmask_desc;
> +
> +void prep_args()
> +{
> + nfds = 42;
> + FD_ZERO();
> + FD_ZERO();
> + FD_ZERO();
> + FD_SET(0, );
> + FD_SET(1, );
> + FD_SET(2, );
> + timeout.tv_sec = 0;
> + timeout.tv_nsec = 123;
> + sigemptyset();
> + sigaddset(, SIGINT);
> + sigaddset(, SIGUSR2);
> + sigaddset(, SIGRTMAX);
> + sigmask_desc.sp =
> + sigmask_desc.sz = 8; /* bytes */
> +}
> +
> +static void print_flags(const char *name, unsigned long r)
> +{
> + static const char *bitarray[] = {
> + "\n" ,"c\n" ,/* Carry Flag */
> + "0 " ,"1 " ,/* Bit 1 - always on */
> + "" ,"p " ,/* Parity Flag */
> + "0 " ,"3? " ,
> + "" ,"a " ,/* Auxiliary carry Flag */
> + "0 " ,"5? " ,
> + "" ,"z " ,/* Zero Flag */
> + "" ,"s " ,/* Sign Flag */
> + "" ,"t " ,/* Trap Flag */
> + "" ,"i " ,/* Interrupt Flag */
> + "" ,"d " ,/* Direction Flag */
> + "" ,"o " ,/* Overflow Flag */
> + "0 " ,"1 " ,/* I/O Privilege Level (2 bits) */
> + "0" ,"1" ,/* I/O Privilege Level (2 bits) */
> + "" ,"n " ,/* Nested Task */
> + "0 " ,"15? ",
> + "" ,"r " ,/* Resume Flag */
> + "" ,"v " ,/* Virtual Mode */
> + "" ,"ac " ,/* Alignment Check/Access Control */
> + "" ,"vif ",/*
Re: [PATCH] x86/asm/entry/32, selftests: Add test_syscall_vdso test
On Wed, Jun 10, 2015 at 9:59 AM, Denys Vlasenko dvlas...@redhat.com wrote:
The test is fairly simplistic: it checks that all registers
are preserved across 32-bit syscall via VDSO.
Run-tested.
Signed-off-by: Denys Vlasenko dvlas...@redhat.com
CC: Linus Torvalds torva...@linux-foundation.org
CC: Steven Rostedt rost...@goodmis.org
CC: Ingo Molnar mi...@kernel.org
CC: Borislav Petkov b...@alien8.de
CC: H. Peter Anvin h...@zytor.com
CC: Andy Lutomirski l...@amacapital.net
CC: Oleg Nesterov o...@redhat.com
CC: Frederic Weisbecker fweis...@gmail.com
CC: Alexei Starovoitov a...@plumgrid.com
CC: Will Drewry w...@chromium.org
CC: Kees Cook keesc...@chromium.org
CC: x...@kernel.org
CC: linux-kernel@vger.kernel.org
---
tools/testing/selftests/x86/Makefile| 2 +-
tools/testing/selftests/x86/test_syscall_vdso.c | 198
2 files changed, 199 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/x86/test_syscall_vdso.c
I love tests! Very nice. :)
Reviewed-by: Kees Cook keesc...@chromium.org
diff --git a/tools/testing/selftests/x86/Makefile
b/tools/testing/selftests/x86/Makefile
index caa60d5..84effa6 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -5,7 +5,7 @@ include ../lib.mk
.PHONY: all all_32 all_64 warn_32bit_failure clean
TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs
-TARGETS_C_32BIT_ONLY := entry_from_vm86
+TARGETS_C_32BIT_ONLY := entry_from_vm86 test_syscall_vdso
TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
BINARIES_32 := $(TARGETS_C_32BIT_ALL:%=%_32)
diff --git a/tools/testing/selftests/x86/test_syscall_vdso.c
b/tools/testing/selftests/x86/test_syscall_vdso.c
new file mode 100644
index 000..78e6efb
--- /dev/null
+++ b/tools/testing/selftests/x86/test_syscall_vdso.c
@@ -0,0 +1,198 @@
+/*
+ * 32-bit syscall ABI conformance test.
+ *
+ * Copyright (c) 2015 Denys Vlasenko
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+#undef _GNU_SOURCE
+#define _GNU_SOURCE 1
+#undef __USE_GNU
+#define __USE_GNU 1
+#include unistd.h
+#include stdlib.h
+#include stdio.h
+#include signal.h
+#include sys/select.h
+#include sys/time.h
+#include sys/types.h
+#include elf.h
+
+#if !defined(__i386__)
+int main(int argc, char **argv, char **envp)
+{
+ printf([SKIP] Not a 32-bit x86\n);
+ return 0;
+}
+#else
+
+//TODO: check r8..15 for info leaks.
+//TODO: run this test twice, second time under ptrace.
Minor nit-pick: I would recommend using /* */-style comments, not
//-style. And checkpatch.pl would yell too. :)
+
+long syscall_addr;
+long get_syscall(char **envp)
+{
+ Elf32_auxv_t *auxv;
+ while (*envp++ != NULL)
+ continue;
+ for (auxv = (void *)envp; auxv-a_type != AT_NULL; auxv++)
+ if( auxv-a_type == AT_SYSINFO)
+ return auxv-a_un.a_val;
+ printf([SKIP]\tAT_SYSINFO not supplied, can't test\n);
+ exit(0); /* this is not a test failure */
Why is that not a test failure? It would mean it didn't actually test
anything, which seems like a failure to me.
+}
+
+int nfds;
+fd_set rfds;
+fd_set wfds;
+fd_set efds;
+struct timespec timeout;
+sigset_t sigmask;
+struct {
+ sigset_t *sp;
+ int sz;
+} sigmask_desc;
+
+void prep_args()
+{
+ nfds = 42;
+ FD_ZERO(rfds);
+ FD_ZERO(wfds);
+ FD_ZERO(efds);
+ FD_SET(0, rfds);
+ FD_SET(1, wfds);
+ FD_SET(2, efds);
+ timeout.tv_sec = 0;
+ timeout.tv_nsec = 123;
+ sigemptyset(sigmask);
+ sigaddset(sigmask, SIGINT);
+ sigaddset(sigmask, SIGUSR2);
+ sigaddset(sigmask, SIGRTMAX);
+ sigmask_desc.sp = sigmask;
+ sigmask_desc.sz = 8; /* bytes */
+}
+
+static void print_flags(const char *name, unsigned long r)
+{
+ static const char *bitarray[] = {
+ \n ,c\n ,/* Carry Flag */
+ 0 ,1 ,/* Bit 1 - always on */
+ ,p ,/* Parity Flag */
+ 0 ,3? ,
+ ,a ,/* Auxiliary carry Flag */
+ 0 ,5? ,
+ ,z ,/* Zero Flag */
+ ,s ,/* Sign Flag */
+ ,t ,/* Trap Flag */
+ ,i ,/* Interrupt Flag */
+ ,d ,/* Direction Flag */
+ ,o ,/* Overflow Flag */
+ 0 ,1 ,/* I/O Privilege Level (2 bits) */
+ 0 ,1 ,/* I/O Privilege Level (2 bits) */
+ ,n ,/* Nested Task */
+ 0 ,15? ,
+
Re: [PATCH] x86/asm/entry/32, selftests: Add test_syscall_vdso test
On Wed, Jun 10, 2015 at 1:15 PM, Denys Vlasenko dvlas...@redhat.com wrote: On 06/10/2015 10:00 PM, Kees Cook wrote: + printf([SKIP]\tAT_SYSINFO not supplied, can't test\n); + exit(0); /* this is not a test failure */ Why is that not a test failure? It would mean it didn't actually test anything, which seems like a failure to me. Are you objecting to comment wording, or to exiting with 0? I exit with 0 because no bug was detected. It seemed like a test failure to me: you're failing open (couldn't configure test, I guess everything is okay) instead of failing closed (couldn't configure test, something is terribly wrong). If you can't locate how to make a syscall, then the test should fail, IMO, since it was not possible to perform the test, so you don't know if flags are being correctly handled across syscalls. -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/asm/entry/32, selftests: Add test_syscall_vdso test
On Wed, 10 Jun 2015 22:15:58 +0200 Denys Vlasenko dvlas...@redhat.com wrote: On 06/10/2015 10:00 PM, Kees Cook wrote: + printf([SKIP]\tAT_SYSINFO not supplied, can't test\n); + exit(0); /* this is not a test failure */ Why is that not a test failure? It would mean it didn't actually test anything, which seems like a failure to me. Are you objecting to comment wording, or to exiting with 0? I exit with 0 because no bug was detected. I believe there's a way to exit with not supported. That's what the ftrace tests do. -- Steve -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
