[PATCH 0/3] x86/entry/64: Fixes for syscall rework

[PATCH 0/3] x86/entry/64: Fixes for syscall rework

[Andy Lutomirski]

I broke iopl(2) with my syscall rework.  Fix it up.  While debugging
it, I found a bug in my IRQ state handling.  Fix that, too.

Andy Lutomirski (3):
  x86/entry/64: Fix an IRQ state error on ptregs-using syscalls
  x86/entry/64: Fix fast-path syscall return register state
  x86/syscalls/64: Mark sys_iopl as using ptregs

 arch/x86/entry/entry_64.S              | 23 ++++++++++++++++-------
 arch/x86/entry/syscalls/syscall_64.tbl |  2 +-
 2 files changed, 17 insertions(+), 8 deletions(-)

-- 
2.5.0

[PATCH 1/3] x86/entry/64: Fix an IRQ state error on ptregs-using syscalls

[Andy Lutomirski]

I messed up the IRQ state when jumping off the fast path due to
invocation of a ptregs-using syscall.  This bug shouldn't have had
any impact yet, but it would have caused problems with subsequent
context tracking cleanups.

Fixes: 1e423bff959e x86/entry/64: ("Migrate the 64-bit syscall slow path to C")
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/entry_64.S | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 567aa522ac0a..9f7bb808035e 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -191,8 +191,8 @@ entry_SYSCALL_64_fastpath:
 
 	/*
 	 * This call instruction is handled specially in stub_ptregs_64.
-	 * It might end up jumping to the slow path.  If it jumps, RAX is
-	 * clobbered.
+	 * It might end up jumping to the slow path.  If it jumps, RAX
+	 * and all argument registers are clobbered.
 	 */
 	call	*sys_call_table(, %rax, 8)
 .Lentry_SYSCALL_64_after_fastpath_call:
@@ -315,17 +315,24 @@ END(entry_SYSCALL_64)
 ENTRY(stub_ptregs_64)
 	/*
 	 * Syscalls marked as needing ptregs land here.
-	 * If we are on the fast path, we need to save the extra regs.
-	 * If we are on the slow path, the extra regs are already saved.
+	 * If we are on the fast path, we need to save the extra regs,
+	 * which we achieve by trying again on the slow path.  If we are on
+	 * the slow path, the extra regs are already saved.
 	 *
 	 * RAX stores a pointer to the C function implementing the syscall.
+	 * IRQs are on.
 	 */
 	cmpq	$.Lentry_SYSCALL_64_after_fastpath_call, (%rsp)
 	jne	1f
 
-	/* Called from fast path -- pop return address and jump to slow path */
+	/*
+	 * Called from fast path -- disable IRQs again, pop return address
+	 * and jump to slow path
+	 */
+	DISABLE_INTERRUPTS(CLBR_NONE)
+	TRACE_IRQS_OFF
 	popq	%rax
-	jmp	entry_SYSCALL64_slow_path	/* called from fast path */
+	jmp	entry_SYSCALL64_slow_path
 
 1:
 	/* Called from C */
-- 
2.5.0

[tip:x86/asm] x86/entry/64: Fix an IRQ state error on ptregs-using syscalls

[tip-bot for Andy Lutomirski]

Commit-ID:  b7765086b7c5a5be029a739c2caa161da51c2076
Gitweb:     http://git.kernel.org/tip/b7765086b7c5a5be029a739c2caa161da51c2076
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Sun, 31 Jan 2016 09:33:26 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 1 Feb 2016 08:53:25 +0100

x86/entry/64: Fix an IRQ state error on ptregs-using syscalls

I messed up the IRQ state when jumping off the fast path due to
invocation of a ptregs-using syscall.  This bug shouldn't have
had any impact yet, but it would have caused problems with
subsequent context tracking cleanups.

Reported-and-tested-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
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: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 1e423bff959e x86/entry/64: ("Migrate the 64-bit syscall slow path to C")
Link: http://lkml.kernel.org/r/ab92cd365fb7b0a56869e920017790d96610fdca.1454261517.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/entry_64.S | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 567aa52..9f7bb80 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -191,8 +191,8 @@ entry_SYSCALL_64_fastpath:
 
 	/*
 	 * This call instruction is handled specially in stub_ptregs_64.
-	 * It might end up jumping to the slow path.  If it jumps, RAX is
-	 * clobbered.
+	 * It might end up jumping to the slow path.  If it jumps, RAX
+	 * and all argument registers are clobbered.
 	 */
 	call	*sys_call_table(, %rax, 8)
 .Lentry_SYSCALL_64_after_fastpath_call:
@@ -315,17 +315,24 @@ END(entry_SYSCALL_64)
 ENTRY(stub_ptregs_64)
 	/*
 	 * Syscalls marked as needing ptregs land here.
-	 * If we are on the fast path, we need to save the extra regs.
-	 * If we are on the slow path, the extra regs are already saved.
+	 * If we are on the fast path, we need to save the extra regs,
+	 * which we achieve by trying again on the slow path.  If we are on
+	 * the slow path, the extra regs are already saved.
 	 *
 	 * RAX stores a pointer to the C function implementing the syscall.
+	 * IRQs are on.
 	 */
 	cmpq	$.Lentry_SYSCALL_64_after_fastpath_call, (%rsp)
 	jne	1f
 
-	/* Called from fast path -- pop return address and jump to slow path */
+	/*
+	 * Called from fast path -- disable IRQs again, pop return address
+	 * and jump to slow path
+	 */
+	DISABLE_INTERRUPTS(CLBR_NONE)
+	TRACE_IRQS_OFF
 	popq	%rax
-	jmp	entry_SYSCALL64_slow_path	/* called from fast path */
+	jmp	entry_SYSCALL64_slow_path
 
 1:
 	/* Called from C */

[PATCH 2/3] x86/entry/64: Fix fast-path syscall return register state

[Andy Lutomirski]

I was fishing RIP (i.e. RCX) out of pt_regs->cx and RFLAGS (i.e. R11)
out of pt_regs->r11.  While it usually worked (pt_regs started out
with cx == ip and r11 == flags), it was very fragile.  In particular,
it broke iopl because iopl forgot to mark itself as using ptregs.

Undo that part of the syscall rework.  There was no compelling
reason to do it this way.  While I'm at it, load RCX and R11 before
the other regs to be a little friendlier to the CPU, as they will be
the first of the reloaded registers to be used.

Fixes: 1e423bff959e x86/entry/64: ("Migrate the 64-bit syscall slow path to C")
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/entry_64.S | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 9f7bb808035e..70eadb0ea5fa 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -212,7 +212,9 @@ entry_SYSCALL_64_fastpath:
 
 	LOCKDEP_SYS_EXIT
 	TRACE_IRQS_ON		/* user mode is traced as IRQs on */
-	RESTORE_C_REGS
+	movq	RIP(%rsp), %rcx
+	movq	EFLAGS(%rsp), %r11
+	RESTORE_C_REGS_EXCEPT_RCX_R11
 	movq	RSP(%rsp), %rsp
 	USERGS_SYSRET64
 
-- 
2.5.0

[tip:x86/asm] x86/entry/64: Fix fast-path syscall return register state

[tip-bot for Andy Lutomirski]

Commit-ID:  eb2a54c3271cb6443ae93ec44a91687b60c559a3
Gitweb:     http://git.kernel.org/tip/eb2a54c3271cb6443ae93ec44a91687b60c559a3
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Sun, 31 Jan 2016 09:33:27 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 1 Feb 2016 08:53:25 +0100

x86/entry/64: Fix fast-path syscall return register state

I was fishing RIP (i.e. RCX) out of pt_regs->cx and RFLAGS (i.e.
R11) out of pt_regs->r11.  While it usually worked (pt_regs
started out with CX == IP and R11 == FLAGS), it was very
fragile.  In particular, it broke sys_iopl() because sys_iopl()
forgot to mark itself as using ptregs.

Undo that part of the syscall rework.  There was no compelling
reason to do it this way.  While I'm at it, load RCX and R11
before the other regs to be a little friendlier to the CPU, as
they will be the first of the reloaded registers to be used.

Reported-and-tested-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
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: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 1e423bff959e x86/entry/64: ("Migrate the 64-bit syscall slow path to C")
Link: http://lkml.kernel.org/r/a85f8360c397e48186a9bc3e565ad74307a7b011.1454261517.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/entry_64.S | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 9f7bb80..70eadb0 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -212,7 +212,9 @@ entry_SYSCALL_64_fastpath:
 
 	LOCKDEP_SYS_EXIT
 	TRACE_IRQS_ON		/* user mode is traced as IRQs on */
-	RESTORE_C_REGS
+	movq	RIP(%rsp), %rcx
+	movq	EFLAGS(%rsp), %r11
+	RESTORE_C_REGS_EXCEPT_RCX_R11
 	movq	RSP(%rsp), %rsp
 	USERGS_SYSRET64
 

[PATCH 3/3] x86/syscalls/64: Mark sys_iopl as using ptregs

[Andy Lutomirski]

sys_iopl both reads and writes pt_regs->flags.  Mark it as using ptregs.

This isn't strictly necessary, as pt_regs->flags is available even
in the fast path, but this is very lightweight now that we have
syscall qualifiers and it could avoid some pain down the road.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/syscalls/syscall_64.tbl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index dcf107ce2cd4..2e5b565adacc 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -178,7 +178,7 @@
 169	common	reboot			sys_reboot
 170	common	sethostname		sys_sethostname
 171	common	setdomainname		sys_setdomainname
-172	common	iopl			sys_iopl
+172	common	iopl			sys_iopl/ptregs
 173	common	ioperm			sys_ioperm
 174	64	create_module
 175	common	init_module		sys_init_module
-- 
2.5.0

[tip:x86/asm] x86/syscalls/64: Mark sys_iopl() as using ptregs

[tip-bot for Andy Lutomirski]

Commit-ID:  bb56968a37a44070de92d5690c4b08dd98a5d3f1
Gitweb:     http://git.kernel.org/tip/bb56968a37a44070de92d5690c4b08dd98a5d3f1
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Sun, 31 Jan 2016 09:33:28 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 1 Feb 2016 08:53:25 +0100

x86/syscalls/64: Mark sys_iopl() as using ptregs

sys_iopl() both reads and writes pt_regs->flags.  Mark it as using ptregs.

This isn't strictly necessary, as pt_regs->flags is available
even in the fast path, but this is very lightweight now that we
have syscall qualifiers and it could avoid some pain down the
road.

Reported-and-tested-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
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: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/3de0ca692fa8bf414c5e3d7afe3e6195d1a10e1f.1454261517.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/syscalls/syscall_64.tbl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index dcf107c..2e5b565 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -178,7 +178,7 @@
 169	common	reboot			sys_reboot
 170	common	sethostname		sys_sethostname
 171	common	setdomainname		sys_setdomainname
-172	common	iopl			sys_iopl
+172	common	iopl			sys_iopl/ptregs
 173	common	ioperm			sys_ioperm
 174	64	create_module
 175	common	init_module		sys_init_module

Re: [PATCH 0/3] x86/entry/64: Fixes for syscall rework

[Borislav Petkov]

On Sun, Jan 31, 2016 at 09:33:25AM -0800, Andy Lutomirski wrote:
> I broke iopl(2) with my syscall rework.  Fix it up.  While debugging
> it, I found a bug in my IRQ state handling.  Fix that, too.
> 
> Andy Lutomirski (3):
>   x86/entry/64: Fix an IRQ state error on ptregs-using syscalls
>   x86/entry/64: Fix fast-path syscall return register state
>   x86/syscalls/64: Mark sys_iopl as using ptregs
> 
>  arch/x86/entry/entry_64.S              | 23 ++++++++++++++++-------
>  arch/x86/entry/syscalls/syscall_64.tbl |  2 +-
>  2 files changed, 17 insertions(+), 8 deletions(-)

Looks good so far:

Reported-and-tested-by: Borislav Petkov <bp@suse.de>

We better hammer on those a lot more until the next merge window for
more confidence.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH 0/3] x86/entry/64: Fixes for syscall rework

[Ingo Molnar]

* Borislav Petkov <bp@alien8.de> wrote:

> On Sun, Jan 31, 2016 at 09:33:25AM -0800, Andy Lutomirski wrote:
> > I broke iopl(2) with my syscall rework.  Fix it up.  While debugging
> > it, I found a bug in my IRQ state handling.  Fix that, too.
> > 
> > Andy Lutomirski (3):
> >   x86/entry/64: Fix an IRQ state error on ptregs-using syscalls
> >   x86/entry/64: Fix fast-path syscall return register state
> >   x86/syscalls/64: Mark sys_iopl as using ptregs
> > 
> >  arch/x86/entry/entry_64.S              | 23 ++++++++++++++++-------
> >  arch/x86/entry/syscalls/syscall_64.tbl |  2 +-
> >  2 files changed, 17 insertions(+), 8 deletions(-)
> 
> Looks good so far:
> 
> Reported-and-tested-by: Borislav Petkov <bp@suse.de>
> 
> We better hammer on those a lot more until the next merge window for
> more confidence.

Yes, this is why I started merging this right after -rc1.

In particular it would be interesting to get feedback from our 'special' syscall 
ABI users: emulators (Wine, Steam, etc.) and non-GNU toolchains (Android-x86) - at 
least the ones that utilize 64-bit syscalls.

Thanks,

	Ingo