Re: [PATCH 02/12] selftests/x86: Add check_initial_reg_state

[Andy Lutomirski <luto@amacapital.net>]

On Tue, Dec 8, 2015 at 1:54 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Dec 07, 2015 at 01:51:27PM -0800, Andy Lutomirski wrote:
>> This checks that ELF binaries are started with an appropriately
>> blank register state.
>>
>> (There's currently a nasty special case in the entry asm to arrange
>>  for this.  I'm planning on removing the special case, and this will
>>  help make sure I don't break it.)
>>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>  tools/testing/selftests/x86/Makefile               |   8 +-
>>  .../selftests/x86/check_initial_reg_state.c        | 108 +++++++++++++++++++++
>>  2 files changed, 115 insertions(+), 1 deletion(-)
>>  create mode 100644 tools/testing/selftests/x86/check_initial_reg_state.c
>>
>> diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
>> index a460fe7c5365..b82409421fa6 100644
>> --- a/tools/testing/selftests/x86/Makefile
>> +++ b/tools/testing/selftests/x86/Makefile
>> @@ -4,7 +4,7 @@ include ../lib.mk
>>
>>  .PHONY: all all_32 all_64 warn_32bit_failure clean
>>
>> -TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs ldt_gdt syscall_nt ptrace_syscall
>> +TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs ldt_gdt syscall_nt ptrace_syscall check_initial_reg_state
>>  TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault sigreturn test_syscall_vdso unwind_vdso
>>
>>  TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY)
>> @@ -63,3 +63,9 @@ endif
>>  sysret_ss_attrs_64: thunks.S
>>  ptrace_syscall_32: raw_syscall_helper_32.S
>>  test_syscall_vdso_32: thunks_32.S
>> +
>> +# check_initial_reg_state is special: it needs a custom entry, and it
>> +# needs to be static so that its interpreter doesn't destroy its initial
>> +# state.
>> +check_initial_reg_state_32: CFLAGS += -Wl,-ereal_start -static
>> +check_initial_reg_state_64: CFLAGS += -Wl,-ereal_start -static
>> diff --git a/tools/testing/selftests/x86/check_initial_reg_state.c b/tools/testing/selftests/x86/check_initial_reg_state.c
>> new file mode 100644
>> index 000000000000..0cb565f7786d
>> --- /dev/null
>> +++ b/tools/testing/selftests/x86/check_initial_reg_state.c
>> @@ -0,0 +1,108 @@
>> +/*
>> + * check_initial_reg_state.c - check that execve sets the correct state
>> + * Copyright (c) 2014-2015 Andrew Lutomirski
>> + *
>> + * 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.
>> + */
>> +
>> +#define _GNU_SOURCE
>> +
>> +#include <stdio.h>
>> +
>> +unsigned long ax, bx, cx, dx, si, di, bp, sp, flags;
>> +unsigned long r8, r9, r10, r11, r12, r13, r14, r15;
>> +
>> +asm (".pushsection .text\n\t"
>
> WARNING: please, no spaces at the start of a line
> #82: FILE: tools/testing/selftests/x86/check_initial_reg_state.c:23:
> +     ".type real_start, @function\n\t"$
>
> Something trampled over the preceding tabs in that whole asm().

That was intentional -- everything lines up with the top-level "asm(".
checkpatch is confused because it understands "\t " at the front of a
line but not just a space.

That being said, I could easily be convinced to switch to tabs there.

--Andy