Hi Sasha,
I would object including this to stable tree:
1. It's selftest fixup
2. I have never saw selftests failing because of it (but it's possible)
I saw this in CRIU (Checkpoint Restore In Userspace) project
triggering, so I've fixed the selftests, mostly for documentation
reasons - as people in userspace can reuse this code and think
it's good and will just work after copy-paste (which is not the case).
So, this patch doesn't look urgent to include it in -stable kernel.
Thanks,
Dmitry
2017-11-29 17:21 GMT+00:00 <alexander.levin@xxxxxxxxxxx>:
> From: Dmitry Safonov <dsafonov@xxxxxxxxxxxxx>
>
> [ Upstream commit 2a4d0c627f5374f365a873dea4e10ae0bb437680 ]
>
> Kernel erases R8..R11 registers prior returning to userspace
> from int80:
>
> https://lkml.org/lkml/2009/10/1/164
>
> GCC can reuse these registers and doesn't expect them to change
> during syscall invocation. I met this kind of bug in CRIU once
> GCC 6.1 and CLANG stored local variables in those registers
> and the kernel zerofied them during syscall:
>
> https://github.com/xemul/criu/commit/990d33f1a1cdd17bca6c2eb059ab3be2564f7fa2
>
> By that reason I suggest to add those registers to clobbers
> in selftests. Also, as noted by Andy - removed unneeded clobber
> for flags in INT $0x80 inline asm.
>
> Signed-off-by: Dmitry Safonov <dsafonov@xxxxxxxxxxxxx>
> Acked-by: Andy Lutomirski <luto@xxxxxxxxxx>
> Cc: 0x7f454c46@xxxxxxxxx
> Cc: Borislav Petkov <bp@xxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxx>
> Cc: Brian Gerst <brgerst@xxxxxxxxx>
> Cc: Denys Vlasenko <dvlasenk@xxxxxxxxxx>
> Cc: H. Peter Anvin <hpa@xxxxxxxxx>
> Cc: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Shuah Khan <shuah@xxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: linux-kselftest@xxxxxxxxxxxxxxx
> Link: http://lkml.kernel.org/r/20170213101336.20486-1-dsafonov@xxxxxxxxxxxxx
> Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
> Signed-off-by: Sasha Levin <alexander.levin@xxxxxxxxxxx>
> ---
> tools/testing/selftests/x86/fsgsbase.c | 2 +-
> tools/testing/selftests/x86/ldt_gdt.c | 16 +++++++++++-----
> tools/testing/selftests/x86/ptrace_syscall.c | 3 ++-
> tools/testing/selftests/x86/single_step_syscall.c | 5 ++++-
> 4 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/x86/fsgsbase.c b/tools/testing/selftests/x86/fsgsbase.c
> index 9b4610c6d3fb..f249e042b3b5 100644
> --- a/tools/testing/selftests/x86/fsgsbase.c
> +++ b/tools/testing/selftests/x86/fsgsbase.c
> @@ -245,7 +245,7 @@ void do_unexpected_base(void)
> long ret;
> asm volatile ("int $0x80"
> : "=a" (ret) : "a" (243), "b" (low_desc)
> - : "flags");
> + : "r8", "r9", "r10", "r11");
> memcpy(&desc, low_desc, sizeof(desc));
> munmap(low_desc, sizeof(desc));
>
> diff --git a/tools/testing/selftests/x86/ldt_gdt.c b/tools/testing/selftests/x86/ldt_gdt.c
> index e717fed80219..b9a22f18566a 100644
> --- a/tools/testing/selftests/x86/ldt_gdt.c
> +++ b/tools/testing/selftests/x86/ldt_gdt.c
> @@ -45,6 +45,12 @@
> #define AR_DB (1 << 22)
> #define AR_G (1 << 23)
>
> +#ifdef __x86_64__
> +# define INT80_CLOBBERS "r8", "r9", "r10", "r11"
> +#else
> +# define INT80_CLOBBERS
> +#endif
> +
> static int nerrs;
>
> /* Points to an array of 1024 ints, each holding its own index. */
> @@ -634,7 +640,7 @@ static int invoke_set_thread_area(void)
> asm volatile ("int $0x80"
> : "=a" (ret), "+m" (low_user_desc) :
> "a" (243), "b" (low_user_desc)
> - : "flags");
> + : INT80_CLOBBERS);
> return ret;
> }
>
> @@ -703,7 +709,7 @@ static void test_gdt_invalidation(void)
> "+a" (eax)
> : "m" (low_user_desc_clear),
> [arg1] "r" ((unsigned int)(unsigned long)low_user_desc_clear)
> - : "flags");
> + : INT80_CLOBBERS);
>
> if (sel != 0) {
> result = "FAIL";
> @@ -734,7 +740,7 @@ static void test_gdt_invalidation(void)
> "+a" (eax)
> : "m" (low_user_desc_clear),
> [arg1] "r" ((unsigned int)(unsigned long)low_user_desc_clear)
> - : "flags");
> + : INT80_CLOBBERS);
>
> if (sel != 0) {
> result = "FAIL";
> @@ -767,7 +773,7 @@ static void test_gdt_invalidation(void)
> "+a" (eax)
> : "m" (low_user_desc_clear),
> [arg1] "r" ((unsigned int)(unsigned long)low_user_desc_clear)
> - : "flags");
> + : INT80_CLOBBERS);
>
> #ifdef __x86_64__
> syscall(SYS_arch_prctl, ARCH_GET_FS, &new_base);
> @@ -820,7 +826,7 @@ static void test_gdt_invalidation(void)
> "+a" (eax)
> : "m" (low_user_desc_clear),
> [arg1] "r" ((unsigned int)(unsigned long)low_user_desc_clear)
> - : "flags");
> + : INT80_CLOBBERS);
>
> #ifdef __x86_64__
> syscall(SYS_arch_prctl, ARCH_GET_GS, &new_base);
> diff --git a/tools/testing/selftests/x86/ptrace_syscall.c b/tools/testing/selftests/x86/ptrace_syscall.c
> index b037ce9cf116..eaea92439708 100644
> --- a/tools/testing/selftests/x86/ptrace_syscall.c
> +++ b/tools/testing/selftests/x86/ptrace_syscall.c
> @@ -58,7 +58,8 @@ static void do_full_int80(struct syscall_args32 *args)
> asm volatile ("int $0x80"
> : "+a" (args->nr),
> "+b" (args->arg0), "+c" (args->arg1), "+d" (args->arg2),
> - "+S" (args->arg3), "+D" (args->arg4), "+r" (bp));
> + "+S" (args->arg3), "+D" (args->arg4), "+r" (bp)
> + : : "r8", "r9", "r10", "r11");
> args->arg5 = bp;
> #else
> sys32_helper(args, int80_and_ret);
> diff --git a/tools/testing/selftests/x86/single_step_syscall.c b/tools/testing/selftests/x86/single_step_syscall.c
> index 50c26358e8b7..a48da95c18fd 100644
> --- a/tools/testing/selftests/x86/single_step_syscall.c
> +++ b/tools/testing/selftests/x86/single_step_syscall.c
> @@ -56,9 +56,11 @@ static volatile sig_atomic_t sig_traps;
> #ifdef __x86_64__
> # define REG_IP REG_RIP
> # define WIDTH "q"
> +# define INT80_CLOBBERS "r8", "r9", "r10", "r11"
> #else
> # define REG_IP REG_EIP
> # define WIDTH "l"
> +# define INT80_CLOBBERS
> #endif
>
> static unsigned long get_eflags(void)
> @@ -140,7 +142,8 @@ int main()
>
> printf("[RUN]\tSet TF and check int80\n");
> set_eflags(get_eflags() | X86_EFLAGS_TF);
> - asm volatile ("int $0x80" : "=a" (tmp) : "a" (SYS_getpid));
> + asm volatile ("int $0x80" : "=a" (tmp) : "a" (SYS_getpid)
> + : INT80_CLOBBERS);
> check_result();
>
> /*
> --
> 2.11.0