* [PATCH 0/2] x86/64: espfix64 cleanups and testing
@ 2016-09-12 22:05 Andy Lutomirski
2016-09-12 22:05 ` [PATCH 1/2] selftests/x86/sigreturn: Use CX, not AX, as the scratch register Andy Lutomirski
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Andy Lutomirski @ 2016-09-12 22:05 UTC (permalink / raw)
To: x86; +Cc: Borislav Petkov, linux-kernel, Brian Gerst, Andy Lutomirski
This tidies up ESPFIX a bit and improves the test case slightly.
Andy Lutomirski (2):
selftests/x86/sigreturn: Use CX, not AX, as the scratch register
x86/entry/64: Clean up and document espfix64 stack setup
arch/x86/entry/entry_64.S | 64 +++++++++++++++++++++++++++------
tools/testing/selftests/x86/sigreturn.c | 16 ++++-----
2 files changed, 61 insertions(+), 19 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] selftests/x86/sigreturn: Use CX, not AX, as the scratch register 2016-09-12 22:05 [PATCH 0/2] x86/64: espfix64 cleanups and testing Andy Lutomirski @ 2016-09-12 22:05 ` Andy Lutomirski 2016-09-13 22:08 ` [tip:x86/asm] " tip-bot for Andy Lutomirski 2016-09-12 22:05 ` [PATCH 2/2] x86/entry/64: Clean up and document espfix64 stack setup Andy Lutomirski 2016-09-13 9:04 ` [PATCH 0/2] x86/64: espfix64 cleanups and testing Borislav Petkov 2 siblings, 1 reply; 6+ messages in thread From: Andy Lutomirski @ 2016-09-12 22:05 UTC (permalink / raw) To: x86; +Cc: Borislav Petkov, linux-kernel, Brian Gerst, Andy Lutomirski RAX is handled specially in ESPFIX64. Use CX as our scratch register so that, if something goes wrong with RAX handling, we'll notice. Signed-off-by: Andy Lutomirski <luto@kernel.org> --- tools/testing/selftests/x86/sigreturn.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/x86/sigreturn.c b/tools/testing/selftests/x86/sigreturn.c index 8a577e7070c6..246145b84a12 100644 --- a/tools/testing/selftests/x86/sigreturn.c +++ b/tools/testing/selftests/x86/sigreturn.c @@ -106,7 +106,7 @@ asm (".pushsection .text\n\t" ".type int3, @function\n\t" ".align 4096\n\t" "int3:\n\t" - "mov %ss,%eax\n\t" + "mov %ss,%ecx\n\t" "int3\n\t" ".size int3, . - int3\n\t" ".align 4096, 0xcc\n\t" @@ -306,7 +306,7 @@ static volatile sig_atomic_t sig_corrupt_final_ss; #ifdef __x86_64__ # define REG_IP REG_RIP # define REG_SP REG_RSP -# define REG_AX REG_RAX +# define REG_CX REG_RCX struct selectors { unsigned short cs, gs, fs, ss; @@ -326,7 +326,7 @@ static unsigned short *csptr(ucontext_t *ctx) #else # define REG_IP REG_EIP # define REG_SP REG_ESP -# define REG_AX REG_EAX +# define REG_CX REG_ECX static greg_t *ssptr(ucontext_t *ctx) { @@ -457,10 +457,10 @@ static void sigusr1(int sig, siginfo_t *info, void *ctx_void) ctx->uc_mcontext.gregs[REG_IP] = sig_cs == code16_sel ? 0 : (unsigned long)&int3; ctx->uc_mcontext.gregs[REG_SP] = (unsigned long)0x8badf00d5aadc0deULL; - ctx->uc_mcontext.gregs[REG_AX] = 0; + ctx->uc_mcontext.gregs[REG_CX] = 0; memcpy(&requested_regs, &ctx->uc_mcontext.gregs, sizeof(gregset_t)); - requested_regs[REG_AX] = *ssptr(ctx); /* The asm code does this. */ + requested_regs[REG_CX] = *ssptr(ctx); /* The asm code does this. */ return; } @@ -482,7 +482,7 @@ static void sigtrap(int sig, siginfo_t *info, void *ctx_void) unsigned short ss; asm ("mov %%ss,%0" : "=r" (ss)); - greg_t asm_ss = ctx->uc_mcontext.gregs[REG_AX]; + greg_t asm_ss = ctx->uc_mcontext.gregs[REG_CX]; if (asm_ss != sig_ss && sig == SIGTRAP) { /* Sanity check failure. */ printf("[FAIL]\tSIGTRAP: ss = %hx, frame ss = %hx, ax = %llx\n", @@ -654,8 +654,8 @@ static int test_valid_sigreturn(int cs_bits, bool use_16bit_ss, int force_ss) #endif /* Sanity check on the kernel */ - if (i == REG_AX && requested_regs[i] != resulting_regs[i]) { - printf("[FAIL]\tAX (saved SP) mismatch: requested 0x%llx; got 0x%llx\n", + if (i == REG_CX && requested_regs[i] != resulting_regs[i]) { + printf("[FAIL]\tCX (saved SP) mismatch: requested 0x%llx; got 0x%llx\n", (unsigned long long)requested_regs[i], (unsigned long long)resulting_regs[i]); nerrs++; -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip:x86/asm] selftests/x86/sigreturn: Use CX, not AX, as the scratch register 2016-09-12 22:05 ` [PATCH 1/2] selftests/x86/sigreturn: Use CX, not AX, as the scratch register Andy Lutomirski @ 2016-09-13 22:08 ` tip-bot for Andy Lutomirski 0 siblings, 0 replies; 6+ messages in thread From: tip-bot for Andy Lutomirski @ 2016-09-13 22:08 UTC (permalink / raw) To: linux-tip-commits Cc: shuahkh, torvalds, bp, mingo, jpoimboe, luto, bp, dvlasenk, tglx, hpa, peterz, brgerst, linux-kernel Commit-ID: 1ef0199a1a698d82ecd39d11d1daa3f4ab006c75 Gitweb: http://git.kernel.org/tip/1ef0199a1a698d82ecd39d11d1daa3f4ab006c75 Author: Andy Lutomirski <luto@kernel.org> AuthorDate: Mon, 12 Sep 2016 15:05:50 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 13 Sep 2016 20:34:15 +0200 selftests/x86/sigreturn: Use CX, not AX, as the scratch register RAX is handled specially in ESPFIX64. Use CX as our scratch register so that, if something goes wrong with RAX handling, we'll notice. Signed-off-by: Andy Lutomirski <luto@kernel.org> Reviewed-by: Borislav Petkov <bp@suse.de> Cc: Borislav Petkov <bp@alien8.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Shuah Khan <shuahkh@osg.samsung.com> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/9ceeb24ea56925586c330dc46306f757ddea9fb5.1473717910.git.luto@kernel.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- tools/testing/selftests/x86/sigreturn.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/x86/sigreturn.c b/tools/testing/selftests/x86/sigreturn.c index 8a577e7..246145b 100644 --- a/tools/testing/selftests/x86/sigreturn.c +++ b/tools/testing/selftests/x86/sigreturn.c @@ -106,7 +106,7 @@ asm (".pushsection .text\n\t" ".type int3, @function\n\t" ".align 4096\n\t" "int3:\n\t" - "mov %ss,%eax\n\t" + "mov %ss,%ecx\n\t" "int3\n\t" ".size int3, . - int3\n\t" ".align 4096, 0xcc\n\t" @@ -306,7 +306,7 @@ static volatile sig_atomic_t sig_corrupt_final_ss; #ifdef __x86_64__ # define REG_IP REG_RIP # define REG_SP REG_RSP -# define REG_AX REG_RAX +# define REG_CX REG_RCX struct selectors { unsigned short cs, gs, fs, ss; @@ -326,7 +326,7 @@ static unsigned short *csptr(ucontext_t *ctx) #else # define REG_IP REG_EIP # define REG_SP REG_ESP -# define REG_AX REG_EAX +# define REG_CX REG_ECX static greg_t *ssptr(ucontext_t *ctx) { @@ -457,10 +457,10 @@ static void sigusr1(int sig, siginfo_t *info, void *ctx_void) ctx->uc_mcontext.gregs[REG_IP] = sig_cs == code16_sel ? 0 : (unsigned long)&int3; ctx->uc_mcontext.gregs[REG_SP] = (unsigned long)0x8badf00d5aadc0deULL; - ctx->uc_mcontext.gregs[REG_AX] = 0; + ctx->uc_mcontext.gregs[REG_CX] = 0; memcpy(&requested_regs, &ctx->uc_mcontext.gregs, sizeof(gregset_t)); - requested_regs[REG_AX] = *ssptr(ctx); /* The asm code does this. */ + requested_regs[REG_CX] = *ssptr(ctx); /* The asm code does this. */ return; } @@ -482,7 +482,7 @@ static void sigtrap(int sig, siginfo_t *info, void *ctx_void) unsigned short ss; asm ("mov %%ss,%0" : "=r" (ss)); - greg_t asm_ss = ctx->uc_mcontext.gregs[REG_AX]; + greg_t asm_ss = ctx->uc_mcontext.gregs[REG_CX]; if (asm_ss != sig_ss && sig == SIGTRAP) { /* Sanity check failure. */ printf("[FAIL]\tSIGTRAP: ss = %hx, frame ss = %hx, ax = %llx\n", @@ -654,8 +654,8 @@ static int test_valid_sigreturn(int cs_bits, bool use_16bit_ss, int force_ss) #endif /* Sanity check on the kernel */ - if (i == REG_AX && requested_regs[i] != resulting_regs[i]) { - printf("[FAIL]\tAX (saved SP) mismatch: requested 0x%llx; got 0x%llx\n", + if (i == REG_CX && requested_regs[i] != resulting_regs[i]) { + printf("[FAIL]\tCX (saved SP) mismatch: requested 0x%llx; got 0x%llx\n", (unsigned long long)requested_regs[i], (unsigned long long)resulting_regs[i]); nerrs++; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] x86/entry/64: Clean up and document espfix64 stack setup 2016-09-12 22:05 [PATCH 0/2] x86/64: espfix64 cleanups and testing Andy Lutomirski 2016-09-12 22:05 ` [PATCH 1/2] selftests/x86/sigreturn: Use CX, not AX, as the scratch register Andy Lutomirski @ 2016-09-12 22:05 ` Andy Lutomirski 2016-09-13 22:08 ` [tip:x86/asm] " tip-bot for Andy Lutomirski 2016-09-13 9:04 ` [PATCH 0/2] x86/64: espfix64 cleanups and testing Borislav Petkov 2 siblings, 1 reply; 6+ messages in thread From: Andy Lutomirski @ 2016-09-12 22:05 UTC (permalink / raw) To: x86; +Cc: Borislav Petkov, linux-kernel, Brian Gerst, Andy Lutomirski The espfix64 setup code was a bit inscrutible and contained an unnecessary push of RAX. Remove that push, update all the stack offsets to match, and document the whole mess. Reported-By: Borislav Petkov <bp@alien8.de> Signed-off-by: Andy Lutomirski <luto@kernel.org> --- arch/x86/entry/entry_64.S | 64 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index 0cfc665d6716..2b46384b4a4f 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -589,27 +589,69 @@ native_irq_return_iret: #ifdef CONFIG_X86_ESPFIX64 native_irq_return_ldt: - pushq %rax - pushq %rdi + /* + * We are running with user GSBASE. All GPRs contain their user + * values. We have a percpu ESPFIX stack that is eight slots + * long (see ESPFIX_STACK_SIZE). espfix_waddr points to the bottom + * of the ESPFIX stack. + * + * We clobber RAX and RDI in this code. We stash RDI on the + * normal stack and RAX on the ESPFIX stack. + * + * The ESPFIX stack layout we set up looks like this: + * + * --- top of ESPFIX stack --- + * SS + * RSP + * RFLAGS + * CS + * RIP <-- RSP points here when we're done + * RAX <-- espfix_waddr points here + * --- bottom of ESPFIX stack --- + */ + + pushq %rdi /* Stash user RDI */ SWAPGS movq PER_CPU_VAR(espfix_waddr), %rdi - movq %rax, (0*8)(%rdi) /* RAX */ - movq (2*8)(%rsp), %rax /* RIP */ + movq %rax, (0*8)(%rdi) /* user RAX */ + movq (1*8)(%rsp), %rax /* user RIP */ movq %rax, (1*8)(%rdi) - movq (3*8)(%rsp), %rax /* CS */ + movq (2*8)(%rsp), %rax /* user CS */ movq %rax, (2*8)(%rdi) - movq (4*8)(%rsp), %rax /* RFLAGS */ + movq (3*8)(%rsp), %rax /* user RFLAGS */ movq %rax, (3*8)(%rdi) - movq (6*8)(%rsp), %rax /* SS */ + movq (5*8)(%rsp), %rax /* user SS */ movq %rax, (5*8)(%rdi) - movq (5*8)(%rsp), %rax /* RSP */ + movq (4*8)(%rsp), %rax /* user RSP */ movq %rax, (4*8)(%rdi) - andl $0xffff0000, %eax - popq %rdi + /* Now RAX == RSP. */ + + andl $0xffff0000, %eax /* RAX = (RSP & 0xffff0000) */ + popq %rdi /* Restore user RDI */ + + /* + * espfix_stack[31:16] == 0. The page tables are set up such that + * (espfix_stack | (X & 0xffff0000)) points to a read-only alias of + * espfix_waddr for any X. That is, there are 65536 RO aliases of + * the same page. Set up RSP so that RSP[31:16] contains the + * respective 16 bits of the /userspace/ RSP and RSP nonetheless + * still points to an RO alias of the ESPFIX stack. + */ orq PER_CPU_VAR(espfix_stack), %rax SWAPGS movq %rax, %rsp - popq %rax + + /* + * At this point, we cannot write to the stack any more, but we can + * still read. + */ + popq %rax /* Restore user RAX */ + + /* + * RSP now points to an ordinary IRET frame, except that the page + * is read-only and RSP[31:16] are preloaded with the userspace + * values. We can now IRET back to userspace. + */ jmp native_irq_return_iret #endif END(common_interrupt) -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip:x86/asm] x86/entry/64: Clean up and document espfix64 stack setup 2016-09-12 22:05 ` [PATCH 2/2] x86/entry/64: Clean up and document espfix64 stack setup Andy Lutomirski @ 2016-09-13 22:08 ` tip-bot for Andy Lutomirski 0 siblings, 0 replies; 6+ messages in thread From: tip-bot for Andy Lutomirski @ 2016-09-13 22:08 UTC (permalink / raw) To: linux-tip-commits Cc: torvalds, linux-kernel, brgerst, luto, bp, jpoimboe, mingo, peterz, dvlasenk, hpa, bp, tglx Commit-ID: 85063fac1f72419eec4349621fe829b07f9acb1e Gitweb: http://git.kernel.org/tip/85063fac1f72419eec4349621fe829b07f9acb1e Author: Andy Lutomirski <luto@kernel.org> AuthorDate: Mon, 12 Sep 2016 15:05:51 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 13 Sep 2016 20:34:16 +0200 x86/entry/64: Clean up and document espfix64 stack setup The espfix64 setup code was a bit inscrutible and contained an unnecessary push of RAX. Remove that push, update all the stack offsets to match, and document the whole mess. Reported-By: Borislav Petkov <bp@alien8.de> Signed-off-by: Andy Lutomirski <luto@kernel.org> Reviewed-by: Borislav Petkov <bp@suse.de> Cc: Borislav Petkov <bp@alien8.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/e5459eb10cf1175c8b36b840bc425f210d045f35.1473717910.git.luto@kernel.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/entry/entry_64.S | 64 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index c0373d6..e7fba58 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -586,27 +586,69 @@ native_irq_return_iret: #ifdef CONFIG_X86_ESPFIX64 native_irq_return_ldt: - pushq %rax - pushq %rdi + /* + * We are running with user GSBASE. All GPRs contain their user + * values. We have a percpu ESPFIX stack that is eight slots + * long (see ESPFIX_STACK_SIZE). espfix_waddr points to the bottom + * of the ESPFIX stack. + * + * We clobber RAX and RDI in this code. We stash RDI on the + * normal stack and RAX on the ESPFIX stack. + * + * The ESPFIX stack layout we set up looks like this: + * + * --- top of ESPFIX stack --- + * SS + * RSP + * RFLAGS + * CS + * RIP <-- RSP points here when we're done + * RAX <-- espfix_waddr points here + * --- bottom of ESPFIX stack --- + */ + + pushq %rdi /* Stash user RDI */ SWAPGS movq PER_CPU_VAR(espfix_waddr), %rdi - movq %rax, (0*8)(%rdi) /* RAX */ - movq (2*8)(%rsp), %rax /* RIP */ + movq %rax, (0*8)(%rdi) /* user RAX */ + movq (1*8)(%rsp), %rax /* user RIP */ movq %rax, (1*8)(%rdi) - movq (3*8)(%rsp), %rax /* CS */ + movq (2*8)(%rsp), %rax /* user CS */ movq %rax, (2*8)(%rdi) - movq (4*8)(%rsp), %rax /* RFLAGS */ + movq (3*8)(%rsp), %rax /* user RFLAGS */ movq %rax, (3*8)(%rdi) - movq (6*8)(%rsp), %rax /* SS */ + movq (5*8)(%rsp), %rax /* user SS */ movq %rax, (5*8)(%rdi) - movq (5*8)(%rsp), %rax /* RSP */ + movq (4*8)(%rsp), %rax /* user RSP */ movq %rax, (4*8)(%rdi) - andl $0xffff0000, %eax - popq %rdi + /* Now RAX == RSP. */ + + andl $0xffff0000, %eax /* RAX = (RSP & 0xffff0000) */ + popq %rdi /* Restore user RDI */ + + /* + * espfix_stack[31:16] == 0. The page tables are set up such that + * (espfix_stack | (X & 0xffff0000)) points to a read-only alias of + * espfix_waddr for any X. That is, there are 65536 RO aliases of + * the same page. Set up RSP so that RSP[31:16] contains the + * respective 16 bits of the /userspace/ RSP and RSP nonetheless + * still points to an RO alias of the ESPFIX stack. + */ orq PER_CPU_VAR(espfix_stack), %rax SWAPGS movq %rax, %rsp - popq %rax + + /* + * At this point, we cannot write to the stack any more, but we can + * still read. + */ + popq %rax /* Restore user RAX */ + + /* + * RSP now points to an ordinary IRET frame, except that the page + * is read-only and RSP[31:16] are preloaded with the userspace + * values. We can now IRET back to userspace. + */ jmp native_irq_return_iret #endif END(common_interrupt) ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] x86/64: espfix64 cleanups and testing 2016-09-12 22:05 [PATCH 0/2] x86/64: espfix64 cleanups and testing Andy Lutomirski 2016-09-12 22:05 ` [PATCH 1/2] selftests/x86/sigreturn: Use CX, not AX, as the scratch register Andy Lutomirski 2016-09-12 22:05 ` [PATCH 2/2] x86/entry/64: Clean up and document espfix64 stack setup Andy Lutomirski @ 2016-09-13 9:04 ` Borislav Petkov 2 siblings, 0 replies; 6+ messages in thread From: Borislav Petkov @ 2016-09-13 9:04 UTC (permalink / raw) To: Andy Lutomirski; +Cc: x86, linux-kernel, Brian Gerst On Mon, Sep 12, 2016 at 03:05:49PM -0700, Andy Lutomirski wrote: > This tidies up ESPFIX a bit and improves the test case slightly. > > Andy Lutomirski (2): > selftests/x86/sigreturn: Use CX, not AX, as the scratch register > x86/entry/64: Clean up and document espfix64 stack setup > > arch/x86/entry/entry_64.S | 64 +++++++++++++++++++++++++++------ > tools/testing/selftests/x86/sigreturn.c | 16 ++++----- > 2 files changed, 61 insertions(+), 19 deletions(-) Both: Reviewed-by: Borislav Petkov <bp@suse.de> Thanks. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-13 22:09 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-09-12 22:05 [PATCH 0/2] x86/64: espfix64 cleanups and testing Andy Lutomirski 2016-09-12 22:05 ` [PATCH 1/2] selftests/x86/sigreturn: Use CX, not AX, as the scratch register Andy Lutomirski 2016-09-13 22:08 ` [tip:x86/asm] " tip-bot for Andy Lutomirski 2016-09-12 22:05 ` [PATCH 2/2] x86/entry/64: Clean up and document espfix64 stack setup Andy Lutomirski 2016-09-13 22:08 ` [tip:x86/asm] " tip-bot for Andy Lutomirski 2016-09-13 9:04 ` [PATCH 0/2] x86/64: espfix64 cleanups and testing Borislav Petkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox