Re: [PATCH] x86/asm/entry/32, selftests: Add test_syscall_vdso test

[Kees Cook]

On Wed, Jun 10, 2015 at 9:59 AM, Denys Vlasenko <dvlasenk@xxxxxxxxxx> 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 <dvlasenk@xxxxxxxxxx>
> CC: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> CC: Steven Rostedt <rostedt@xxxxxxxxxxx>
> CC: Ingo Molnar <mingo@xxxxxxxxxx>
> CC: Borislav Petkov <bp@xxxxxxxxx>
> CC: "H. Peter Anvin" <hpa@xxxxxxxxx>
> CC: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
> CC: Oleg Nesterov <oleg@xxxxxxxxxx>
> CC: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> CC: Alexei Starovoitov <ast@xxxxxxxxxxxx>
> CC: Will Drewry <wad@xxxxxxxxxxxx>
> CC: Kees Cook <keescook@xxxxxxxxxxxx>
> CC: x86@xxxxxxxxxx
> CC: linux-kernel@xxxxxxxxxxxxxxx
> ---
>  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 <keescook@xxxxxxxxxxxx>

> 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 0000000..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? ",
> +       ""   ,"r "  ,/* Resume Flag */
> +       ""   ,"v "  ,/* Virtual Mode */
> +       ""   ,"ac " ,/* Alignment Check/Access Control */
> +       ""   ,"vif ",/* Virtual Interrupt Flag */
> +       ""   ,"vip ",/* Virtual Interrupt Pending */
> +       ""   ,"id " ,/* CPUID detection */
> +       NULL
> +       };
> +       const char **bitstr;
> +       int bit;
> +
> +       printf("%s=%016lx ", name, r);
> +       bitstr = bitarray + 42;
> +       bit = 21;
> +       if ((r >> 22) != 0)
> +               printf("(extra bits are set) ");
> +       do {
> +               if (bitstr[(r >> bit) & 1][0])
> +                       printf(bitstr[(r >> bit) & 1]);
> +               bitstr -= 2;
> +               bit--;
> +       } while (bit >= 0);
> +}
> +
> +int main(int argc, char **argv, char **envp)
> +{
> +       long flags, bad_arg;
> +
> +       // This only works for non-static builds:
> +       //syscall_addr = dlsym(dlopen("linux-gate.so.1", RTLD_NOW), "__kernel_vsyscall");
> +       syscall_addr = get_syscall(envp);
> +
> +       prep_args();
> +
> +       printf("[RUN] executing 6-argument 32-bit syscall\n");
> +
> +       asm("\n"
> +       // Try 6-arg syscall: pselect. It should return quickly
> +       "       push    %%ebp\n"
> +       "       mov     $308, %%eax\n"     // PSELECT
> +       "       mov     nfds, %%ebx\n"     // ebx  arg1
> +       "       mov     $rfds, %%ecx\n"    // ecx  arg2
> +       "       mov     $wfds, %%edx\n"    // edx  arg3
> +       "       mov     $efds, %%esi\n"    // esi  arg4
> +       "       mov     $timeout, %%edi\n" // edi  arg5
> +       "       mov     $sigmask_desc, %%ebp\n" // %ebp arg6
> +       "       push    $0x200ed7\n"      // set almost all flags
> +       "       popf\n"         // except TF, IOPL, NT, RF, VM, AC, VIF, VIP
> +       "       call    *syscall_addr\n"
> +       // Check that registers are not clobbered
> +       "       pushf\n"
> +       "       pop     %%eax\n"
> +       "       cld\n"
> +       "       cmp     nfds, %%ebx\n"     // ebx  arg1
> +       "       mov     $1, %%ebx\n"
> +       "       jne     1f\n"
> +       "       cmp     $rfds, %%ecx\n"    // ecx  arg2
> +       "       mov     $2, %%ebx\n"
> +       "       jne     1f\n"
> +       "       cmp     $wfds, %%edx\n"    // edx  arg3
> +       "       mov     $3, %%ebx\n"
> +       "       jne     1f\n"
> +       "       cmp     $efds, %%esi\n"    // esi  arg4
> +       "       mov     $4, %%ebx\n"
> +       "       jne     1f\n"
> +       "       cmp     $timeout, %%edi\n" // edi  arg5
> +       "       mov     $5, %%ebx\n"
> +       "       jne     1f\n"
> +       "       cmpl    $sigmask_desc, %%ebp\n" // %ebp arg6
> +       "       mov     $6, %%ebx\n"
> +       "       jne     1f\n"
> +       "       mov     $0, %%ebx\n"
> +       "1:\n"
> +       "       pop     %%ebp\n"
> +       : "=a" (flags), "=b" (bad_arg)
> +       :
> +       : "cx", "dx", "si", "di"
> +       );
> +
> +       /*
> +        * On paravirt kernels, flags are not preserved across syscalls.
> +        * Thus, we do not consider it a bug if some are changed.
> +        * We just show ones which do.
> +        */
> +       if ((0x200ed7 ^ flags) != 0) {
> +               print_flags("[WARN] flags before", 0x200ed7);
> +               print_flags("[WARN] flags  after", flags);
> +               print_flags("[WARN] changed", (0x200ed7 ^ flags));
> +       }
> +
> +       if (bad_arg) {
> +               printf("[FAIL] arg#%ld clobbered\n", bad_arg);
> +               return 1;
> +       }
> +       printf("[Ok] arguments are preserved across syscalls\n");
> +       return 0;
> +}
> +#endif
> --
> 1.8.1.4
>

Thanks!

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/