* [PATCH 1/6] x86/entry: Assert that syscalls are on the right stack
2020-06-26 17:21 [PATCH 0/6] x86/entry: Fixes Andy Lutomirski
@ 2020-06-26 17:21 ` Andy Lutomirski
2020-07-01 8:04 ` [tip: x86/urgent] " tip-bot2 for Andy Lutomirski
2020-07-06 14:34 ` [PATCH] x86/entry: Mark check_user_regs() noinstr Peter Zijlstra
2020-06-26 17:21 ` [PATCH 2/6] x86/entry: Move SYSENTER's regs->sp and regs->flags fixups into C Andy Lutomirski
` (4 subsequent siblings)
5 siblings, 2 replies; 23+ messages in thread
From: Andy Lutomirski @ 2020-06-26 17:21 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Andrew Cooper, Juergen Gross, Andy Lutomirski
Now that the entry stack is a full page, it's too easy to regress
the system call entry code and end up on the wrong stack without
noticing. Assert that all system calls (SYSCALL64, SYSCALL32,
SYSENTER, and INT80) are on the right stack and have pt_regs in the
right place.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/entry/common.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index bd3f14175193..ed8ccc820995 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -45,6 +45,15 @@
#define CREATE_TRACE_POINTS
#include <trace/events/syscalls.h>
+/* Check that the stack and regs on entry from user mode are sane. */
+static void check_user_regs(struct pt_regs *regs)
+{
+ if (IS_ENABLED(CONFIG_DEBUG_ENTRY)) {
+ WARN_ON_ONCE(!on_thread_stack());
+ WARN_ON_ONCE(regs != task_pt_regs(current));
+ }
+}
+
#ifdef CONFIG_CONTEXT_TRACKING
/**
* enter_from_user_mode - Establish state when coming from user mode
@@ -127,9 +136,6 @@ static long syscall_trace_enter(struct pt_regs *regs)
unsigned long ret = 0;
u32 work;
- if (IS_ENABLED(CONFIG_DEBUG_ENTRY))
- BUG_ON(regs != task_pt_regs(current));
-
work = READ_ONCE(ti->flags);
if (work & (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_EMU)) {
@@ -346,6 +352,8 @@ __visible noinstr void do_syscall_64(unsigned long nr, struct pt_regs *regs)
{
struct thread_info *ti;
+ check_user_regs(regs);
+
enter_from_user_mode();
instrumentation_begin();
@@ -409,6 +417,8 @@ static void do_syscall_32_irqs_on(struct pt_regs *regs)
/* Handles int $0x80 */
__visible noinstr void do_int80_syscall_32(struct pt_regs *regs)
{
+ check_user_regs(regs);
+
enter_from_user_mode();
instrumentation_begin();
@@ -460,6 +470,8 @@ __visible noinstr long do_fast_syscall_32(struct pt_regs *regs)
vdso_image_32.sym_int80_landing_pad;
bool success;
+ check_user_regs(regs);
+
/*
* SYSENTER loses EIP, and even SYSCALL32 needs us to skip forward
* so that 'regs->ip -= 2' lands back on an int $0x80 instruction.
--
2.25.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [tip: x86/urgent] x86/entry: Assert that syscalls are on the right stack
2020-06-26 17:21 ` [PATCH 1/6] x86/entry: Assert that syscalls are on the right stack Andy Lutomirski
@ 2020-07-01 8:04 ` tip-bot2 for Andy Lutomirski
2020-07-06 14:34 ` [PATCH] x86/entry: Mark check_user_regs() noinstr Peter Zijlstra
1 sibling, 0 replies; 23+ messages in thread
From: tip-bot2 for Andy Lutomirski @ 2020-07-01 8:04 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Andy Lutomirski, Thomas Gleixner, x86, LKML
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: c9c26150e61de441ab58b25c1f64afc049ee0fee
Gitweb: https://git.kernel.org/tip/c9c26150e61de441ab58b25c1f64afc049ee0fee
Author: Andy Lutomirski <luto@kernel.org>
AuthorDate: Fri, 26 Jun 2020 10:21:11 -07:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 01 Jul 2020 10:00:25 +02:00
x86/entry: Assert that syscalls are on the right stack
Now that the entry stack is a full page, it's too easy to regress the
system call entry code and end up on the wrong stack without noticing.
Assert that all system calls (SYSCALL64, SYSCALL32, SYSENTER, and INT80)
are on the right stack and have pt_regs in the right place.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/52059e42bb0ab8551153d012d68f7be18d72ff8e.1593191971.git.luto@kernel.org
---
arch/x86/entry/common.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index bd3f141..ed8ccc8 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -45,6 +45,15 @@
#define CREATE_TRACE_POINTS
#include <trace/events/syscalls.h>
+/* Check that the stack and regs on entry from user mode are sane. */
+static void check_user_regs(struct pt_regs *regs)
+{
+ if (IS_ENABLED(CONFIG_DEBUG_ENTRY)) {
+ WARN_ON_ONCE(!on_thread_stack());
+ WARN_ON_ONCE(regs != task_pt_regs(current));
+ }
+}
+
#ifdef CONFIG_CONTEXT_TRACKING
/**
* enter_from_user_mode - Establish state when coming from user mode
@@ -127,9 +136,6 @@ static long syscall_trace_enter(struct pt_regs *regs)
unsigned long ret = 0;
u32 work;
- if (IS_ENABLED(CONFIG_DEBUG_ENTRY))
- BUG_ON(regs != task_pt_regs(current));
-
work = READ_ONCE(ti->flags);
if (work & (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_EMU)) {
@@ -346,6 +352,8 @@ __visible noinstr void do_syscall_64(unsigned long nr, struct pt_regs *regs)
{
struct thread_info *ti;
+ check_user_regs(regs);
+
enter_from_user_mode();
instrumentation_begin();
@@ -409,6 +417,8 @@ static void do_syscall_32_irqs_on(struct pt_regs *regs)
/* Handles int $0x80 */
__visible noinstr void do_int80_syscall_32(struct pt_regs *regs)
{
+ check_user_regs(regs);
+
enter_from_user_mode();
instrumentation_begin();
@@ -460,6 +470,8 @@ __visible noinstr long do_fast_syscall_32(struct pt_regs *regs)
vdso_image_32.sym_int80_landing_pad;
bool success;
+ check_user_regs(regs);
+
/*
* SYSENTER loses EIP, and even SYSCALL32 needs us to skip forward
* so that 'regs->ip -= 2' lands back on an int $0x80 instruction.
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH] x86/entry: Mark check_user_regs() noinstr
2020-06-26 17:21 ` [PATCH 1/6] x86/entry: Assert that syscalls are on the right stack Andy Lutomirski
2020-07-01 8:04 ` [tip: x86/urgent] " tip-bot2 for Andy Lutomirski
@ 2020-07-06 14:34 ` Peter Zijlstra
1 sibling, 0 replies; 23+ messages in thread
From: Peter Zijlstra @ 2020-07-06 14:34 UTC (permalink / raw)
To: Andy Lutomirski; +Cc: x86, linux-kernel, Andrew Cooper, Juergen Gross
On Fri, Jun 26, 2020 at 10:21:11AM -0700, Andy Lutomirski wrote:
> +static void check_user_regs(struct pt_regs *regs)
---
Subject: x86/entry: Mark check_user_regs() noinstr
vmlinux.o: warning: objtool: do_syscall_64()+0xb: call to check_user_regs() leaves .noinstr.text section
vmlinux.o: warning: objtool: do_int80_syscall_32()+0x4: call to check_user_regs() leaves .noinstr.text section
vmlinux.o: warning: objtool: do_fast_syscall_32()+0x25: call to check_user_regs() leaves .noinstr.text section
vmlinux.o: warning: objtool: idtentry_enter_cond_rcu()+0x3d: call to check_user_regs() leaves .noinstr.text section
vmlinux.o: warning: objtool: idtentry_enter_user()+0x0: call to check_user_regs() leaves .noinstr.text section
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/entry/common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index e83b3f14897c..ea7b515e3bc2 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -46,7 +46,7 @@
#include <trace/events/syscalls.h>
/* Check that the stack and regs on entry from user mode are sane. */
-static void check_user_regs(struct pt_regs *regs)
+static noinstr void check_user_regs(struct pt_regs *regs)
{
if (IS_ENABLED(CONFIG_DEBUG_ENTRY)) {
/*
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/6] x86/entry: Move SYSENTER's regs->sp and regs->flags fixups into C
2020-06-26 17:21 [PATCH 0/6] x86/entry: Fixes Andy Lutomirski
2020-06-26 17:21 ` [PATCH 1/6] x86/entry: Assert that syscalls are on the right stack Andy Lutomirski
@ 2020-06-26 17:21 ` Andy Lutomirski
2020-07-01 8:04 ` [tip: x86/urgent] " tip-bot2 for Andy Lutomirski
2020-06-26 17:21 ` Andy Lutomirski
` (3 subsequent siblings)
5 siblings, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2020-06-26 17:21 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Andrew Cooper, Juergen Gross, Andy Lutomirski
The SYSENTER asm (32-bit and compat) contains fixups for regs->sp
and regs->flags. Move the fixups into C and fix some comments while
we're at it.
IMO this is a valid cleanup all by itself, and it also simplifies
the subsequent patch that will fix Xen PV SYSENTER.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/entry/common.c | 12 ++++++++++++
arch/x86/entry/entry_32.S | 5 ++---
arch/x86/entry/entry_64_compat.S | 11 +++++------
3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index ed8ccc820995..f392a8bcd1c3 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -522,6 +522,18 @@ __visible noinstr long do_fast_syscall_32(struct pt_regs *regs)
(regs->flags & (X86_EFLAGS_RF | X86_EFLAGS_TF | X86_EFLAGS_VM)) == 0;
#endif
}
+
+/* Returns 0 to return using IRET or 1 to return using SYSEXIT/SYSRETL. */
+__visible noinstr long do_SYSENTER_32(struct pt_regs *regs)
+{
+ /* SYSENTER loses RSP, but the vDSO saved it in RBP. */
+ regs->sp = regs->bp;
+
+ /* SYSENTER clobbers EFLAGS.IF. Assume it was set in usermode. */
+ regs->flags |= X86_EFLAGS_IF;
+
+ return do_fast_syscall_32(regs);
+}
#endif
SYSCALL_DEFINE0(ni_syscall)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 024d7d276cd4..2d0bd5d5f032 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -933,9 +933,8 @@ SYM_FUNC_START(entry_SYSENTER_32)
.Lsysenter_past_esp:
pushl $__USER_DS /* pt_regs->ss */
- pushl %ebp /* pt_regs->sp (stashed in bp) */
+ pushl $0 /* pt_regs->sp (placeholder) */
pushfl /* pt_regs->flags (except IF = 0) */
- orl $X86_EFLAGS_IF, (%esp) /* Fix IF */
pushl $__USER_CS /* pt_regs->cs */
pushl $0 /* pt_regs->ip = 0 (placeholder) */
pushl %eax /* pt_regs->orig_ax */
@@ -965,7 +964,7 @@ SYM_FUNC_START(entry_SYSENTER_32)
.Lsysenter_flags_fixed:
movl %esp, %eax
- call do_fast_syscall_32
+ call do_SYSENTER_32
/* XEN PV guests always use IRET path */
ALTERNATIVE "testl %eax, %eax; jz .Lsyscall_32_done", \
"jmp .Lsyscall_32_done", X86_FEATURE_XENPV
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 0f974ae01e62..7b9d8150f652 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -68,16 +68,15 @@ SYM_CODE_START(entry_SYSENTER_compat)
/* Construct struct pt_regs on stack */
pushq $__USER32_DS /* pt_regs->ss */
- pushq %rbp /* pt_regs->sp (stashed in bp) */
+ pushq $0 /* pt_regs->sp = 0 (placeholder) */
/*
* Push flags. This is nasty. First, interrupts are currently
- * off, but we need pt_regs->flags to have IF set. Second, even
- * if TF was set when SYSENTER started, it's clear by now. We fix
- * that later using TIF_SINGLESTEP.
+ * off, but we need pt_regs->flags to have IF set. Second, if TS
+ * was set in usermode, it's still set, and we're singlestepping
+ * through this code. do_SYSENTER_32() will fix up IF.
*/
pushfq /* pt_regs->flags (except IF = 0) */
- orl $X86_EFLAGS_IF, (%rsp) /* Fix saved flags */
pushq $__USER32_CS /* pt_regs->cs */
pushq $0 /* pt_regs->ip = 0 (placeholder) */
pushq %rax /* pt_regs->orig_ax */
@@ -135,7 +134,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
.Lsysenter_flags_fixed:
movq %rsp, %rdi
- call do_fast_syscall_32
+ call do_SYSENTER_32
/* XEN PV guests always use IRET path */
ALTERNATIVE "testl %eax, %eax; jz swapgs_restore_regs_and_return_to_usermode", \
"jmp swapgs_restore_regs_and_return_to_usermode", X86_FEATURE_XENPV
--
2.25.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [tip: x86/urgent] x86/entry: Move SYSENTER's regs->sp and regs->flags fixups into C
2020-06-26 17:21 ` [PATCH 2/6] x86/entry: Move SYSENTER's regs->sp and regs->flags fixups into C Andy Lutomirski
@ 2020-07-01 8:04 ` tip-bot2 for Andy Lutomirski
0 siblings, 0 replies; 23+ messages in thread
From: tip-bot2 for Andy Lutomirski @ 2020-07-01 8:04 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Andy Lutomirski, Thomas Gleixner, x86, LKML
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: d1721250f3ffed9afba3e1fb729947cec64c5a8a
Gitweb: https://git.kernel.org/tip/d1721250f3ffed9afba3e1fb729947cec64c5a8a
Author: Andy Lutomirski <luto@kernel.org>
AuthorDate: Fri, 26 Jun 2020 10:21:12 -07:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 01 Jul 2020 10:00:25 +02:00
x86/entry: Move SYSENTER's regs->sp and regs->flags fixups into C
The SYSENTER asm (32-bit and compat) contains fixups for regs->sp and
regs->flags. Move the fixups into C and fix some comments while at it.
This is a valid cleanup all by itself, and it also simplifies the
subsequent patch that will fix Xen PV SYSENTER.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/fe62bef67eda7fac75b8f3dbafccf571dc4ece6b.1593191971.git.luto@kernel.org
---
arch/x86/entry/common.c | 12 ++++++++++++
arch/x86/entry/entry_32.S | 5 ++---
arch/x86/entry/entry_64_compat.S | 11 +++++------
3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index ed8ccc8..f392a8b 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -522,6 +522,18 @@ __visible noinstr long do_fast_syscall_32(struct pt_regs *regs)
(regs->flags & (X86_EFLAGS_RF | X86_EFLAGS_TF | X86_EFLAGS_VM)) == 0;
#endif
}
+
+/* Returns 0 to return using IRET or 1 to return using SYSEXIT/SYSRETL. */
+__visible noinstr long do_SYSENTER_32(struct pt_regs *regs)
+{
+ /* SYSENTER loses RSP, but the vDSO saved it in RBP. */
+ regs->sp = regs->bp;
+
+ /* SYSENTER clobbers EFLAGS.IF. Assume it was set in usermode. */
+ regs->flags |= X86_EFLAGS_IF;
+
+ return do_fast_syscall_32(regs);
+}
#endif
SYSCALL_DEFINE0(ni_syscall)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 024d7d2..2d0bd5d 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -933,9 +933,8 @@ SYM_FUNC_START(entry_SYSENTER_32)
.Lsysenter_past_esp:
pushl $__USER_DS /* pt_regs->ss */
- pushl %ebp /* pt_regs->sp (stashed in bp) */
+ pushl $0 /* pt_regs->sp (placeholder) */
pushfl /* pt_regs->flags (except IF = 0) */
- orl $X86_EFLAGS_IF, (%esp) /* Fix IF */
pushl $__USER_CS /* pt_regs->cs */
pushl $0 /* pt_regs->ip = 0 (placeholder) */
pushl %eax /* pt_regs->orig_ax */
@@ -965,7 +964,7 @@ SYM_FUNC_START(entry_SYSENTER_32)
.Lsysenter_flags_fixed:
movl %esp, %eax
- call do_fast_syscall_32
+ call do_SYSENTER_32
/* XEN PV guests always use IRET path */
ALTERNATIVE "testl %eax, %eax; jz .Lsyscall_32_done", \
"jmp .Lsyscall_32_done", X86_FEATURE_XENPV
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 0f974ae..7b9d815 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -68,16 +68,15 @@ SYM_CODE_START(entry_SYSENTER_compat)
/* Construct struct pt_regs on stack */
pushq $__USER32_DS /* pt_regs->ss */
- pushq %rbp /* pt_regs->sp (stashed in bp) */
+ pushq $0 /* pt_regs->sp = 0 (placeholder) */
/*
* Push flags. This is nasty. First, interrupts are currently
- * off, but we need pt_regs->flags to have IF set. Second, even
- * if TF was set when SYSENTER started, it's clear by now. We fix
- * that later using TIF_SINGLESTEP.
+ * off, but we need pt_regs->flags to have IF set. Second, if TS
+ * was set in usermode, it's still set, and we're singlestepping
+ * through this code. do_SYSENTER_32() will fix up IF.
*/
pushfq /* pt_regs->flags (except IF = 0) */
- orl $X86_EFLAGS_IF, (%rsp) /* Fix saved flags */
pushq $__USER32_CS /* pt_regs->cs */
pushq $0 /* pt_regs->ip = 0 (placeholder) */
pushq %rax /* pt_regs->orig_ax */
@@ -135,7 +134,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
.Lsysenter_flags_fixed:
movq %rsp, %rdi
- call do_fast_syscall_32
+ call do_SYSENTER_32
/* XEN PV guests always use IRET path */
ALTERNATIVE "testl %eax, %eax; jz swapgs_restore_regs_and_return_to_usermode", \
"jmp swapgs_restore_regs_and_return_to_usermode", X86_FEATURE_XENPV
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
2020-06-26 17:21 [PATCH 0/6] x86/entry: Fixes Andy Lutomirski
@ 2020-06-26 17:21 ` Andy Lutomirski
2020-06-26 17:21 ` [PATCH 2/6] x86/entry: Move SYSENTER's regs->sp and regs->flags fixups into C Andy Lutomirski
` (4 subsequent siblings)
5 siblings, 0 replies; 23+ messages in thread
From: Andy Lutomirski @ 2020-06-26 17:21 UTC (permalink / raw)
To: x86
Cc: linux-kernel, Andrew Cooper, Juergen Gross, Andy Lutomirski,
Boris Ostrovsky, Stefano Stabellini, xen-devel
The SYSENTER frame setup was nonsense. It worked by accident
because the normal code into which the Xen asm jumped
(entry_SYSENTER_32/compat) threw away SP without touching the stack.
entry_SYSENTER_compat was recently modified such that it relied on
having a valid stack pointer, so now the Xen asm needs to invoke it
with a valid stack.
Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
metal prologue.
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org
Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/entry/entry_64_compat.S | 1 +
arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 7b9d8150f652..381a6de7de9c 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
pushfq /* pt_regs->flags (except IF = 0) */
pushq $__USER32_CS /* pt_regs->cs */
pushq $0 /* pt_regs->ip = 0 (placeholder) */
+SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
pushq %rax /* pt_regs->orig_ax */
pushq %rdi /* pt_regs->di */
pushq %rsi /* pt_regs->si */
diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
index 5d252aaeade8..e1e1c7eafa60 100644
--- a/arch/x86/xen/xen-asm_64.S
+++ b/arch/x86/xen/xen-asm_64.S
@@ -161,10 +161,22 @@ SYM_FUNC_END(xen_syscall32_target)
/* 32-bit compat sysenter target */
SYM_FUNC_START(xen_sysenter_target)
- mov 0*8(%rsp), %rcx
- mov 1*8(%rsp), %r11
- mov 5*8(%rsp), %rsp
- jmp entry_SYSENTER_compat
+ /*
+ * NB: Xen is polite and clears TF from EFLAGS for us. This means
+ * that we don't need to guard against single step exceptions here.
+ */
+ popq %rcx
+ popq %r11
+
+ /*
+ * Neither Xen nor the kernel really knows what the old SS and
+ * CS were. The kernel expects __USER32_DS and __USER32_CS, so
+ * report those values even though Xen will guess its own values.
+ */
+ movq $__USER32_DS, 4*8(%rsp)
+ movq $__USER32_CS, 1*8(%rsp)
+
+ jmp entry_SYSENTER_compat_after_hwframe
SYM_FUNC_END(xen_sysenter_target)
#else /* !CONFIG_IA32_EMULATION */
--
2.25.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
@ 2020-06-26 17:21 ` Andy Lutomirski
0 siblings, 0 replies; 23+ messages in thread
From: Andy Lutomirski @ 2020-06-26 17:21 UTC (permalink / raw)
To: x86
Cc: Juergen Gross, Stefano Stabellini, Andrew Cooper, linux-kernel,
Andy Lutomirski, xen-devel, Boris Ostrovsky
The SYSENTER frame setup was nonsense. It worked by accident
because the normal code into which the Xen asm jumped
(entry_SYSENTER_32/compat) threw away SP without touching the stack.
entry_SYSENTER_compat was recently modified such that it relied on
having a valid stack pointer, so now the Xen asm needs to invoke it
with a valid stack.
Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
metal prologue.
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org
Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/entry/entry_64_compat.S | 1 +
arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 7b9d8150f652..381a6de7de9c 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
pushfq /* pt_regs->flags (except IF = 0) */
pushq $__USER32_CS /* pt_regs->cs */
pushq $0 /* pt_regs->ip = 0 (placeholder) */
+SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
pushq %rax /* pt_regs->orig_ax */
pushq %rdi /* pt_regs->di */
pushq %rsi /* pt_regs->si */
diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
index 5d252aaeade8..e1e1c7eafa60 100644
--- a/arch/x86/xen/xen-asm_64.S
+++ b/arch/x86/xen/xen-asm_64.S
@@ -161,10 +161,22 @@ SYM_FUNC_END(xen_syscall32_target)
/* 32-bit compat sysenter target */
SYM_FUNC_START(xen_sysenter_target)
- mov 0*8(%rsp), %rcx
- mov 1*8(%rsp), %r11
- mov 5*8(%rsp), %rsp
- jmp entry_SYSENTER_compat
+ /*
+ * NB: Xen is polite and clears TF from EFLAGS for us. This means
+ * that we don't need to guard against single step exceptions here.
+ */
+ popq %rcx
+ popq %r11
+
+ /*
+ * Neither Xen nor the kernel really knows what the old SS and
+ * CS were. The kernel expects __USER32_DS and __USER32_CS, so
+ * report those values even though Xen will guess its own values.
+ */
+ movq $__USER32_DS, 4*8(%rsp)
+ movq $__USER32_CS, 1*8(%rsp)
+
+ jmp entry_SYSENTER_compat_after_hwframe
SYM_FUNC_END(xen_sysenter_target)
#else /* !CONFIG_IA32_EMULATION */
--
2.25.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
2020-06-26 17:21 ` Andy Lutomirski
@ 2020-06-28 2:47 ` Boris Ostrovsky
-1 siblings, 0 replies; 23+ messages in thread
From: Boris Ostrovsky @ 2020-06-28 2:47 UTC (permalink / raw)
To: Andy Lutomirski, x86
Cc: linux-kernel, Andrew Cooper, Juergen Gross, Stefano Stabellini,
xen-devel
On 6/26/20 1:21 PM, Andy Lutomirski wrote:
> The SYSENTER frame setup was nonsense. It worked by accident
> because the normal code into which the Xen asm jumped
> (entry_SYSENTER_32/compat) threw away SP without touching the stack.
> entry_SYSENTER_compat was recently modified such that it relied on
> having a valid stack pointer, so now the Xen asm needs to invoke it
> with a valid stack.
>
> Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
> metal prologue.
>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: xen-devel@lists.xenproject.org
> Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
@ 2020-06-28 2:47 ` Boris Ostrovsky
0 siblings, 0 replies; 23+ messages in thread
From: Boris Ostrovsky @ 2020-06-28 2:47 UTC (permalink / raw)
To: Andy Lutomirski, x86
Cc: Juergen Gross, Andrew Cooper, Stefano Stabellini, linux-kernel,
xen-devel
On 6/26/20 1:21 PM, Andy Lutomirski wrote:
> The SYSENTER frame setup was nonsense. It worked by accident
> because the normal code into which the Xen asm jumped
> (entry_SYSENTER_32/compat) threw away SP without touching the stack.
> entry_SYSENTER_compat was recently modified such that it relied on
> having a valid stack pointer, so now the Xen asm needs to invoke it
> with a valid stack.
>
> Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
> metal prologue.
>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: xen-devel@lists.xenproject.org
> Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [tip: x86/urgent] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
2020-06-26 17:21 ` Andy Lutomirski
(?)
(?)
@ 2020-07-01 8:04 ` tip-bot2 for Andy Lutomirski
-1 siblings, 0 replies; 23+ messages in thread
From: tip-bot2 for Andy Lutomirski @ 2020-07-01 8:04 UTC (permalink / raw)
To: linux-tip-commits
Cc: Andy Lutomirski, Thomas Gleixner, Boris Ostrovsky, x86, LKML
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: ffae641f57476369b4d503402b37ebe489d23395
Gitweb: https://git.kernel.org/tip/ffae641f57476369b4d503402b37ebe489d23395
Author: Andy Lutomirski <luto@kernel.org>
AuthorDate: Fri, 26 Jun 2020 10:21:13 -07:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 01 Jul 2020 10:00:26 +02:00
x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
The SYSENTER frame setup was nonsense. It worked by accident because the
normal code into which the Xen asm jumped (entry_SYSENTER_32/compat) threw
away SP without touching the stack. entry_SYSENTER_compat was recently
modified such that it relied on having a valid stack pointer, so now the
Xen asm needs to invoke it with a valid stack.
Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
metal prologue.
Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Link: https://lkml.kernel.org/r/947880c41ade688ff4836f665d0c9fcaa9bd1201.1593191971.git.luto@kernel.org
---
arch/x86/entry/entry_64_compat.S | 1 +
arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 7b9d815..381a6de 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
pushfq /* pt_regs->flags (except IF = 0) */
pushq $__USER32_CS /* pt_regs->cs */
pushq $0 /* pt_regs->ip = 0 (placeholder) */
+SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
pushq %rax /* pt_regs->orig_ax */
pushq %rdi /* pt_regs->di */
pushq %rsi /* pt_regs->si */
diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
index 5d252aa..e1e1c7e 100644
--- a/arch/x86/xen/xen-asm_64.S
+++ b/arch/x86/xen/xen-asm_64.S
@@ -161,10 +161,22 @@ SYM_FUNC_END(xen_syscall32_target)
/* 32-bit compat sysenter target */
SYM_FUNC_START(xen_sysenter_target)
- mov 0*8(%rsp), %rcx
- mov 1*8(%rsp), %r11
- mov 5*8(%rsp), %rsp
- jmp entry_SYSENTER_compat
+ /*
+ * NB: Xen is polite and clears TF from EFLAGS for us. This means
+ * that we don't need to guard against single step exceptions here.
+ */
+ popq %rcx
+ popq %r11
+
+ /*
+ * Neither Xen nor the kernel really knows what the old SS and
+ * CS were. The kernel expects __USER32_DS and __USER32_CS, so
+ * report those values even though Xen will guess its own values.
+ */
+ movq $__USER32_DS, 4*8(%rsp)
+ movq $__USER32_CS, 1*8(%rsp)
+
+ jmp entry_SYSENTER_compat_after_hwframe
SYM_FUNC_END(xen_sysenter_target)
#else /* !CONFIG_IA32_EMULATION */
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
2020-06-26 17:21 ` Andy Lutomirski
@ 2020-07-01 15:42 ` Brian Gerst
-1 siblings, 0 replies; 23+ messages in thread
From: Brian Gerst @ 2020-07-01 15:42 UTC (permalink / raw)
To: Andy Lutomirski
Cc: the arch/x86 maintainers, Linux Kernel Mailing List,
Andrew Cooper, Juergen Gross, Boris Ostrovsky,
Stefano Stabellini, xen-devel
On Fri, Jun 26, 2020 at 1:30 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> The SYSENTER frame setup was nonsense. It worked by accident
> because the normal code into which the Xen asm jumped
> (entry_SYSENTER_32/compat) threw away SP without touching the stack.
> entry_SYSENTER_compat was recently modified such that it relied on
> having a valid stack pointer, so now the Xen asm needs to invoke it
> with a valid stack.
>
> Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
> metal prologue.
>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: xen-devel@lists.xenproject.org
> Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> arch/x86/entry/entry_64_compat.S | 1 +
> arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
> index 7b9d8150f652..381a6de7de9c 100644
> --- a/arch/x86/entry/entry_64_compat.S
> +++ b/arch/x86/entry/entry_64_compat.S
> @@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
> pushfq /* pt_regs->flags (except IF = 0) */
> pushq $__USER32_CS /* pt_regs->cs */
> pushq $0 /* pt_regs->ip = 0 (placeholder) */
> +SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
This skips over the section that truncates the syscall number to
32-bits. The comments present some doubt that it is actually
necessary, but the Xen path shouldn't differ from native. That code
should be moved after this new label.
--
Brian Gerst
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
@ 2020-07-01 15:42 ` Brian Gerst
0 siblings, 0 replies; 23+ messages in thread
From: Brian Gerst @ 2020-07-01 15:42 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Juergen Gross, Stefano Stabellini, Andrew Cooper,
the arch/x86 maintainers, Linux Kernel Mailing List, xen-devel,
Boris Ostrovsky
On Fri, Jun 26, 2020 at 1:30 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> The SYSENTER frame setup was nonsense. It worked by accident
> because the normal code into which the Xen asm jumped
> (entry_SYSENTER_32/compat) threw away SP without touching the stack.
> entry_SYSENTER_compat was recently modified such that it relied on
> having a valid stack pointer, so now the Xen asm needs to invoke it
> with a valid stack.
>
> Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
> metal prologue.
>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: xen-devel@lists.xenproject.org
> Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> arch/x86/entry/entry_64_compat.S | 1 +
> arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
> index 7b9d8150f652..381a6de7de9c 100644
> --- a/arch/x86/entry/entry_64_compat.S
> +++ b/arch/x86/entry/entry_64_compat.S
> @@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
> pushfq /* pt_regs->flags (except IF = 0) */
> pushq $__USER32_CS /* pt_regs->cs */
> pushq $0 /* pt_regs->ip = 0 (placeholder) */
> +SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
This skips over the section that truncates the syscall number to
32-bits. The comments present some doubt that it is actually
necessary, but the Xen path shouldn't differ from native. That code
should be moved after this new label.
--
Brian Gerst
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
2020-07-01 15:42 ` Brian Gerst
@ 2020-07-01 18:39 ` Andy Lutomirski
-1 siblings, 0 replies; 23+ messages in thread
From: Andy Lutomirski @ 2020-07-01 18:39 UTC (permalink / raw)
To: Brian Gerst
Cc: Andy Lutomirski, the arch/x86 maintainers,
Linux Kernel Mailing List, Andrew Cooper, Juergen Gross,
Boris Ostrovsky, Stefano Stabellini, xen-devel
On Wed, Jul 1, 2020 at 8:42 AM Brian Gerst <brgerst@gmail.com> wrote:
>
> On Fri, Jun 26, 2020 at 1:30 PM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > The SYSENTER frame setup was nonsense. It worked by accident
> > because the normal code into which the Xen asm jumped
> > (entry_SYSENTER_32/compat) threw away SP without touching the stack.
> > entry_SYSENTER_compat was recently modified such that it relied on
> > having a valid stack pointer, so now the Xen asm needs to invoke it
> > with a valid stack.
> >
> > Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
> > metal prologue.
> >
> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > Cc: Juergen Gross <jgross@suse.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: xen-devel@lists.xenproject.org
> > Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> > arch/x86/entry/entry_64_compat.S | 1 +
> > arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
> > 2 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
> > index 7b9d8150f652..381a6de7de9c 100644
> > --- a/arch/x86/entry/entry_64_compat.S
> > +++ b/arch/x86/entry/entry_64_compat.S
> > @@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
> > pushfq /* pt_regs->flags (except IF = 0) */
> > pushq $__USER32_CS /* pt_regs->cs */
> > pushq $0 /* pt_regs->ip = 0 (placeholder) */
> > +SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
>
> This skips over the section that truncates the syscall number to
> 32-bits. The comments present some doubt that it is actually
> necessary, but the Xen path shouldn't differ from native. That code
> should be moved after this new label.
Whoops. I thought I caught that myself, but apparently not. I'll fix it.
>
> --
> Brian Gerst
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
@ 2020-07-01 18:39 ` Andy Lutomirski
0 siblings, 0 replies; 23+ messages in thread
From: Andy Lutomirski @ 2020-07-01 18:39 UTC (permalink / raw)
To: Brian Gerst
Cc: Juergen Gross, Stefano Stabellini, Andrew Cooper,
the arch/x86 maintainers, Linux Kernel Mailing List,
Andy Lutomirski, xen-devel, Boris Ostrovsky
On Wed, Jul 1, 2020 at 8:42 AM Brian Gerst <brgerst@gmail.com> wrote:
>
> On Fri, Jun 26, 2020 at 1:30 PM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > The SYSENTER frame setup was nonsense. It worked by accident
> > because the normal code into which the Xen asm jumped
> > (entry_SYSENTER_32/compat) threw away SP without touching the stack.
> > entry_SYSENTER_compat was recently modified such that it relied on
> > having a valid stack pointer, so now the Xen asm needs to invoke it
> > with a valid stack.
> >
> > Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
> > metal prologue.
> >
> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > Cc: Juergen Gross <jgross@suse.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: xen-devel@lists.xenproject.org
> > Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> > arch/x86/entry/entry_64_compat.S | 1 +
> > arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
> > 2 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
> > index 7b9d8150f652..381a6de7de9c 100644
> > --- a/arch/x86/entry/entry_64_compat.S
> > +++ b/arch/x86/entry/entry_64_compat.S
> > @@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
> > pushfq /* pt_regs->flags (except IF = 0) */
> > pushq $__USER32_CS /* pt_regs->cs */
> > pushq $0 /* pt_regs->ip = 0 (placeholder) */
> > +SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
>
> This skips over the section that truncates the syscall number to
> 32-bits. The comments present some doubt that it is actually
> necessary, but the Xen path shouldn't differ from native. That code
> should be moved after this new label.
Whoops. I thought I caught that myself, but apparently not. I'll fix it.
>
> --
> Brian Gerst
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
2020-07-01 18:39 ` Andy Lutomirski
@ 2020-07-02 12:54 ` Thomas Gleixner
-1 siblings, 0 replies; 23+ messages in thread
From: Thomas Gleixner @ 2020-07-02 12:54 UTC (permalink / raw)
To: Andy Lutomirski, Brian Gerst
Cc: Andy Lutomirski, the arch/x86 maintainers,
Linux Kernel Mailing List, Andrew Cooper, Juergen Gross,
Boris Ostrovsky, Stefano Stabellini, xen-devel
Andy Lutomirski <luto@kernel.org> writes:
> On Wed, Jul 1, 2020 at 8:42 AM Brian Gerst <brgerst@gmail.com> wrote:
> > On Fri, Jun 26, 2020 at 1:30 PM Andy Lutomirski <luto@kernel.org> wrote:
>> >
>> > The SYSENTER frame setup was nonsense. It worked by accident
>> > because the normal code into which the Xen asm jumped
>> > (entry_SYSENTER_32/compat) threw away SP without touching the stack.
>> > entry_SYSENTER_compat was recently modified such that it relied on
>> > having a valid stack pointer, so now the Xen asm needs to invoke it
>> > with a valid stack.
>> >
>> > Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
>> > metal prologue.
>> >
>> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> > Cc: Juergen Gross <jgross@suse.com>
>> > Cc: Stefano Stabellini <sstabellini@kernel.org>
>> > Cc: xen-devel@lists.xenproject.org
>> > Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
>> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> > ---
>> > arch/x86/entry/entry_64_compat.S | 1 +
>> > arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
>> > 2 files changed, 17 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
>> > index 7b9d8150f652..381a6de7de9c 100644
>> > --- a/arch/x86/entry/entry_64_compat.S
>> > +++ b/arch/x86/entry/entry_64_compat.S
>> > @@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
>> > pushfq /* pt_regs->flags (except IF = 0) */
>> > pushq $__USER32_CS /* pt_regs->cs */
>> > pushq $0 /* pt_regs->ip = 0 (placeholder) */
>> > +SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
>>
>> This skips over the section that truncates the syscall number to
>> 32-bits. The comments present some doubt that it is actually
>> necessary, but the Xen path shouldn't differ from native. That code
>> should be moved after this new label.
>
> Whoops. I thought I caught that myself, but apparently not. I'll fix it.
Darn. I already applied that lot. Can you please send a delta fix?
Thanks,
tglx
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 3/6] x86/entry/64/compat: Fix Xen PV SYSENTER frame setup
@ 2020-07-02 12:54 ` Thomas Gleixner
0 siblings, 0 replies; 23+ messages in thread
From: Thomas Gleixner @ 2020-07-02 12:54 UTC (permalink / raw)
To: Andy Lutomirski, Brian Gerst
Cc: Juergen Gross, Stefano Stabellini, Andrew Cooper,
the arch/x86 maintainers, Linux Kernel Mailing List,
Andy Lutomirski, xen-devel, Boris Ostrovsky
Andy Lutomirski <luto@kernel.org> writes:
> On Wed, Jul 1, 2020 at 8:42 AM Brian Gerst <brgerst@gmail.com> wrote:
> > On Fri, Jun 26, 2020 at 1:30 PM Andy Lutomirski <luto@kernel.org> wrote:
>> >
>> > The SYSENTER frame setup was nonsense. It worked by accident
>> > because the normal code into which the Xen asm jumped
>> > (entry_SYSENTER_32/compat) threw away SP without touching the stack.
>> > entry_SYSENTER_compat was recently modified such that it relied on
>> > having a valid stack pointer, so now the Xen asm needs to invoke it
>> > with a valid stack.
>> >
>> > Fix it up like SYSCALL: use the Xen-provided frame and skip the bare
>> > metal prologue.
>> >
>> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> > Cc: Juergen Gross <jgross@suse.com>
>> > Cc: Stefano Stabellini <sstabellini@kernel.org>
>> > Cc: xen-devel@lists.xenproject.org
>> > Fixes: 1c3e5d3f60e2 ("x86/entry: Make entry_64_compat.S objtool clean")
>> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> > ---
>> > arch/x86/entry/entry_64_compat.S | 1 +
>> > arch/x86/xen/xen-asm_64.S | 20 ++++++++++++++++----
>> > 2 files changed, 17 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
>> > index 7b9d8150f652..381a6de7de9c 100644
>> > --- a/arch/x86/entry/entry_64_compat.S
>> > +++ b/arch/x86/entry/entry_64_compat.S
>> > @@ -79,6 +79,7 @@ SYM_CODE_START(entry_SYSENTER_compat)
>> > pushfq /* pt_regs->flags (except IF = 0) */
>> > pushq $__USER32_CS /* pt_regs->cs */
>> > pushq $0 /* pt_regs->ip = 0 (placeholder) */
>> > +SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
>>
>> This skips over the section that truncates the syscall number to
>> 32-bits. The comments present some doubt that it is actually
>> necessary, but the Xen path shouldn't differ from native. That code
>> should be moved after this new label.
>
> Whoops. I thought I caught that myself, but apparently not. I'll fix it.
Darn. I already applied that lot. Can you please send a delta fix?
Thanks,
tglx
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/6] selftests/x86/syscall_nt: Add more flag combinations
2020-06-26 17:21 [PATCH 0/6] x86/entry: Fixes Andy Lutomirski
` (2 preceding siblings ...)
2020-06-26 17:21 ` Andy Lutomirski
@ 2020-06-26 17:21 ` Andy Lutomirski
2020-07-01 8:04 ` [tip: x86/urgent] " tip-bot2 for Andy Lutomirski
2020-06-26 17:21 ` [PATCH 5/6] selftests/x86/syscall_nt: Clear weird flags after each test Andy Lutomirski
2020-06-26 17:21 ` [PATCH 6/6] selftests/x86: Consolidate and fix get/set_eflags() helpers Andy Lutomirski
5 siblings, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2020-06-26 17:21 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Andrew Cooper, Juergen Gross, Andy Lutomirski
Add EFLAGS.AC to the mix.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
tools/testing/selftests/x86/syscall_nt.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/tools/testing/selftests/x86/syscall_nt.c b/tools/testing/selftests/x86/syscall_nt.c
index 02309a195041..f060534b66a0 100644
--- a/tools/testing/selftests/x86/syscall_nt.c
+++ b/tools/testing/selftests/x86/syscall_nt.c
@@ -73,6 +73,12 @@ int main(void)
printf("[RUN]\tSet NT and issue a syscall\n");
do_it(X86_EFLAGS_NT);
+ printf("[RUN]\tSet AC and issue a syscall\n");
+ do_it(X86_EFLAGS_AC);
+
+ printf("[RUN]\tSet NT|AC and issue a syscall\n");
+ do_it(X86_EFLAGS_NT | X86_EFLAGS_AC);
+
/*
* Now try it again with TF set -- TF forces returns via IRET in all
* cases except non-ptregs-using 64-bit full fast path syscalls.
@@ -80,8 +86,17 @@ int main(void)
sethandler(SIGTRAP, sigtrap, 0);
+ printf("[RUN]\tSet TF and issue a syscall\n");
+ do_it(X86_EFLAGS_TF);
+
printf("[RUN]\tSet NT|TF and issue a syscall\n");
do_it(X86_EFLAGS_NT | X86_EFLAGS_TF);
+ printf("[RUN]\tSet AC|TF and issue a syscall\n");
+ do_it(X86_EFLAGS_AC | X86_EFLAGS_TF);
+
+ printf("[RUN]\tSet NT|AC|TF and issue a syscall\n");
+ do_it(X86_EFLAGS_NT | X86_EFLAGS_AC | X86_EFLAGS_TF);
+
return nerrs == 0 ? 0 : 1;
}
--
2.25.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [tip: x86/urgent] selftests/x86/syscall_nt: Add more flag combinations
2020-06-26 17:21 ` [PATCH 4/6] selftests/x86/syscall_nt: Add more flag combinations Andy Lutomirski
@ 2020-07-01 8:04 ` tip-bot2 for Andy Lutomirski
0 siblings, 0 replies; 23+ messages in thread
From: tip-bot2 for Andy Lutomirski @ 2020-07-01 8:04 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Andy Lutomirski, Thomas Gleixner, x86, LKML
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: e4ef7de160c6b12639c4fc49bcacb25b860ac76d
Gitweb: https://git.kernel.org/tip/e4ef7de160c6b12639c4fc49bcacb25b860ac76d
Author: Andy Lutomirski <luto@kernel.org>
AuthorDate: Fri, 26 Jun 2020 10:21:14 -07:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 01 Jul 2020 10:00:26 +02:00
selftests/x86/syscall_nt: Add more flag combinations
Add EFLAGS.AC to the mix.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/12924e2fe2c5826568b7fc9436d85ca7f5eb1743.1593191971.git.luto@kernel.org
---
tools/testing/selftests/x86/syscall_nt.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/tools/testing/selftests/x86/syscall_nt.c b/tools/testing/selftests/x86/syscall_nt.c
index 02309a1..f060534 100644
--- a/tools/testing/selftests/x86/syscall_nt.c
+++ b/tools/testing/selftests/x86/syscall_nt.c
@@ -73,6 +73,12 @@ int main(void)
printf("[RUN]\tSet NT and issue a syscall\n");
do_it(X86_EFLAGS_NT);
+ printf("[RUN]\tSet AC and issue a syscall\n");
+ do_it(X86_EFLAGS_AC);
+
+ printf("[RUN]\tSet NT|AC and issue a syscall\n");
+ do_it(X86_EFLAGS_NT | X86_EFLAGS_AC);
+
/*
* Now try it again with TF set -- TF forces returns via IRET in all
* cases except non-ptregs-using 64-bit full fast path syscalls.
@@ -80,8 +86,17 @@ int main(void)
sethandler(SIGTRAP, sigtrap, 0);
+ printf("[RUN]\tSet TF and issue a syscall\n");
+ do_it(X86_EFLAGS_TF);
+
printf("[RUN]\tSet NT|TF and issue a syscall\n");
do_it(X86_EFLAGS_NT | X86_EFLAGS_TF);
+ printf("[RUN]\tSet AC|TF and issue a syscall\n");
+ do_it(X86_EFLAGS_AC | X86_EFLAGS_TF);
+
+ printf("[RUN]\tSet NT|AC|TF and issue a syscall\n");
+ do_it(X86_EFLAGS_NT | X86_EFLAGS_AC | X86_EFLAGS_TF);
+
return nerrs == 0 ? 0 : 1;
}
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 5/6] selftests/x86/syscall_nt: Clear weird flags after each test
2020-06-26 17:21 [PATCH 0/6] x86/entry: Fixes Andy Lutomirski
` (3 preceding siblings ...)
2020-06-26 17:21 ` [PATCH 4/6] selftests/x86/syscall_nt: Add more flag combinations Andy Lutomirski
@ 2020-06-26 17:21 ` Andy Lutomirski
2020-07-01 8:04 ` [tip: x86/urgent] " tip-bot2 for Andy Lutomirski
2020-06-26 17:21 ` [PATCH 6/6] selftests/x86: Consolidate and fix get/set_eflags() helpers Andy Lutomirski
5 siblings, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2020-06-26 17:21 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Andrew Cooper, Juergen Gross, Andy Lutomirski
Clear the weird flags before logging to improve strace output --
logging results while, say, TF is set does no one any favors.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
tools/testing/selftests/x86/syscall_nt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/x86/syscall_nt.c b/tools/testing/selftests/x86/syscall_nt.c
index f060534b66a0..5fc82b9cebed 100644
--- a/tools/testing/selftests/x86/syscall_nt.c
+++ b/tools/testing/selftests/x86/syscall_nt.c
@@ -59,6 +59,7 @@ static void do_it(unsigned long extraflags)
set_eflags(get_eflags() | extraflags);
syscall(SYS_getpid);
flags = get_eflags();
+ set_eflags(X86_EFLAGS_IF | X86_EFLAGS_FIXED);
if ((flags & extraflags) == extraflags) {
printf("[OK]\tThe syscall worked and flags are still set\n");
} else {
--
2.25.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [tip: x86/urgent] selftests/x86/syscall_nt: Clear weird flags after each test
2020-06-26 17:21 ` [PATCH 5/6] selftests/x86/syscall_nt: Clear weird flags after each test Andy Lutomirski
@ 2020-07-01 8:04 ` tip-bot2 for Andy Lutomirski
0 siblings, 0 replies; 23+ messages in thread
From: tip-bot2 for Andy Lutomirski @ 2020-07-01 8:04 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Andy Lutomirski, Thomas Gleixner, x86, LKML
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: a61fa2799ef9bf6c4f54cf7295036577cececc72
Gitweb: https://git.kernel.org/tip/a61fa2799ef9bf6c4f54cf7295036577cececc72
Author: Andy Lutomirski <luto@kernel.org>
AuthorDate: Fri, 26 Jun 2020 10:21:15 -07:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 01 Jul 2020 10:00:26 +02:00
selftests/x86/syscall_nt: Clear weird flags after each test
Clear the weird flags before logging to improve strace output --
logging results while, say, TF is set does no one any favors.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/907bfa5a42d4475b8245e18b67a04b13ca51ffdb.1593191971.git.luto@kernel.org
---
tools/testing/selftests/x86/syscall_nt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/x86/syscall_nt.c b/tools/testing/selftests/x86/syscall_nt.c
index f060534..5fc82b9 100644
--- a/tools/testing/selftests/x86/syscall_nt.c
+++ b/tools/testing/selftests/x86/syscall_nt.c
@@ -59,6 +59,7 @@ static void do_it(unsigned long extraflags)
set_eflags(get_eflags() | extraflags);
syscall(SYS_getpid);
flags = get_eflags();
+ set_eflags(X86_EFLAGS_IF | X86_EFLAGS_FIXED);
if ((flags & extraflags) == extraflags) {
printf("[OK]\tThe syscall worked and flags are still set\n");
} else {
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 6/6] selftests/x86: Consolidate and fix get/set_eflags() helpers
2020-06-26 17:21 [PATCH 0/6] x86/entry: Fixes Andy Lutomirski
` (4 preceding siblings ...)
2020-06-26 17:21 ` [PATCH 5/6] selftests/x86/syscall_nt: Clear weird flags after each test Andy Lutomirski
@ 2020-06-26 17:21 ` Andy Lutomirski
2020-07-01 8:04 ` [tip: x86/urgent] " tip-bot2 for Andy Lutomirski
5 siblings, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2020-06-26 17:21 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Andrew Cooper, Juergen Gross, Andy Lutomirski
We had several copies of get_eflags() and set_eflags() and they were
all buggy. Consolidate them and fix them. The fixes are:
Add memory clobbers. These are probably unnecessary but they make
sure sure that the compiler doesn't move something past one of these
calls when it shouldn't.
Respect the redzone on x86_64. I haven't seen a failure related to
this, but it's definitely a bug.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
tools/testing/selftests/x86/Makefile | 4 +-
tools/testing/selftests/x86/helpers.h | 41 +++++++++++++++++++
.../selftests/x86/single_step_syscall.c | 17 +-------
.../testing/selftests/x86/syscall_arg_fault.c | 21 +---------
tools/testing/selftests/x86/syscall_nt.c | 20 +--------
tools/testing/selftests/x86/test_vsyscall.c | 15 +------
tools/testing/selftests/x86/unwind_vdso.c | 23 +----------
7 files changed, 51 insertions(+), 90 deletions(-)
create mode 100644 tools/testing/selftests/x86/helpers.h
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index 3ff94575d02d..6703c7906b71 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -70,10 +70,10 @@ all_64: $(BINARIES_64)
EXTRA_CLEAN := $(BINARIES_32) $(BINARIES_64)
-$(BINARIES_32): $(OUTPUT)/%_32: %.c
+$(BINARIES_32): $(OUTPUT)/%_32: %.c helpers.h
$(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl -lm
-$(BINARIES_64): $(OUTPUT)/%_64: %.c
+$(BINARIES_64): $(OUTPUT)/%_64: %.c helpers.h
$(CC) -m64 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl
# x86_64 users should be encouraged to install 32-bit libraries
diff --git a/tools/testing/selftests/x86/helpers.h b/tools/testing/selftests/x86/helpers.h
new file mode 100644
index 000000000000..f5ff2a2615df
--- /dev/null
+++ b/tools/testing/selftests/x86/helpers.h
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#ifndef __SELFTESTS_X86_HELPERS_H
+#define __SELFTESTS_X86_HELPERS_H
+
+#include <asm/processor-flags.h>
+
+static inline unsigned long get_eflags(void)
+{
+ unsigned long eflags;
+
+ asm volatile (
+#ifdef __x86_64__
+ "subq $128, %%rsp\n\t"
+ "pushfq\n\t"
+ "popq %0\n\t"
+ "addq $128, %%rsp"
+#else
+ "pushfl\n\t"
+ "popl %0"
+#endif
+ : "=r" (eflags) :: "memory");
+
+ return eflags;
+}
+
+static inline void set_eflags(unsigned long eflags)
+{
+ asm volatile (
+#ifdef __x86_64__
+ "subq $128, %%rsp\n\t"
+ "pushq %0\n\t"
+ "popfq\n\t"
+ "addq $128, %%rsp"
+#else
+ "pushl %0\n\t"
+ "popfl"
+#endif
+ :: "r" (eflags) : "flags", "memory");
+}
+
+#endif /* __SELFTESTS_X86_HELPERS_H */
diff --git a/tools/testing/selftests/x86/single_step_syscall.c b/tools/testing/selftests/x86/single_step_syscall.c
index 1063328e275c..120ac741fe44 100644
--- a/tools/testing/selftests/x86/single_step_syscall.c
+++ b/tools/testing/selftests/x86/single_step_syscall.c
@@ -31,6 +31,8 @@
#include <sys/ptrace.h>
#include <sys/user.h>
+#include "helpers.h"
+
static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
int flags)
{
@@ -67,21 +69,6 @@ static unsigned char altstack_data[SIGSTKSZ];
# define INT80_CLOBBERS
#endif
-static unsigned long get_eflags(void)
-{
- unsigned long eflags;
- asm volatile ("pushf" WIDTH "\n\tpop" WIDTH " %0" : "=rm" (eflags));
- return eflags;
-}
-
-static void set_eflags(unsigned long eflags)
-{
- asm volatile ("push" WIDTH " %0\n\tpopf" WIDTH
- : : "rm" (eflags) : "flags");
-}
-
-#define X86_EFLAGS_TF (1UL << 8)
-
static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
{
ucontext_t *ctx = (ucontext_t*)ctx_void;
diff --git a/tools/testing/selftests/x86/syscall_arg_fault.c b/tools/testing/selftests/x86/syscall_arg_fault.c
index bc0ecc2e862e..5b7abebbcbb9 100644
--- a/tools/testing/selftests/x86/syscall_arg_fault.c
+++ b/tools/testing/selftests/x86/syscall_arg_fault.c
@@ -15,30 +15,11 @@
#include <setjmp.h>
#include <errno.h>
-#ifdef __x86_64__
-# define WIDTH "q"
-#else
-# define WIDTH "l"
-#endif
+#include "helpers.h"
/* Our sigaltstack scratch space. */
static unsigned char altstack_data[SIGSTKSZ];
-static unsigned long get_eflags(void)
-{
- unsigned long eflags;
- asm volatile ("pushf" WIDTH "\n\tpop" WIDTH " %0" : "=rm" (eflags));
- return eflags;
-}
-
-static void set_eflags(unsigned long eflags)
-{
- asm volatile ("push" WIDTH " %0\n\tpopf" WIDTH
- : : "rm" (eflags) : "flags");
-}
-
-#define X86_EFLAGS_TF (1UL << 8)
-
static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
int flags)
{
diff --git a/tools/testing/selftests/x86/syscall_nt.c b/tools/testing/selftests/x86/syscall_nt.c
index 5fc82b9cebed..970e5e14d96d 100644
--- a/tools/testing/selftests/x86/syscall_nt.c
+++ b/tools/testing/selftests/x86/syscall_nt.c
@@ -13,29 +13,11 @@
#include <signal.h>
#include <err.h>
#include <sys/syscall.h>
-#include <asm/processor-flags.h>
-#ifdef __x86_64__
-# define WIDTH "q"
-#else
-# define WIDTH "l"
-#endif
+#include "helpers.h"
static unsigned int nerrs;
-static unsigned long get_eflags(void)
-{
- unsigned long eflags;
- asm volatile ("pushf" WIDTH "\n\tpop" WIDTH " %0" : "=rm" (eflags));
- return eflags;
-}
-
-static void set_eflags(unsigned long eflags)
-{
- asm volatile ("push" WIDTH " %0\n\tpopf" WIDTH
- : : "rm" (eflags) : "flags");
-}
-
static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
int flags)
{
diff --git a/tools/testing/selftests/x86/test_vsyscall.c b/tools/testing/selftests/x86/test_vsyscall.c
index a4f4d4cf22c3..c41f24b517f4 100644
--- a/tools/testing/selftests/x86/test_vsyscall.c
+++ b/tools/testing/selftests/x86/test_vsyscall.c
@@ -20,6 +20,8 @@
#include <setjmp.h>
#include <sys/uio.h>
+#include "helpers.h"
+
#ifdef __x86_64__
# define VSYS(x) (x)
#else
@@ -493,21 +495,8 @@ static int test_process_vm_readv(void)
}
#ifdef __x86_64__
-#define X86_EFLAGS_TF (1UL << 8)
static volatile sig_atomic_t num_vsyscall_traps;
-static unsigned long get_eflags(void)
-{
- unsigned long eflags;
- asm volatile ("pushfq\n\tpopq %0" : "=rm" (eflags));
- return eflags;
-}
-
-static void set_eflags(unsigned long eflags)
-{
- asm volatile ("pushq %0\n\tpopfq" : : "rm" (eflags) : "flags");
-}
-
static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
{
ucontext_t *ctx = (ucontext_t *)ctx_void;
diff --git a/tools/testing/selftests/x86/unwind_vdso.c b/tools/testing/selftests/x86/unwind_vdso.c
index 0075ccd65407..4c311e1af4c7 100644
--- a/tools/testing/selftests/x86/unwind_vdso.c
+++ b/tools/testing/selftests/x86/unwind_vdso.c
@@ -11,6 +11,8 @@
#include <features.h>
#include <stdio.h>
+#include "helpers.h"
+
#if defined(__GLIBC__) && __GLIBC__ == 2 && __GLIBC_MINOR__ < 16
int main()
@@ -53,27 +55,6 @@ static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
err(1, "sigaction");
}
-#ifdef __x86_64__
-# define WIDTH "q"
-#else
-# define WIDTH "l"
-#endif
-
-static unsigned long get_eflags(void)
-{
- unsigned long eflags;
- asm volatile ("pushf" WIDTH "\n\tpop" WIDTH " %0" : "=rm" (eflags));
- return eflags;
-}
-
-static void set_eflags(unsigned long eflags)
-{
- asm volatile ("push" WIDTH " %0\n\tpopf" WIDTH
- : : "rm" (eflags) : "flags");
-}
-
-#define X86_EFLAGS_TF (1UL << 8)
-
static volatile sig_atomic_t nerrs;
static unsigned long sysinfo;
static bool got_sysinfo = false;
--
2.25.4
^ permalink raw reply related [flat|nested] 23+ messages in thread* [tip: x86/urgent] selftests/x86: Consolidate and fix get/set_eflags() helpers
2020-06-26 17:21 ` [PATCH 6/6] selftests/x86: Consolidate and fix get/set_eflags() helpers Andy Lutomirski
@ 2020-07-01 8:04 ` tip-bot2 for Andy Lutomirski
0 siblings, 0 replies; 23+ messages in thread
From: tip-bot2 for Andy Lutomirski @ 2020-07-01 8:04 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Andy Lutomirski, Thomas Gleixner, x86, LKML
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: cced0b24bb545bfe74fea96de84adc23c0146b05
Gitweb: https://git.kernel.org/tip/cced0b24bb545bfe74fea96de84adc23c0146b05
Author: Andy Lutomirski <luto@kernel.org>
AuthorDate: Fri, 26 Jun 2020 10:21:16 -07:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 01 Jul 2020 10:00:27 +02:00
selftests/x86: Consolidate and fix get/set_eflags() helpers
There are several copies of get_eflags() and set_eflags() and they all are
buggy. Consolidate them and fix them. The fixes are:
Add memory clobbers. These are probably unnecessary but they make sure
that the compiler doesn't move something past one of these calls when it
shouldn't.
Respect the redzone on x86_64. There has no failure been observed related
to this, but it's definitely a bug.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/982ce58ae8dea2f1e57093ee894760e35267e751.1593191971.git.luto@kernel.org
---
tools/testing/selftests/x86/Makefile | 4 +-
tools/testing/selftests/x86/helpers.h | 41 ++++++++++++++-
tools/testing/selftests/x86/single_step_syscall.c | 17 +------
tools/testing/selftests/x86/syscall_arg_fault.c | 21 +-------
tools/testing/selftests/x86/syscall_nt.c | 20 +-------
tools/testing/selftests/x86/test_vsyscall.c | 15 +-----
tools/testing/selftests/x86/unwind_vdso.c | 23 +--------
7 files changed, 51 insertions(+), 90 deletions(-)
create mode 100644 tools/testing/selftests/x86/helpers.h
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index 5f16821..d2796ea 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -70,10 +70,10 @@ all_64: $(BINARIES_64)
EXTRA_CLEAN := $(BINARIES_32) $(BINARIES_64)
-$(BINARIES_32): $(OUTPUT)/%_32: %.c
+$(BINARIES_32): $(OUTPUT)/%_32: %.c helpers.h
$(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl -lm
-$(BINARIES_64): $(OUTPUT)/%_64: %.c
+$(BINARIES_64): $(OUTPUT)/%_64: %.c helpers.h
$(CC) -m64 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl
# x86_64 users should be encouraged to install 32-bit libraries
diff --git a/tools/testing/selftests/x86/helpers.h b/tools/testing/selftests/x86/helpers.h
new file mode 100644
index 0000000..f5ff2a2
--- /dev/null
+++ b/tools/testing/selftests/x86/helpers.h
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#ifndef __SELFTESTS_X86_HELPERS_H
+#define __SELFTESTS_X86_HELPERS_H
+
+#include <asm/processor-flags.h>
+
+static inline unsigned long get_eflags(void)
+{
+ unsigned long eflags;
+
+ asm volatile (
+#ifdef __x86_64__
+ "subq $128, %%rsp\n\t"
+ "pushfq\n\t"
+ "popq %0\n\t"
+ "addq $128, %%rsp"
+#else
+ "pushfl\n\t"
+ "popl %0"
+#endif
+ : "=r" (eflags) :: "memory");
+
+ return eflags;
+}
+
+static inline void set_eflags(unsigned long eflags)
+{
+ asm volatile (
+#ifdef __x86_64__
+ "subq $128, %%rsp\n\t"
+ "pushq %0\n\t"
+ "popfq\n\t"
+ "addq $128, %%rsp"
+#else
+ "pushl %0\n\t"
+ "popfl"
+#endif
+ :: "r" (eflags) : "flags", "memory");
+}
+
+#endif /* __SELFTESTS_X86_HELPERS_H */
diff --git a/tools/testing/selftests/x86/single_step_syscall.c b/tools/testing/selftests/x86/single_step_syscall.c
index 1063328..120ac74 100644
--- a/tools/testing/selftests/x86/single_step_syscall.c
+++ b/tools/testing/selftests/x86/single_step_syscall.c
@@ -31,6 +31,8 @@
#include <sys/ptrace.h>
#include <sys/user.h>
+#include "helpers.h"
+
static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
int flags)
{
@@ -67,21 +69,6 @@ static unsigned char altstack_data[SIGSTKSZ];
# define INT80_CLOBBERS
#endif
-static unsigned long get_eflags(void)
-{
- unsigned long eflags;
- asm volatile ("pushf" WIDTH "\n\tpop" WIDTH " %0" : "=rm" (eflags));
- return eflags;
-}
-
-static void set_eflags(unsigned long eflags)
-{
- asm volatile ("push" WIDTH " %0\n\tpopf" WIDTH
- : : "rm" (eflags) : "flags");
-}
-
-#define X86_EFLAGS_TF (1UL << 8)
-
static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
{
ucontext_t *ctx = (ucontext_t*)ctx_void;
diff --git a/tools/testing/selftests/x86/syscall_arg_fault.c b/tools/testing/selftests/x86/syscall_arg_fault.c
index bc0ecc2..5b7abeb 100644
--- a/tools/testing/selftests/x86/syscall_arg_fault.c
+++ b/tools/testing/selftests/x86/syscall_arg_fault.c
@@ -15,30 +15,11 @@
#include <setjmp.h>
#include <errno.h>
-#ifdef __x86_64__
-# define WIDTH "q"
-#else
-# define WIDTH "l"
-#endif
+#include "helpers.h"
/* Our sigaltstack scratch space. */
static unsigned char altstack_data[SIGSTKSZ];
-static unsigned long get_eflags(void)
-{
- unsigned long eflags;
- asm volatile ("pushf" WIDTH "\n\tpop" WIDTH " %0" : "=rm" (eflags));
- return eflags;
-}
-
-static void set_eflags(unsigned long eflags)
-{
- asm volatile ("push" WIDTH " %0\n\tpopf" WIDTH
- : : "rm" (eflags) : "flags");
-}
-
-#define X86_EFLAGS_TF (1UL << 8)
-
static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
int flags)
{
diff --git a/tools/testing/selftests/x86/syscall_nt.c b/tools/testing/selftests/x86/syscall_nt.c
index 5fc82b9..970e5e1 100644
--- a/tools/testing/selftests/x86/syscall_nt.c
+++ b/tools/testing/selftests/x86/syscall_nt.c
@@ -13,29 +13,11 @@
#include <signal.h>
#include <err.h>
#include <sys/syscall.h>
-#include <asm/processor-flags.h>
-#ifdef __x86_64__
-# define WIDTH "q"
-#else
-# define WIDTH "l"
-#endif
+#include "helpers.h"
static unsigned int nerrs;
-static unsigned long get_eflags(void)
-{
- unsigned long eflags;
- asm volatile ("pushf" WIDTH "\n\tpop" WIDTH " %0" : "=rm" (eflags));
- return eflags;
-}
-
-static void set_eflags(unsigned long eflags)
-{
- asm volatile ("push" WIDTH " %0\n\tpopf" WIDTH
- : : "rm" (eflags) : "flags");
-}
-
static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
int flags)
{
diff --git a/tools/testing/selftests/x86/test_vsyscall.c b/tools/testing/selftests/x86/test_vsyscall.c
index a4f4d4c..c41f24b 100644
--- a/tools/testing/selftests/x86/test_vsyscall.c
+++ b/tools/testing/selftests/x86/test_vsyscall.c
@@ -20,6 +20,8 @@
#include <setjmp.h>
#include <sys/uio.h>
+#include "helpers.h"
+
#ifdef __x86_64__
# define VSYS(x) (x)
#else
@@ -493,21 +495,8 @@ static int test_process_vm_readv(void)
}
#ifdef __x86_64__
-#define X86_EFLAGS_TF (1UL << 8)
static volatile sig_atomic_t num_vsyscall_traps;
-static unsigned long get_eflags(void)
-{
- unsigned long eflags;
- asm volatile ("pushfq\n\tpopq %0" : "=rm" (eflags));
- return eflags;
-}
-
-static void set_eflags(unsigned long eflags)
-{
- asm volatile ("pushq %0\n\tpopfq" : : "rm" (eflags) : "flags");
-}
-
static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
{
ucontext_t *ctx = (ucontext_t *)ctx_void;
diff --git a/tools/testing/selftests/x86/unwind_vdso.c b/tools/testing/selftests/x86/unwind_vdso.c
index 0075ccd..4c311e1 100644
--- a/tools/testing/selftests/x86/unwind_vdso.c
+++ b/tools/testing/selftests/x86/unwind_vdso.c
@@ -11,6 +11,8 @@
#include <features.h>
#include <stdio.h>
+#include "helpers.h"
+
#if defined(__GLIBC__) && __GLIBC__ == 2 && __GLIBC_MINOR__ < 16
int main()
@@ -53,27 +55,6 @@ static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
err(1, "sigaction");
}
-#ifdef __x86_64__
-# define WIDTH "q"
-#else
-# define WIDTH "l"
-#endif
-
-static unsigned long get_eflags(void)
-{
- unsigned long eflags;
- asm volatile ("pushf" WIDTH "\n\tpop" WIDTH " %0" : "=rm" (eflags));
- return eflags;
-}
-
-static void set_eflags(unsigned long eflags)
-{
- asm volatile ("push" WIDTH " %0\n\tpopf" WIDTH
- : : "rm" (eflags) : "flags");
-}
-
-#define X86_EFLAGS_TF (1UL << 8)
-
static volatile sig_atomic_t nerrs;
static unsigned long sysinfo;
static bool got_sysinfo = false;
^ permalink raw reply related [flat|nested] 23+ messages in thread