[PATCH 2/2] x86/asm/entry/32: Reorder and explain register restore before SYSRETs.

[Denys Vlasenko]

We do there three different things. This patch adds comments to explain it.

First, we prepare %ecx and %r11 for SYSRET to restore EIP and FLAGS.
These insns need to be scheduled first, since SYSRET depends on them.
Move these loads up.

Second, we restore those registers which userspace expects to not be clobbered.
Here requiremends differ for SYSENTER and SYSCALL code paths:
SYSENTER code path does not preserve %edx and %ebp,
SYSCALL path does.

And third, we clear registers which may still have some data from kernel in them.
While at it, replace XORQ with equivalent XORL: even though for r8+ regs
XORL form is the same length as XORQ (we don't save on REX prefix),
XORL reg,reg is faster than XORQ on Silvermont CPUs.

Signed-off-by: Denys Vlasenko <dvlasenk@xxxxxxxxxx>
CC: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
CC: Steven Rostedt <rostedt@xxxxxxxxxxx>
CC: Ingo Molnar <mingo@xxxxxxxxxx>
CC: Borislav Petkov <bp@xxxxxxxxx>
CC: "H. Peter Anvin" <hpa@xxxxxxxxx>
CC: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
CC: Oleg Nesterov <oleg@xxxxxxxxxx>
CC: Frederic Weisbecker <fweisbec@xxxxxxxxx>
CC: Alexei Starovoitov <ast@xxxxxxxxxxxx>
CC: Will Drewry <wad@xxxxxxxxxxxx>
CC: Kees Cook <keescook@xxxxxxxxxxxx>
CC: x86@xxxxxxxxxx
CC: linux-kernel@xxxxxxxxxxxxxxx
---
 arch/x86/entry/entry_64_compat.S | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index b868cfc..f2064bd 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -139,14 +139,22 @@ sysexit_from_sys_call:
 	 * with 'sysenter' and it uses the SYSENTER calling convention.
 	 */
 	andl	$~TS_COMPAT, ASM_THREAD_INFO(TI_status, %rsp, SIZEOF_PTREGS)
+	/* Prepare registers for SYSRET insn */
 	movl	RIP(%rsp), %ecx		/* User %eip */
+	movl	EFLAGS(%rsp), %r11d	/* User eflags *
+	/* Restore registers per SYSEXIT ABI requirements: */
+	/* arg1 (ebx): preserved by virtue of being a callee-saved register */
+	/* arg2 (ecx): used by SYSEXIT to restore esp (and by SYSRET to restore eip) */
+	/* arg3 (edx): used by SYSEXIT to restore eip */
+	/* arg4,5 (esi,edi): preserved */
 	movl	RSI(%rsp), %esi
 	movl	RDI(%rsp), %edi
-	xorl	%edx, %edx		/* Do not leak kernel information */
-	xorq	%r8, %r8
-	xorq	%r9, %r9
-	xorq	%r10, %r10
-	movl	EFLAGS(%rsp), %r11d	/* User eflags */
+	/* user stack (ebp): clobbered */
+	/* Clear callee-clobbered registers (preventing info leaks) */
+	xorl	%edx, %edx
+	xorl	%r8d, %r8d
+	xorl	%r9d, %r9d
+	xorl	%r10d, %r10d
 	TRACE_IRQS_ON
 
 	/*
@@ -170,8 +178,7 @@ sysexit_from_sys_call:
 	 *  pop %ecx
 	 *
 	 * Therefore, we invoke SYSRETL with EDX and R8-R10 zeroed to
-	 * avoid info leaks.  R11 ends up with VDSO32_SYSENTER_RETURN's
-	 * address (already known to user code), and R12-R15 are
+	 * avoid info leaks.  R11 ends up with EFLAGS, and R12-R15 are
 	 * callee-saved and therefore don't contain any interesting
 	 * kernel data.
 	 */
@@ -367,14 +374,20 @@ cstar_dispatch:
 
 sysretl_from_sys_call:
 	andl	$~TS_COMPAT, ASM_THREAD_INFO(TI_status, %rsp, SIZEOF_PTREGS)
+	/* Prepare registers for SYSRET insn */
+	movl	RIP(%rsp), %ecx		/* User %eip */
+	movl	EFLAGS(%rsp), %r11d	/* User eflags *
+	/* Restore registers per SYSRET ABI requirements: */
+	/* arg1 (ebx): preserved by virtue of being a callee-saved register */
+	/* arg2 (ebp): preserved (already restored, see above) */
+	/* arg3,4,5 (edx,esi,edi): preserved */
 	movl	RDX(%rsp), %edx
 	movl	RSI(%rsp), %esi
 	movl	RDI(%rsp), %edi
-	movl	RIP(%rsp), %ecx
-	movl	EFLAGS(%rsp), %r11d
-	xorq	%r10, %r10
-	xorq	%r9, %r9
-	xorq	%r8, %r8
+	/* Clear callee-clobbered registers (preventing info leaks) */
+	xorl	%r8d, %r8d
+	xorl	%r9d, %r9d
+	xorl	%r10d, %r10d
 	TRACE_IRQS_ON
 	movl	RSP(%rsp), %esp
 	/*
-- 
1.8.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/