[PATCH v3 0/3] x86/entry: Clear registers to sanitize speculative usages

[PATCH v3 0/3] x86/entry: Clear registers to sanitize speculative usages

[Dan Williams]

Changes since v2 [1]:
* Interleave clearing with preserving in the syscall and compat syscall
  path (Linus)

* Extend clearing protection to r10 and r11 in the sycall path and r8 +
  r9 in the compat syscall path. (Ingo)

* Drop some redundant clearing at interrupt / exception entry.

[1]: https://lkml.org/lkml/2018/2/4/125

---

At entry userspace may have populated callee saved registers with values
that could be useful in a speculative execution attack. Clear them to
minimize the kernel's attack surface.

Note, this is done to make it harder to find / manipulate exploitable
sequences in the kernel.

The clearing is limited to the 64-bit 'extra' registers since those are
the most likely to survive with user populated values deep into the call
chain. Normal register pressure likely clobbers values in the lower
registers and the 32-bit case.

As for cycle impact, interleaving the clearing with pushing values onto
the stack hides most the overhead.

---

Andi Kleen (2):
      x86/entry: Clear registers for 64bit exceptions/interrupts
      x86/entry: Clear registers for compat syscalls

Dan Williams (1):
      x86/entry: Clear extra registers beyond syscall arguments for 64bit kernels


 arch/x86/entry/calling.h         |   19 +++++++++++++++++++
 arch/x86/entry/entry_64.S        |   20 ++++++++++++++++++--
 arch/x86/entry/entry_64_compat.S |   30 ++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 2 deletions(-)

[PATCH v3 1/3] x86/entry: Clear extra registers beyond syscall arguments for 64bit kernels

[Dan Williams]

At entry userspace may have populated the extra registers outside the
syscall calling convention with values that could be useful in a
speculative execution attack. Clear them to minimize the kernel's attack
surface. Note, this only clears the extra registers and not the unused
registers for syscalls less than 6 arguments since those registers are
likely to be clobbered well before their values could be put to use
under speculation.

Note, Linus found that the 'xor' instructions can be executed with
minimized cost if interleaved with the 'push' instructions, and Ingo's
analysis found that r10 and r11 should be included in the register
clearing beyond the typical 'extra' syscall calling convention
registers.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Andy Lutomirski <luto@kernel.org>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/x86/entry/entry_64.S |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index c752abe89d80..e8c3a902333d 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -235,13 +235,26 @@ GLOBAL(entry_SYSCALL_64_after_hwframe)
 	pushq	%r8				/* pt_regs->r8 */
 	pushq	%r9				/* pt_regs->r9 */
 	pushq	%r10				/* pt_regs->r10 */
+	/*
+	 * Sanitize extra registers of values that a speculation attack
+	 * might want to exploit. Interleave xor with pushq for better
+	 * uop scheduling.
+	 */
+	xorq	%r10, %r10			/* nospec   r10 */
 	pushq	%r11				/* pt_regs->r11 */
+	xorq	%r11, %r11			/* nospec   r11 */
 	pushq	%rbx				/* pt_regs->rbx */
+	xorl	%ebx, %ebx			/* nospec   rbx */
 	pushq	%rbp				/* pt_regs->rbp */
+	xorl	%ebp, %ebp			/* nospec   rbp */
 	pushq	%r12				/* pt_regs->r12 */
+	xorq	%r12, %r12			/* nospec   r12 */
 	pushq	%r13				/* pt_regs->r13 */
+	xorq	%r13, %r13			/* nospec   r13 */
 	pushq	%r14				/* pt_regs->r14 */
+	xorq	%r14, %r14			/* nospec   r14 */
 	pushq	%r15				/* pt_regs->r15 */
+	xorq	%r15, %r15			/* nospec   r15 */
 	UNWIND_HINT_REGS
 
 	TRACE_IRQS_OFF

[PATCH v3 2/3] x86/entry: Clear registers for 64bit exceptions/interrupts

[Dan Williams]

From: Andi Kleen <ak@linux.intel.com>

Clear the 'extra' registers on entering the 64bit kernel for exceptions
and interrupts. The common registers are not cleared since they are
likely clobbered well before they can be exploited in a speculative
execution attack.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/x86/entry/calling.h  |   19 +++++++++++++++++++
 arch/x86/entry/entry_64.S |    7 +++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index 3f48f695d5e6..5203bb57fb24 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -147,6 +147,25 @@ For 32-bit we have the following conventions - kernel is built with
 	UNWIND_HINT_REGS offset=\offset
 	.endm
 
+	/*
+	 * Sanitize registers of values that a speculation attack
+	 * might want to exploit. The lower registers are likely
+	 * clobbered well before they could be put to use in a
+	 * speculative execution gadget.
+	 */
+	.macro CLEAR_REGS_NOSPEC
+	xorl %ebp, %ebp
+	xorl %ebx, %ebx
+	xorq %r8, %r8
+	xorq %r9, %r9
+	xorq %r10, %r10
+	xorq %r11, %r11
+	xorq %r12, %r12
+	xorq %r13, %r13
+	xorq %r14, %r14
+	xorq %r15, %r15
+	.endm
+
 	.macro POP_EXTRA_REGS
 	popq %r15
 	popq %r14
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index e8c3a902333d..fd88be099b6a 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -575,6 +575,7 @@ END(irq_entries_start)
 	ALLOC_PT_GPREGS_ON_STACK
 	SAVE_C_REGS
 	SAVE_EXTRA_REGS
+	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER
 
 	testb	$3, CS(%rsp)
@@ -1133,6 +1134,7 @@ ENTRY(xen_failsafe_callback)
 	ALLOC_PT_GPREGS_ON_STACK
 	SAVE_C_REGS
 	SAVE_EXTRA_REGS
+	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER
 	jmp	error_exit
 END(xen_failsafe_callback)
@@ -1178,6 +1180,7 @@ ENTRY(paranoid_entry)
 	cld
 	SAVE_C_REGS 8
 	SAVE_EXTRA_REGS 8
+	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER 8
 	movl	$1, %ebx
 	movl	$MSR_GS_BASE, %ecx
@@ -1185,7 +1188,6 @@ ENTRY(paranoid_entry)
 	testl	%edx, %edx
 	js	1f				/* negative -> in kernel */
 	SWAPGS
-	xorl	%ebx, %ebx
 
 1:
 	SAVE_AND_SWITCH_TO_KERNEL_CR3 scratch_reg=%rax save_reg=%r14
@@ -1230,8 +1232,8 @@ ENTRY(error_entry)
 	cld
 	SAVE_C_REGS 8
 	SAVE_EXTRA_REGS 8
+	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER 8
-	xorl	%ebx, %ebx
 	testb	$3, CS+8(%rsp)
 	jz	.Lerror_kernelspace
 
@@ -1428,6 +1430,7 @@ ENTRY(nmi)
 	pushq	%r14		/* pt_regs->r14 */
 	pushq	%r15		/* pt_regs->r15 */
 	UNWIND_HINT_REGS
+	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER
 
 	/*

[PATCH v3 3/3] x86/entry: Clear registers for compat syscalls

[Dan Williams]

From: Andi Kleen <ak@linux.intel.com>

At entry userspace may have populated registers with values that could
be useful in a speculative execution attack. Clear them to minimize the
kernel's attack surface.

[djbw: interleave the clearing with setting up the stack ]
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 arch/x86/entry/entry_64_compat.S |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 98d5358e4041..fd65e016e413 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -85,15 +85,25 @@ ENTRY(entry_SYSENTER_compat)
 	pushq	%rcx			/* pt_regs->cx */
 	pushq	$-ENOSYS		/* pt_regs->ax */
 	pushq   $0			/* pt_regs->r8  = 0 */
+	xorq	%r8, %r8		/* nospec   r8 */
 	pushq   $0			/* pt_regs->r9  = 0 */
+	xorq	%r9, %r9		/* nospec   r9 */
 	pushq   $0			/* pt_regs->r10 = 0 */
+	xorq	%r10, %r10		/* nospec   r10 */
 	pushq   $0			/* pt_regs->r11 = 0 */
+	xorq	%r11, %r11		/* nospec   r11 */
 	pushq   %rbx                    /* pt_regs->rbx */
+	xorl	%ebx, %ebx		/* nospec   rbx */
 	pushq   %rbp                    /* pt_regs->rbp (will be overwritten) */
+	xorl	%ebp, %ebp		/* nospec   rbp */
 	pushq   $0			/* pt_regs->r12 = 0 */
+	xorq	%r12, %r12		/* nospec   r12 */
 	pushq   $0			/* pt_regs->r13 = 0 */
+	xorq	%r13, %r13		/* nospec   r13 */
 	pushq   $0			/* pt_regs->r14 = 0 */
+	xorq	%r14, %r14		/* nospec   r14 */
 	pushq   $0			/* pt_regs->r15 = 0 */
+	xorq	%r15, %r15		/* nospec   r15 */
 	cld
 
 	/*
@@ -214,15 +224,25 @@ GLOBAL(entry_SYSCALL_compat_after_hwframe)
 	pushq	%rbp			/* pt_regs->cx (stashed in bp) */
 	pushq	$-ENOSYS		/* pt_regs->ax */
 	pushq   $0			/* pt_regs->r8  = 0 */
+	xorq	%r8, %r8		/* nospec   r8 */
 	pushq   $0			/* pt_regs->r9  = 0 */
+	xorq	%r9, %r9		/* nospec   r9 */
 	pushq   $0			/* pt_regs->r10 = 0 */
+	xorq	%r10, %r10		/* nospec   r10 */
 	pushq   $0			/* pt_regs->r11 = 0 */
+	xorq	%r11, %r11		/* nospec   r11 */
 	pushq   %rbx                    /* pt_regs->rbx */
+	xorl	%ebx, %ebx		/* nospec   rbx */
 	pushq   %rbp                    /* pt_regs->rbp (will be overwritten) */
+	xorl	%ebp, %ebp		/* nospec   rbp */
 	pushq   $0			/* pt_regs->r12 = 0 */
+	xorq	%r12, %r12		/* nospec   r12 */
 	pushq   $0			/* pt_regs->r13 = 0 */
+	xorq	%r13, %r13		/* nospec   r13 */
 	pushq   $0			/* pt_regs->r14 = 0 */
+	xorq	%r14, %r14		/* nospec   r14 */
 	pushq   $0			/* pt_regs->r15 = 0 */
+	xorq	%r15, %r15		/* nospec   r15 */
 
 	/*
 	 * User mode is traced as though IRQs are on, and SYSENTER
@@ -338,15 +358,25 @@ ENTRY(entry_INT80_compat)
 	pushq	%rcx			/* pt_regs->cx */
 	pushq	$-ENOSYS		/* pt_regs->ax */
 	pushq   $0			/* pt_regs->r8  = 0 */
+	xorq	%r8, %r8		/* nospec   r8 */
 	pushq   $0			/* pt_regs->r9  = 0 */
+	xorq	%r9, %r9		/* nospec   r9 */
 	pushq   $0			/* pt_regs->r10 = 0 */
+	xorq	%r10, %r10		/* nospec   r10 */
 	pushq   $0			/* pt_regs->r11 = 0 */
+	xorq	%r11, %r11		/* nospec   r11 */
 	pushq   %rbx                    /* pt_regs->rbx */
+	xorl	%ebx, %ebx		/* nospec   rbx */
 	pushq   %rbp                    /* pt_regs->rbp */
+	xorl	%ebp, %ebp		/* nospec   rbp */
 	pushq   %r12                    /* pt_regs->r12 */
+	xorq	%r12, %r12		/* nospec   r12 */
 	pushq   %r13                    /* pt_regs->r13 */
+	xorq	%r13, %r13		/* nospec   r13 */
 	pushq   %r14                    /* pt_regs->r14 */
+	xorq	%r14, %r14		/* nospec   r14 */
 	pushq   %r15                    /* pt_regs->r15 */
+	xorq	%r15, %r15		/* nospec   r15 */
 	cld
 
 	/*

Re: [PATCH v3 3/3] x86/entry: Clear registers for compat syscalls

[Ingo Molnar]

* Dan Williams <dan.j.williams@intel.com> wrote:

> From: Andi Kleen <ak@linux.intel.com>
> 
> At entry userspace may have populated registers with values that could
> be useful in a speculative execution attack. Clear them to minimize the
> kernel's attack surface.
> 
> [djbw: interleave the clearing with setting up the stack ]
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  arch/x86/entry/entry_64_compat.S |   30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
> index 98d5358e4041..fd65e016e413 100644
> --- a/arch/x86/entry/entry_64_compat.S
> +++ b/arch/x86/entry/entry_64_compat.S
> @@ -85,15 +85,25 @@ ENTRY(entry_SYSENTER_compat)
>  	pushq	%rcx			/* pt_regs->cx */
>  	pushq	$-ENOSYS		/* pt_regs->ax */
>  	pushq   $0			/* pt_regs->r8  = 0 */
> +	xorq	%r8, %r8		/* nospec   r8 */
>  	pushq   $0			/* pt_regs->r9  = 0 */
> +	xorq	%r9, %r9		/* nospec   r9 */
>  	pushq   $0			/* pt_regs->r10 = 0 */
> +	xorq	%r10, %r10		/* nospec   r10 */
>  	pushq   $0			/* pt_regs->r11 = 0 */
> +	xorq	%r11, %r11		/* nospec   r11 */
>  	pushq   %rbx                    /* pt_regs->rbx */
> +	xorl	%ebx, %ebx		/* nospec   rbx */
>  	pushq   %rbp                    /* pt_regs->rbp (will be overwritten) */
> +	xorl	%ebp, %ebp		/* nospec   rbp */
>  	pushq   $0			/* pt_regs->r12 = 0 */
> +	xorq	%r12, %r12		/* nospec   r12 */
>  	pushq   $0			/* pt_regs->r13 = 0 */
> +	xorq	%r13, %r13		/* nospec   r13 */
>  	pushq   $0			/* pt_regs->r14 = 0 */
> +	xorq	%r14, %r14		/* nospec   r14 */
>  	pushq   $0			/* pt_regs->r15 = 0 */
> +	xorq	%r15, %r15		/* nospec   r15 */
>  	cld
>  
>  	/*
> @@ -214,15 +224,25 @@ GLOBAL(entry_SYSCALL_compat_after_hwframe)
>  	pushq	%rbp			/* pt_regs->cx (stashed in bp) */
>  	pushq	$-ENOSYS		/* pt_regs->ax */
>  	pushq   $0			/* pt_regs->r8  = 0 */
> +	xorq	%r8, %r8		/* nospec   r8 */
>  	pushq   $0			/* pt_regs->r9  = 0 */
> +	xorq	%r9, %r9		/* nospec   r9 */
>  	pushq   $0			/* pt_regs->r10 = 0 */
> +	xorq	%r10, %r10		/* nospec   r10 */
>  	pushq   $0			/* pt_regs->r11 = 0 */
> +	xorq	%r11, %r11		/* nospec   r11 */
>  	pushq   %rbx                    /* pt_regs->rbx */
> +	xorl	%ebx, %ebx		/* nospec   rbx */
>  	pushq   %rbp                    /* pt_regs->rbp (will be overwritten) */
> +	xorl	%ebp, %ebp		/* nospec   rbp */
>  	pushq   $0			/* pt_regs->r12 = 0 */
> +	xorq	%r12, %r12		/* nospec   r12 */
>  	pushq   $0			/* pt_regs->r13 = 0 */
> +	xorq	%r13, %r13		/* nospec   r13 */
>  	pushq   $0			/* pt_regs->r14 = 0 */
> +	xorq	%r14, %r14		/* nospec   r14 */
>  	pushq   $0			/* pt_regs->r15 = 0 */
> +	xorq	%r15, %r15		/* nospec   r15 */

I really love it how you've aligned the comment fields vertically - nice!

>  	/*
>  	 * User mode is traced as though IRQs are on, and SYSENTER
> @@ -338,15 +358,25 @@ ENTRY(entry_INT80_compat)
>  	pushq	%rcx			/* pt_regs->cx */
>  	pushq	$-ENOSYS		/* pt_regs->ax */
>  	pushq   $0			/* pt_regs->r8  = 0 */
> +	xorq	%r8, %r8		/* nospec   r8 */
>  	pushq   $0			/* pt_regs->r9  = 0 */
> +	xorq	%r9, %r9		/* nospec   r9 */
>  	pushq   $0			/* pt_regs->r10 = 0 */
> +	xorq	%r10, %r10		/* nospec   r10 */
>  	pushq   $0			/* pt_regs->r11 = 0 */
> +	xorq	%r11, %r11		/* nospec   r11 */
>  	pushq   %rbx                    /* pt_regs->rbx */
> +	xorl	%ebx, %ebx		/* nospec   rbx */
>  	pushq   %rbp                    /* pt_regs->rbp */
> +	xorl	%ebp, %ebp		/* nospec   rbp */
>  	pushq   %r12                    /* pt_regs->r12 */
> +	xorq	%r12, %r12		/* nospec   r12 */
>  	pushq   %r13                    /* pt_regs->r13 */
> +	xorq	%r13, %r13		/* nospec   r13 */
>  	pushq   %r14                    /* pt_regs->r14 */
> +	xorq	%r14, %r14		/* nospec   r14 */
>  	pushq   %r15                    /* pt_regs->r15 */
> +	xorq	%r15, %r15		/* nospec   r15 */
>  	cld

BTW., these last two patches have changed *significantly* from Andi's original 
patches that were submitted originally, so I changed them over to:

  From: Dan Williams <dan.j.williams@intel.com>
  ...
  Originally-From: Andi Kleen <ak@linux.intel.com>
  Signed-off-by: Dan Williams <dan.j.williams@intel.com>

... to better show authorship history.

Please let me know if that's not OK.

Thanks,

	Ingo

Re: [PATCH v3 3/3] x86/entry: Clear registers for compat syscalls

[Dan Williams]

On Mon, Feb 5, 2018 at 11:26 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Dan Williams <dan.j.williams@intel.com> wrote:
>
>> From: Andi Kleen <ak@linux.intel.com>
>>
>> At entry userspace may have populated registers with values that could
>> be useful in a speculative execution attack. Clear them to minimize the
>> kernel's attack surface.
>>
>> [djbw: interleave the clearing with setting up the stack ]
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: x86@kernel.org
>> Cc: Andy Lutomirski <luto@kernel.org>
>> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>>  arch/x86/entry/entry_64_compat.S |   30 ++++++++++++++++++++++++++++++
>>  1 file changed, 30 insertions(+)
>>
>> diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
>> index 98d5358e4041..fd65e016e413 100644
>> --- a/arch/x86/entry/entry_64_compat.S
>> +++ b/arch/x86/entry/entry_64_compat.S
>> @@ -85,15 +85,25 @@ ENTRY(entry_SYSENTER_compat)
>>       pushq   %rcx                    /* pt_regs->cx */
>>       pushq   $-ENOSYS                /* pt_regs->ax */
>>       pushq   $0                      /* pt_regs->r8  = 0 */
>> +     xorq    %r8, %r8                /* nospec   r8 */
>>       pushq   $0                      /* pt_regs->r9  = 0 */
>> +     xorq    %r9, %r9                /* nospec   r9 */
>>       pushq   $0                      /* pt_regs->r10 = 0 */
>> +     xorq    %r10, %r10              /* nospec   r10 */
>>       pushq   $0                      /* pt_regs->r11 = 0 */
>> +     xorq    %r11, %r11              /* nospec   r11 */
>>       pushq   %rbx                    /* pt_regs->rbx */
>> +     xorl    %ebx, %ebx              /* nospec   rbx */
>>       pushq   %rbp                    /* pt_regs->rbp (will be overwritten) */
>> +     xorl    %ebp, %ebp              /* nospec   rbp */
>>       pushq   $0                      /* pt_regs->r12 = 0 */
>> +     xorq    %r12, %r12              /* nospec   r12 */
>>       pushq   $0                      /* pt_regs->r13 = 0 */
>> +     xorq    %r13, %r13              /* nospec   r13 */
>>       pushq   $0                      /* pt_regs->r14 = 0 */
>> +     xorq    %r14, %r14              /* nospec   r14 */
>>       pushq   $0                      /* pt_regs->r15 = 0 */
>> +     xorq    %r15, %r15              /* nospec   r15 */
>>       cld
>>
>>       /*
>> @@ -214,15 +224,25 @@ GLOBAL(entry_SYSCALL_compat_after_hwframe)
>>       pushq   %rbp                    /* pt_regs->cx (stashed in bp) */
>>       pushq   $-ENOSYS                /* pt_regs->ax */
>>       pushq   $0                      /* pt_regs->r8  = 0 */
>> +     xorq    %r8, %r8                /* nospec   r8 */
>>       pushq   $0                      /* pt_regs->r9  = 0 */
>> +     xorq    %r9, %r9                /* nospec   r9 */
>>       pushq   $0                      /* pt_regs->r10 = 0 */
>> +     xorq    %r10, %r10              /* nospec   r10 */
>>       pushq   $0                      /* pt_regs->r11 = 0 */
>> +     xorq    %r11, %r11              /* nospec   r11 */
>>       pushq   %rbx                    /* pt_regs->rbx */
>> +     xorl    %ebx, %ebx              /* nospec   rbx */
>>       pushq   %rbp                    /* pt_regs->rbp (will be overwritten) */
>> +     xorl    %ebp, %ebp              /* nospec   rbp */
>>       pushq   $0                      /* pt_regs->r12 = 0 */
>> +     xorq    %r12, %r12              /* nospec   r12 */
>>       pushq   $0                      /* pt_regs->r13 = 0 */
>> +     xorq    %r13, %r13              /* nospec   r13 */
>>       pushq   $0                      /* pt_regs->r14 = 0 */
>> +     xorq    %r14, %r14              /* nospec   r14 */
>>       pushq   $0                      /* pt_regs->r15 = 0 */
>> +     xorq    %r15, %r15              /* nospec   r15 */
>
> I really love it how you've aligned the comment fields vertically - nice!
>
>>       /*
>>        * User mode is traced as though IRQs are on, and SYSENTER
>> @@ -338,15 +358,25 @@ ENTRY(entry_INT80_compat)
>>       pushq   %rcx                    /* pt_regs->cx */
>>       pushq   $-ENOSYS                /* pt_regs->ax */
>>       pushq   $0                      /* pt_regs->r8  = 0 */
>> +     xorq    %r8, %r8                /* nospec   r8 */
>>       pushq   $0                      /* pt_regs->r9  = 0 */
>> +     xorq    %r9, %r9                /* nospec   r9 */
>>       pushq   $0                      /* pt_regs->r10 = 0 */
>> +     xorq    %r10, %r10              /* nospec   r10 */
>>       pushq   $0                      /* pt_regs->r11 = 0 */
>> +     xorq    %r11, %r11              /* nospec   r11 */
>>       pushq   %rbx                    /* pt_regs->rbx */
>> +     xorl    %ebx, %ebx              /* nospec   rbx */
>>       pushq   %rbp                    /* pt_regs->rbp */
>> +     xorl    %ebp, %ebp              /* nospec   rbp */
>>       pushq   %r12                    /* pt_regs->r12 */
>> +     xorq    %r12, %r12              /* nospec   r12 */
>>       pushq   %r13                    /* pt_regs->r13 */
>> +     xorq    %r13, %r13              /* nospec   r13 */
>>       pushq   %r14                    /* pt_regs->r14 */
>> +     xorq    %r14, %r14              /* nospec   r14 */
>>       pushq   %r15                    /* pt_regs->r15 */
>> +     xorq    %r15, %r15              /* nospec   r15 */
>>       cld
>
> BTW., these last two patches have changed *significantly* from Andi's original
> patches that were submitted originally, so I changed them over to:
>
>   From: Dan Williams <dan.j.williams@intel.com>
>   ...
>   Originally-From: Andi Kleen <ak@linux.intel.com>
>   Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>
> ... to better show authorship history.
>
> Please let me know if that's not OK.

Works for me.

Thanks Ingo.

Re: [PATCH v3 2/3] x86/entry: Clear registers for 64bit exceptions/interrupts

[Dominik Brodowski]

On Mon, Feb 05, 2018 at 05:18:11PM -0800, Dan Williams wrote:
> @@ -1178,6 +1180,7 @@ ENTRY(paranoid_entry)
>  	cld
>  	SAVE_C_REGS 8
>  	SAVE_EXTRA_REGS 8
> +	CLEAR_REGS_NOSPEC
>  	ENCODE_FRAME_POINTER 8
>  	movl	$1, %ebx
>  	movl	$MSR_GS_BASE, %ecx
> @@ -1185,7 +1188,6 @@ ENTRY(paranoid_entry)
>  	testl	%edx, %edx
>  	js	1f				/* negative -> in kernel */
>  	SWAPGS
> -	xorl	%ebx, %ebx

Here, %ebx will be filled with $1 (see code snipped above) *after* the
call to CLEAR_REGS_NOSPEC. That's what this line has been clearing in the
past. So I'm not sure whether this line should be removed.

Thanks,
	Dominik

Re: [PATCH v3 2/3] x86/entry: Clear registers for 64bit exceptions/interrupts

[Dominik Brodowski]

On Mon, Feb 05, 2018 at 05:18:11PM -0800, Dan Williams wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Clear the 'extra' registers on entering the 64bit kernel for exceptions
> and interrupts. The common registers are not cleared since they are
> likely clobbered well before they can be exploited in a speculative
> execution attack.

Could the clever trick from patch 3/3 (interleave xorq with movq) be
used here as well? Something like below (untested)? This includes removing
the (seemingly?) unused SAVE_C_REGS_EXCEPT_* macros, which probably needs to
become a spearate patch.

Thanks,
	Dominik


diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index 3f48f695d5e6..7bdee6d14f0a 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -101,49 +101,36 @@ For 32-bit we have the following conventions - kernel is built with
 	addq	$-(15*8), %rsp
 	.endm
 
-	.macro SAVE_C_REGS_HELPER offset=0 rax=1 rcx=1 r8910=1 r11=1
-	.if \r11
+	.macro SAVE_C_REGS offset=0
+	xorq %r11, %r11				/* nospec r11 */
 	movq %r11, 6*8+\offset(%rsp)
-	.endif
-	.if \r8910
 	movq %r10, 7*8+\offset(%rsp)
+	xorq %r10, %r10				/* nospec r10 */
 	movq %r9,  8*8+\offset(%rsp)
+	xorq %r9, %r9				/* nospec r9 */
 	movq %r8,  9*8+\offset(%rsp)
-	.endif
-	.if \rax
+	xorq %r8, %r8				/* nospec r8 */
 	movq %rax, 10*8+\offset(%rsp)
-	.endif
-	.if \rcx
 	movq %rcx, 11*8+\offset(%rsp)
-	.endif
 	movq %rdx, 12*8+\offset(%rsp)
 	movq %rsi, 13*8+\offset(%rsp)
 	movq %rdi, 14*8+\offset(%rsp)
 	UNWIND_HINT_REGS offset=\offset extra=0
 	.endm
-	.macro SAVE_C_REGS offset=0
-	SAVE_C_REGS_HELPER \offset, 1, 1, 1, 1
-	.endm
-	.macro SAVE_C_REGS_EXCEPT_RAX_RCX offset=0
-	SAVE_C_REGS_HELPER \offset, 0, 0, 1, 1
-	.endm
-	.macro SAVE_C_REGS_EXCEPT_R891011
-	SAVE_C_REGS_HELPER 0, 1, 1, 0, 0
-	.endm
-	.macro SAVE_C_REGS_EXCEPT_RCX_R891011
-	SAVE_C_REGS_HELPER 0, 1, 0, 0, 0
-	.endm
-	.macro SAVE_C_REGS_EXCEPT_RAX_RCX_R11
-	SAVE_C_REGS_HELPER 0, 0, 0, 1, 0
-	.endm
 
 	.macro SAVE_EXTRA_REGS offset=0
 	movq %r15, 0*8+\offset(%rsp)
+	xorq %r15, %r15				/* nospec r15 */
 	movq %r14, 1*8+\offset(%rsp)
+	xorq %r14, %r14				/* nospec r14 */
 	movq %r13, 2*8+\offset(%rsp)
+	xorq %r13, %r13				/* nospec r13 */
 	movq %r12, 3*8+\offset(%rsp)
+	xorq %r12, %r12				/* nospec r12*/
 	movq %rbp, 4*8+\offset(%rsp)
+	xorl %ebp, %ebp				/* nospec rbp */
 	movq %rbx, 5*8+\offset(%rsp)
+	xorl %ebx, %ebx				/* nospec rbx */
 	UNWIND_HINT_REGS offset=\offset
 	.endm
 
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index c752abe89d80..de19a46f40b2 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1218,7 +1218,6 @@ ENTRY(error_entry)
 	SAVE_C_REGS 8
 	SAVE_EXTRA_REGS 8
 	ENCODE_FRAME_POINTER 8
-	xorl	%ebx, %ebx
 	testb	$3, CS+8(%rsp)
 	jz	.Lerror_kernelspace
 
@@ -1405,15 +1404,25 @@ ENTRY(nmi)
 	pushq   %rcx		/* pt_regs->cx */
 	pushq   %rax		/* pt_regs->ax */
 	pushq   %r8		/* pt_regs->r8 */
+	xorq    %r8, %r8	/* nospec   r8 */
 	pushq   %r9		/* pt_regs->r9 */
+	xorq    %r9, %r9	/* nospec   r9 */
 	pushq   %r10		/* pt_regs->r10 */
+	xorq    %r10, %r10	/* nospec   r10 */
 	pushq   %r11		/* pt_regs->r11 */
+	xorq    %r11, %r11	/* nospec   r11*/
 	pushq	%rbx		/* pt_regs->rbx */
+	xorl    %ebx, %ebx	/* nospec   rbx*/
 	pushq	%rbp		/* pt_regs->rbp */
+	xorl    %ebp, %ebp	/* nospec   rbp*/
 	pushq	%r12		/* pt_regs->r12 */
+	xorq    %r12, %r12	/* nospec   r12*/
 	pushq	%r13		/* pt_regs->r13 */
+	xorq    %r13, %r13	/* nospec   r13*/
 	pushq	%r14		/* pt_regs->r14 */
+	xorq    %r14, %r14	/* nospec   r14*/
 	pushq	%r15		/* pt_regs->r15 */
+	xorq    %r15, %r15	/* nospec   r15*/
 	UNWIND_HINT_REGS
 	ENCODE_FRAME_POINTER
 

Re: [PATCH v3 2/3] x86/entry: Clear registers for 64bit exceptions/interrupts

[Ingo Molnar]

* Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> On Mon, Feb 05, 2018 at 05:18:11PM -0800, Dan Williams wrote:
> > @@ -1178,6 +1180,7 @@ ENTRY(paranoid_entry)
> >  	cld
> >  	SAVE_C_REGS 8
> >  	SAVE_EXTRA_REGS 8
> > +	CLEAR_REGS_NOSPEC
> >  	ENCODE_FRAME_POINTER 8
> >  	movl	$1, %ebx
> >  	movl	$MSR_GS_BASE, %ecx
> > @@ -1185,7 +1188,6 @@ ENTRY(paranoid_entry)
> >  	testl	%edx, %edx
> >  	js	1f				/* negative -> in kernel */
> >  	SWAPGS
> > -	xorl	%ebx, %ebx
> 
> Here, %ebx will be filled with $1 (see code snipped above) *after* the
> call to CLEAR_REGS_NOSPEC. That's what this line has been clearing in the
> past. So I'm not sure whether this line should be removed.

Good point - I have fixed this bug in the tip:x86/pti version of the patch.

Thanks,

	Ingo

Re: [PATCH v3 2/3] x86/entry: Clear registers for 64bit exceptions/interrupts

[Ingo Molnar]

* Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> On Mon, Feb 05, 2018 at 05:18:11PM -0800, Dan Williams wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > Clear the 'extra' registers on entering the 64bit kernel for exceptions
> > and interrupts. The common registers are not cleared since they are
> > likely clobbered well before they can be exploited in a speculative
> > execution attack.
> 
> Could the clever trick from patch 3/3 (interleave xorq with movq) be
> used here as well? Something like below (untested)? This includes removing
> the (seemingly?) unused SAVE_C_REGS_EXCEPT_* macros, which probably needs to
> become a spearate patch.
> 
> Thanks,
> 	Dominik
> 
> 
> diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
> index 3f48f695d5e6..7bdee6d14f0a 100644
> --- a/arch/x86/entry/calling.h
> +++ b/arch/x86/entry/calling.h
> @@ -101,49 +101,36 @@ For 32-bit we have the following conventions - kernel is built with
>  	addq	$-(15*8), %rsp
>  	.endm
>  
> -	.macro SAVE_C_REGS_HELPER offset=0 rax=1 rcx=1 r8910=1 r11=1
> -	.if \r11
> +	.macro SAVE_C_REGS offset=0
> +	xorq %r11, %r11				/* nospec r11 */
>  	movq %r11, 6*8+\offset(%rsp)
> -	.endif
> -	.if \r8910
>  	movq %r10, 7*8+\offset(%rsp)
> +	xorq %r10, %r10				/* nospec r10 */
>  	movq %r9,  8*8+\offset(%rsp)
> +	xorq %r9, %r9				/* nospec r9 */
>  	movq %r8,  9*8+\offset(%rsp)
> -	.endif
> -	.if \rax
> +	xorq %r8, %r8				/* nospec r8 */
>  	movq %rax, 10*8+\offset(%rsp)
> -	.endif
> -	.if \rcx
>  	movq %rcx, 11*8+\offset(%rsp)
> -	.endif
>  	movq %rdx, 12*8+\offset(%rsp)
>  	movq %rsi, 13*8+\offset(%rsp)
>  	movq %rdi, 14*8+\offset(%rsp)
>  	UNWIND_HINT_REGS offset=\offset extra=0
>  	.endm
> -	.macro SAVE_C_REGS offset=0
> -	SAVE_C_REGS_HELPER \offset, 1, 1, 1, 1
> -	.endm
> -	.macro SAVE_C_REGS_EXCEPT_RAX_RCX offset=0
> -	SAVE_C_REGS_HELPER \offset, 0, 0, 1, 1
> -	.endm
> -	.macro SAVE_C_REGS_EXCEPT_R891011
> -	SAVE_C_REGS_HELPER 0, 1, 1, 0, 0
> -	.endm
> -	.macro SAVE_C_REGS_EXCEPT_RCX_R891011
> -	SAVE_C_REGS_HELPER 0, 1, 0, 0, 0
> -	.endm
> -	.macro SAVE_C_REGS_EXCEPT_RAX_RCX_R11
> -	SAVE_C_REGS_HELPER 0, 0, 0, 1, 0
> -	.endm
>  
>  	.macro SAVE_EXTRA_REGS offset=0
>  	movq %r15, 0*8+\offset(%rsp)
> +	xorq %r15, %r15				/* nospec r15 */
>  	movq %r14, 1*8+\offset(%rsp)
> +	xorq %r14, %r14				/* nospec r14 */
>  	movq %r13, 2*8+\offset(%rsp)
> +	xorq %r13, %r13				/* nospec r13 */
>  	movq %r12, 3*8+\offset(%rsp)
> +	xorq %r12, %r12				/* nospec r12*/
>  	movq %rbp, 4*8+\offset(%rsp)
> +	xorl %ebp, %ebp				/* nospec rbp */
>  	movq %rbx, 5*8+\offset(%rsp)
> +	xorl %ebx, %ebx				/* nospec rbx */
>  	UNWIND_HINT_REGS offset=\offset
>  	.endm
>  
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index c752abe89d80..de19a46f40b2 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -1218,7 +1218,6 @@ ENTRY(error_entry)
>  	SAVE_C_REGS 8
>  	SAVE_EXTRA_REGS 8
>  	ENCODE_FRAME_POINTER 8
> -	xorl	%ebx, %ebx
>  	testb	$3, CS+8(%rsp)
>  	jz	.Lerror_kernelspace
>  
> @@ -1405,15 +1404,25 @@ ENTRY(nmi)
>  	pushq   %rcx		/* pt_regs->cx */
>  	pushq   %rax		/* pt_regs->ax */
>  	pushq   %r8		/* pt_regs->r8 */
> +	xorq    %r8, %r8	/* nospec   r8 */
>  	pushq   %r9		/* pt_regs->r9 */
> +	xorq    %r9, %r9	/* nospec   r9 */
>  	pushq   %r10		/* pt_regs->r10 */
> +	xorq    %r10, %r10	/* nospec   r10 */
>  	pushq   %r11		/* pt_regs->r11 */
> +	xorq    %r11, %r11	/* nospec   r11*/
>  	pushq	%rbx		/* pt_regs->rbx */
> +	xorl    %ebx, %ebx	/* nospec   rbx*/
>  	pushq	%rbp		/* pt_regs->rbp */
> +	xorl    %ebp, %ebp	/* nospec   rbp*/
>  	pushq	%r12		/* pt_regs->r12 */
> +	xorq    %r12, %r12	/* nospec   r12*/
>  	pushq	%r13		/* pt_regs->r13 */
> +	xorq    %r13, %r13	/* nospec   r13*/
>  	pushq	%r14		/* pt_regs->r14 */
> +	xorq    %r14, %r14	/* nospec   r14*/
>  	pushq	%r15		/* pt_regs->r15 */
> +	xorq    %r15, %r15	/* nospec   r15*/
>  	UNWIND_HINT_REGS
>  	ENCODE_FRAME_POINTER

Makes sense and it's also more readable - could you send this patch on top of 
tip:x86/pti or tip:master, once I've pushed out the latest version (within the 
next few hours)?

Thanks,

	Ingo

Re: [PATCH v3 2/3] x86/entry: Clear registers for 64bit exceptions/interrupts

[Dominik Brodowski]

On Tue, Feb 06, 2018 at 11:51:39AM +0100, Ingo Molnar wrote:
> 
> * Dominik Brodowski <linux@dominikbrodowski.net> wrote:
> 
> > On Mon, Feb 05, 2018 at 05:18:11PM -0800, Dan Williams wrote:
> > > From: Andi Kleen <ak@linux.intel.com>
> > > 
> > > Clear the 'extra' registers on entering the 64bit kernel for exceptions
> > > and interrupts. The common registers are not cleared since they are
> > > likely clobbered well before they can be exploited in a speculative
> > > execution attack.
> > 
> > Could the clever trick from patch 3/3 (interleave xorq with movq) be
> > used here as well? Something like below (untested)? This includes removing
> > the (seemingly?) unused SAVE_C_REGS_EXCEPT_* macros, which probably needs to
> > become a spearate patch.
> > 
> > Thanks,
> > 	Dominik
> > 
> > 
> > diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
> > index 3f48f695d5e6..7bdee6d14f0a 100644
> > --- a/arch/x86/entry/calling.h
> > +++ b/arch/x86/entry/calling.h
> > @@ -101,49 +101,36 @@ For 32-bit we have the following conventions - kernel is built with
> >  	addq	$-(15*8), %rsp
> >  	.endm
> >  
> > -	.macro SAVE_C_REGS_HELPER offset=0 rax=1 rcx=1 r8910=1 r11=1
> > -	.if \r11
> > +	.macro SAVE_C_REGS offset=0
> > +	xorq %r11, %r11				/* nospec r11 */
> >  	movq %r11, 6*8+\offset(%rsp)
> > -	.endif
> > -	.if \r8910
> >  	movq %r10, 7*8+\offset(%rsp)
> > +	xorq %r10, %r10				/* nospec r10 */
> >  	movq %r9,  8*8+\offset(%rsp)
> > +	xorq %r9, %r9				/* nospec r9 */
> >  	movq %r8,  9*8+\offset(%rsp)
> > -	.endif
> > -	.if \rax
> > +	xorq %r8, %r8				/* nospec r8 */
> >  	movq %rax, 10*8+\offset(%rsp)
> > -	.endif
> > -	.if \rcx
> >  	movq %rcx, 11*8+\offset(%rsp)
> > -	.endif
> >  	movq %rdx, 12*8+\offset(%rsp)
> >  	movq %rsi, 13*8+\offset(%rsp)
> >  	movq %rdi, 14*8+\offset(%rsp)
> >  	UNWIND_HINT_REGS offset=\offset extra=0
> >  	.endm
> > -	.macro SAVE_C_REGS offset=0
> > -	SAVE_C_REGS_HELPER \offset, 1, 1, 1, 1
> > -	.endm
> > -	.macro SAVE_C_REGS_EXCEPT_RAX_RCX offset=0
> > -	SAVE_C_REGS_HELPER \offset, 0, 0, 1, 1
> > -	.endm
> > -	.macro SAVE_C_REGS_EXCEPT_R891011
> > -	SAVE_C_REGS_HELPER 0, 1, 1, 0, 0
> > -	.endm
> > -	.macro SAVE_C_REGS_EXCEPT_RCX_R891011
> > -	SAVE_C_REGS_HELPER 0, 1, 0, 0, 0
> > -	.endm
> > -	.macro SAVE_C_REGS_EXCEPT_RAX_RCX_R11
> > -	SAVE_C_REGS_HELPER 0, 0, 0, 1, 0
> > -	.endm
> >  
> >  	.macro SAVE_EXTRA_REGS offset=0
> >  	movq %r15, 0*8+\offset(%rsp)
> > +	xorq %r15, %r15				/* nospec r15 */
> >  	movq %r14, 1*8+\offset(%rsp)
> > +	xorq %r14, %r14				/* nospec r14 */
> >  	movq %r13, 2*8+\offset(%rsp)
> > +	xorq %r13, %r13				/* nospec r13 */
> >  	movq %r12, 3*8+\offset(%rsp)
> > +	xorq %r12, %r12				/* nospec r12*/
> >  	movq %rbp, 4*8+\offset(%rsp)
> > +	xorl %ebp, %ebp				/* nospec rbp */
> >  	movq %rbx, 5*8+\offset(%rsp)
> > +	xorl %ebx, %ebx				/* nospec rbx */
> >  	UNWIND_HINT_REGS offset=\offset
> >  	.endm
> >  
> > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> > index c752abe89d80..de19a46f40b2 100644
> > --- a/arch/x86/entry/entry_64.S
> > +++ b/arch/x86/entry/entry_64.S
> > @@ -1218,7 +1218,6 @@ ENTRY(error_entry)
> >  	SAVE_C_REGS 8
> >  	SAVE_EXTRA_REGS 8
> >  	ENCODE_FRAME_POINTER 8
> > -	xorl	%ebx, %ebx
> >  	testb	$3, CS+8(%rsp)
> >  	jz	.Lerror_kernelspace
> >  
> > @@ -1405,15 +1404,25 @@ ENTRY(nmi)
> >  	pushq   %rcx		/* pt_regs->cx */
> >  	pushq   %rax		/* pt_regs->ax */
> >  	pushq   %r8		/* pt_regs->r8 */
> > +	xorq    %r8, %r8	/* nospec   r8 */
> >  	pushq   %r9		/* pt_regs->r9 */
> > +	xorq    %r9, %r9	/* nospec   r9 */
> >  	pushq   %r10		/* pt_regs->r10 */
> > +	xorq    %r10, %r10	/* nospec   r10 */
> >  	pushq   %r11		/* pt_regs->r11 */
> > +	xorq    %r11, %r11	/* nospec   r11*/
> >  	pushq	%rbx		/* pt_regs->rbx */
> > +	xorl    %ebx, %ebx	/* nospec   rbx*/
> >  	pushq	%rbp		/* pt_regs->rbp */
> > +	xorl    %ebp, %ebp	/* nospec   rbp*/
> >  	pushq	%r12		/* pt_regs->r12 */
> > +	xorq    %r12, %r12	/* nospec   r12*/
> >  	pushq	%r13		/* pt_regs->r13 */
> > +	xorq    %r13, %r13	/* nospec   r13*/
> >  	pushq	%r14		/* pt_regs->r14 */
> > +	xorq    %r14, %r14	/* nospec   r14*/
> >  	pushq	%r15		/* pt_regs->r15 */
> > +	xorq    %r15, %r15	/* nospec   r15*/
> >  	UNWIND_HINT_REGS
> >  	ENCODE_FRAME_POINTER
> 
> Makes sense and it's also more readable - could you send this patch on top of 
> tip:x86/pti or tip:master, once I've pushed out the latest version (within the 
> next few hours)?

Will do. Thanks,

	Dominik

[tip:x86/pti] x86/entry/64: Clear extra registers beyond syscall arguments, to reduce speculation attack surface

[tip-bot for Dan Williams]

Commit-ID:  8e1eb3fa009aa7c0b944b3c8b26b07de0efb3200
Gitweb:     https://git.kernel.org/tip/8e1eb3fa009aa7c0b944b3c8b26b07de0efb3200
Author:     Dan Williams <dan.j.williams@intel.com>
AuthorDate: Mon, 5 Feb 2018 17:18:05 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 6 Feb 2018 08:30:27 +0100

x86/entry/64: Clear extra registers beyond syscall arguments, to reduce speculation attack surface

At entry userspace may have (maliciously) populated the extra registers
outside the syscall calling convention with arbitrary values that could
be useful in a speculative execution (Spectre style) attack.

Clear these registers to minimize the kernel's attack surface.

Note, this only clears the extra registers and not the unused
registers for syscalls less than 6 arguments, since those registers are
likely to be clobbered well before their values could be put to use
under speculation.

Note, Linus found that the XOR instructions can be executed with
minimized cost if interleaved with the PUSH instructions, and Ingo's
analysis found that R10 and R11 should be included in the register
clearing beyond the typical 'extra' syscall calling convention
registers.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Cc: <stable@vger.kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
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: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/151787988577.7847.16733592218894189003.stgit@dwillia2-desk3.amr.corp.intel.com
[ Made small improvements to the changelog and the code comments. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/entry_64.S | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index c752abe..065a71b 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -235,13 +235,26 @@ GLOBAL(entry_SYSCALL_64_after_hwframe)
 	pushq	%r8				/* pt_regs->r8 */
 	pushq	%r9				/* pt_regs->r9 */
 	pushq	%r10				/* pt_regs->r10 */
+	/*
+	 * Clear extra registers that a speculation attack might
+	 * otherwise want to exploit. Interleave XOR with PUSH
+	 * for better uop scheduling:
+	 */
+	xorq	%r10, %r10			/* nospec   r10 */
 	pushq	%r11				/* pt_regs->r11 */
+	xorq	%r11, %r11			/* nospec   r11 */
 	pushq	%rbx				/* pt_regs->rbx */
+	xorl	%ebx, %ebx			/* nospec   rbx */
 	pushq	%rbp				/* pt_regs->rbp */
+	xorl	%ebp, %ebp			/* nospec   rbp */
 	pushq	%r12				/* pt_regs->r12 */
+	xorq	%r12, %r12			/* nospec   r12 */
 	pushq	%r13				/* pt_regs->r13 */
+	xorq	%r13, %r13			/* nospec   r13 */
 	pushq	%r14				/* pt_regs->r14 */
+	xorq	%r14, %r14			/* nospec   r14 */
 	pushq	%r15				/* pt_regs->r15 */
+	xorq	%r15, %r15			/* nospec   r15 */
 	UNWIND_HINT_REGS
 
 	TRACE_IRQS_OFF

[tip:x86/pti] x86/entry/64: Clear registers for exceptions/interrupts, to reduce speculation attack surface

[tip-bot for Dan Williams]

Commit-ID:  3ac6d8c787b835b997eb23e43e09aa0895ef7d58
Gitweb:     https://git.kernel.org/tip/3ac6d8c787b835b997eb23e43e09aa0895ef7d58
Author:     Dan Williams <dan.j.williams@intel.com>
AuthorDate: Mon, 5 Feb 2018 17:18:11 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 6 Feb 2018 11:47:44 +0100

x86/entry/64: Clear registers for exceptions/interrupts, to reduce speculation attack surface

Clear the 'extra' registers on entering the 64-bit kernel for exceptions
and interrupts. The common registers are not cleared since they are
likely clobbered well before they can be exploited in a speculative
execution attack.

Originally-From: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Cc: <stable@vger.kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
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/151787989146.7847.15749181712358213254.stgit@dwillia2-desk3.amr.corp.intel.com
[ Made small improvements to the changelog and the code comments. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/calling.h  | 19 +++++++++++++++++++
 arch/x86/entry/entry_64.S |  6 +++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index 3f48f69..f4b129d 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -147,6 +147,25 @@ For 32-bit we have the following conventions - kernel is built with
 	UNWIND_HINT_REGS offset=\offset
 	.endm
 
+	/*
+	 * Sanitize registers of values that a speculation attack
+	 * might otherwise want to exploit. The lower registers are
+	 * likely clobbered well before they could be put to use in
+	 * a speculative execution gadget:
+	 */
+	.macro CLEAR_REGS_NOSPEC
+	xorl %ebp, %ebp
+	xorl %ebx, %ebx
+	xorq %r8, %r8
+	xorq %r9, %r9
+	xorq %r10, %r10
+	xorq %r11, %r11
+	xorq %r12, %r12
+	xorq %r13, %r13
+	xorq %r14, %r14
+	xorq %r15, %r15
+	.endm
+
 	.macro POP_EXTRA_REGS
 	popq %r15
 	popq %r14
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 065a71b..9e48002 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -575,6 +575,7 @@ END(irq_entries_start)
 	ALLOC_PT_GPREGS_ON_STACK
 	SAVE_C_REGS
 	SAVE_EXTRA_REGS
+	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER
 
 	testb	$3, CS(%rsp)
@@ -1133,6 +1134,7 @@ ENTRY(xen_failsafe_callback)
 	ALLOC_PT_GPREGS_ON_STACK
 	SAVE_C_REGS
 	SAVE_EXTRA_REGS
+	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER
 	jmp	error_exit
 END(xen_failsafe_callback)
@@ -1178,6 +1180,7 @@ ENTRY(paranoid_entry)
 	cld
 	SAVE_C_REGS 8
 	SAVE_EXTRA_REGS 8
+	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER 8
 	movl	$1, %ebx
 	movl	$MSR_GS_BASE, %ecx
@@ -1230,8 +1233,8 @@ ENTRY(error_entry)
 	cld
 	SAVE_C_REGS 8
 	SAVE_EXTRA_REGS 8
+	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER 8
-	xorl	%ebx, %ebx
 	testb	$3, CS+8(%rsp)
 	jz	.Lerror_kernelspace
 
@@ -1428,6 +1431,7 @@ ENTRY(nmi)
 	pushq	%r14		/* pt_regs->r14 */
 	pushq	%r15		/* pt_regs->r15 */
 	UNWIND_HINT_REGS
+	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER
 
 	/*

[tip:x86/pti] x86/entry/64/compat: Clear registers for compat syscalls, to reduce speculation attack surface

[tip-bot for Dan Williams]

Commit-ID:  6b8cf5cc9965673951f1ab3f0e3cf23d06e3e2ee
Gitweb:     https://git.kernel.org/tip/6b8cf5cc9965673951f1ab3f0e3cf23d06e3e2ee
Author:     Dan Williams <dan.j.williams@intel.com>
AuthorDate: Mon, 5 Feb 2018 17:18:17 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 6 Feb 2018 11:47:57 +0100

x86/entry/64/compat: Clear registers for compat syscalls, to reduce speculation attack surface

At entry userspace may have populated registers with values that could
otherwise be useful in a speculative execution attack. Clear them to
minimize the kernel's attack surface.

Originally-From: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Cc: <stable@vger.kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
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/151787989697.7847.4083702787288600552.stgit@dwillia2-desk3.amr.corp.intel.com
[ Made small improvements to the changelog. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/entry_64_compat.S | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 98d5358..fd65e01 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -85,15 +85,25 @@ ENTRY(entry_SYSENTER_compat)
 	pushq	%rcx			/* pt_regs->cx */
 	pushq	$-ENOSYS		/* pt_regs->ax */
 	pushq   $0			/* pt_regs->r8  = 0 */
+	xorq	%r8, %r8		/* nospec   r8 */
 	pushq   $0			/* pt_regs->r9  = 0 */
+	xorq	%r9, %r9		/* nospec   r9 */
 	pushq   $0			/* pt_regs->r10 = 0 */
+	xorq	%r10, %r10		/* nospec   r10 */
 	pushq   $0			/* pt_regs->r11 = 0 */
+	xorq	%r11, %r11		/* nospec   r11 */
 	pushq   %rbx                    /* pt_regs->rbx */
+	xorl	%ebx, %ebx		/* nospec   rbx */
 	pushq   %rbp                    /* pt_regs->rbp (will be overwritten) */
+	xorl	%ebp, %ebp		/* nospec   rbp */
 	pushq   $0			/* pt_regs->r12 = 0 */
+	xorq	%r12, %r12		/* nospec   r12 */
 	pushq   $0			/* pt_regs->r13 = 0 */
+	xorq	%r13, %r13		/* nospec   r13 */
 	pushq   $0			/* pt_regs->r14 = 0 */
+	xorq	%r14, %r14		/* nospec   r14 */
 	pushq   $0			/* pt_regs->r15 = 0 */
+	xorq	%r15, %r15		/* nospec   r15 */
 	cld
 
 	/*
@@ -214,15 +224,25 @@ GLOBAL(entry_SYSCALL_compat_after_hwframe)
 	pushq	%rbp			/* pt_regs->cx (stashed in bp) */
 	pushq	$-ENOSYS		/* pt_regs->ax */
 	pushq   $0			/* pt_regs->r8  = 0 */
+	xorq	%r8, %r8		/* nospec   r8 */
 	pushq   $0			/* pt_regs->r9  = 0 */
+	xorq	%r9, %r9		/* nospec   r9 */
 	pushq   $0			/* pt_regs->r10 = 0 */
+	xorq	%r10, %r10		/* nospec   r10 */
 	pushq   $0			/* pt_regs->r11 = 0 */
+	xorq	%r11, %r11		/* nospec   r11 */
 	pushq   %rbx                    /* pt_regs->rbx */
+	xorl	%ebx, %ebx		/* nospec   rbx */
 	pushq   %rbp                    /* pt_regs->rbp (will be overwritten) */
+	xorl	%ebp, %ebp		/* nospec   rbp */
 	pushq   $0			/* pt_regs->r12 = 0 */
+	xorq	%r12, %r12		/* nospec   r12 */
 	pushq   $0			/* pt_regs->r13 = 0 */
+	xorq	%r13, %r13		/* nospec   r13 */
 	pushq   $0			/* pt_regs->r14 = 0 */
+	xorq	%r14, %r14		/* nospec   r14 */
 	pushq   $0			/* pt_regs->r15 = 0 */
+	xorq	%r15, %r15		/* nospec   r15 */
 
 	/*
 	 * User mode is traced as though IRQs are on, and SYSENTER
@@ -338,15 +358,25 @@ ENTRY(entry_INT80_compat)
 	pushq	%rcx			/* pt_regs->cx */
 	pushq	$-ENOSYS		/* pt_regs->ax */
 	pushq   $0			/* pt_regs->r8  = 0 */
+	xorq	%r8, %r8		/* nospec   r8 */
 	pushq   $0			/* pt_regs->r9  = 0 */
+	xorq	%r9, %r9		/* nospec   r9 */
 	pushq   $0			/* pt_regs->r10 = 0 */
+	xorq	%r10, %r10		/* nospec   r10 */
 	pushq   $0			/* pt_regs->r11 = 0 */
+	xorq	%r11, %r11		/* nospec   r11 */
 	pushq   %rbx                    /* pt_regs->rbx */
+	xorl	%ebx, %ebx		/* nospec   rbx */
 	pushq   %rbp                    /* pt_regs->rbp */
+	xorl	%ebp, %ebp		/* nospec   rbp */
 	pushq   %r12                    /* pt_regs->r12 */
+	xorq	%r12, %r12		/* nospec   r12 */
 	pushq   %r13                    /* pt_regs->r13 */
+	xorq	%r13, %r13		/* nospec   r13 */
 	pushq   %r14                    /* pt_regs->r14 */
+	xorq	%r14, %r14		/* nospec   r14 */
 	pushq   %r15                    /* pt_regs->r15 */
+	xorq	%r15, %r15		/* nospec   r15 */
 	cld
 
 	/*

[PATCH tip-pti 1/2] x86/entry: remove SAVE_C_REGS_EXCEPT_* macros

[Dominik Brodowski]

The macros which save all but specific registers have been unused for
a long time. Remove them and the SAVE_C_REGS_HELPER macro, but
instead provide the SAVE_C_REGS macro directly.

Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index f4b129d4af42..75a237c95ff7 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -101,41 +101,18 @@ For 32-bit we have the following conventions - kernel is built with
 	addq	$-(15*8), %rsp
 	.endm
 
-	.macro SAVE_C_REGS_HELPER offset=0 rax=1 rcx=1 r8910=1 r11=1
-	.if \r11
+	.macro SAVE_C_REGS offset=0
 	movq %r11, 6*8+\offset(%rsp)
-	.endif
-	.if \r8910
 	movq %r10, 7*8+\offset(%rsp)
 	movq %r9,  8*8+\offset(%rsp)
 	movq %r8,  9*8+\offset(%rsp)
-	.endif
-	.if \rax
 	movq %rax, 10*8+\offset(%rsp)
-	.endif
-	.if \rcx
 	movq %rcx, 11*8+\offset(%rsp)
-	.endif
 	movq %rdx, 12*8+\offset(%rsp)
 	movq %rsi, 13*8+\offset(%rsp)
 	movq %rdi, 14*8+\offset(%rsp)
 	UNWIND_HINT_REGS offset=\offset extra=0
 	.endm
-	.macro SAVE_C_REGS offset=0
-	SAVE_C_REGS_HELPER \offset, 1, 1, 1, 1
-	.endm
-	.macro SAVE_C_REGS_EXCEPT_RAX_RCX offset=0
-	SAVE_C_REGS_HELPER \offset, 0, 0, 1, 1
-	.endm
-	.macro SAVE_C_REGS_EXCEPT_R891011
-	SAVE_C_REGS_HELPER 0, 1, 1, 0, 0
-	.endm
-	.macro SAVE_C_REGS_EXCEPT_RCX_R891011
-	SAVE_C_REGS_HELPER 0, 1, 0, 0, 0
-	.endm
-	.macro SAVE_C_REGS_EXCEPT_RAX_RCX_R11
-	SAVE_C_REGS_HELPER 0, 0, 0, 1, 0
-	.endm
 
 	.macro SAVE_EXTRA_REGS offset=0
 	movq %r15, 0*8+\offset(%rsp)

[PATCH tip-pti 2/2] x86/entry: interleave XOR register clearing with PUSH/MOV instructions

[Dominik Brodowski]

Same as is done for syscalls, interleave XOR with PUSH or MOV
instructions for exceptions/interrupts, in order to minimize
the cost of the additional instructions required for register
clearing.

Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index 75a237c95ff7..89a906c868d8 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -102,10 +102,21 @@ For 32-bit we have the following conventions - kernel is built with
 	.endm
 
 	.macro SAVE_C_REGS offset=0
+	/*
+	 * Save registers and sanitize registers of values that a
+	 * speculation attack might otherwise want to exploit. The
+	 * lower registers are likely clobbered well before they
+	 * could be put to use in a speculative execution gadget.
+	 * Interleave XOR with MOV for better uop scheduling:
+	 */
 	movq %r11, 6*8+\offset(%rsp)
+	xorq %r11, %r11				/* nospec r11 */
 	movq %r10, 7*8+\offset(%rsp)
+	xorq %r10, %r10				/* nospec r10 */
 	movq %r9,  8*8+\offset(%rsp)
+	xorq %r9, %r9				/* nospec r9 */
 	movq %r8,  9*8+\offset(%rsp)
+	xorq %r8, %r8				/* nospec r8 */
 	movq %rax, 10*8+\offset(%rsp)
 	movq %rcx, 11*8+\offset(%rsp)
 	movq %rdx, 12*8+\offset(%rsp)
@@ -115,34 +126,28 @@ For 32-bit we have the following conventions - kernel is built with
 	.endm
 
 	.macro SAVE_EXTRA_REGS offset=0
+	/*
+	 * Save registers and sanitize registers of values that a
+	 * speculation attack might otherwise want to exploit. The
+	 * lower registers are likely clobbered well before they
+	 * could be put to use in a speculative execution gadget.
+	 * Interleave XOR with MOV for better uop scheduling:
+	 */
 	movq %r15, 0*8+\offset(%rsp)
+	xorq %r15, %r15				/* nospec r15 */
 	movq %r14, 1*8+\offset(%rsp)
+	xorq %r14, %r14				/* nospec r14 */
 	movq %r13, 2*8+\offset(%rsp)
+	xorq %r13, %r13				/* nospec r13 */
 	movq %r12, 3*8+\offset(%rsp)
+	xorq %r12, %r12				/* nospec r12 */
 	movq %rbp, 4*8+\offset(%rsp)
+	xorl %ebp, %ebp				/* nospec rbp */
 	movq %rbx, 5*8+\offset(%rsp)
+	xorl %ebx, %ebx				/* nospec rbx */
 	UNWIND_HINT_REGS offset=\offset
 	.endm
 
-	/*
-	 * Sanitize registers of values that a speculation attack
-	 * might otherwise want to exploit. The lower registers are
-	 * likely clobbered well before they could be put to use in
-	 * a speculative execution gadget:
-	 */
-	.macro CLEAR_REGS_NOSPEC
-	xorl %ebp, %ebp
-	xorl %ebx, %ebx
-	xorq %r8, %r8
-	xorq %r9, %r9
-	xorq %r10, %r10
-	xorq %r11, %r11
-	xorq %r12, %r12
-	xorq %r13, %r13
-	xorq %r14, %r14
-	xorq %r15, %r15
-	.endm
-
 	.macro POP_EXTRA_REGS
 	popq %r15
 	popq %r14
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 9e48002b953b..903d9088bdb3 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -575,7 +575,6 @@ END(irq_entries_start)
 	ALLOC_PT_GPREGS_ON_STACK
 	SAVE_C_REGS
 	SAVE_EXTRA_REGS
-	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER
 
 	testb	$3, CS(%rsp)
@@ -1134,7 +1133,6 @@ ENTRY(xen_failsafe_callback)
 	ALLOC_PT_GPREGS_ON_STACK
 	SAVE_C_REGS
 	SAVE_EXTRA_REGS
-	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER
 	jmp	error_exit
 END(xen_failsafe_callback)
@@ -1180,7 +1178,6 @@ ENTRY(paranoid_entry)
 	cld
 	SAVE_C_REGS 8
 	SAVE_EXTRA_REGS 8
-	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER 8
 	movl	$1, %ebx
 	movl	$MSR_GS_BASE, %ecx
@@ -1233,7 +1230,6 @@ ENTRY(error_entry)
 	cld
 	SAVE_C_REGS 8
 	SAVE_EXTRA_REGS 8
-	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER 8
 	testb	$3, CS+8(%rsp)
 	jz	.Lerror_kernelspace
@@ -1420,18 +1416,34 @@ ENTRY(nmi)
 	pushq   (%rdx)		/* pt_regs->dx */
 	pushq   %rcx		/* pt_regs->cx */
 	pushq   %rax		/* pt_regs->ax */
+	/*
+	 * Sanitize registers of values that a speculation attack
+	 * might otherwise want to exploit. The lower registers are
+	 * likely clobbered well before they could be put to use in
+	 * a speculative execution gadget. Interleave XOR with PUSH
+	 * for better uop scheduling:
+	 */
 	pushq   %r8		/* pt_regs->r8 */
+	xorq    %r8, %r8	/* nospec   r8 */
 	pushq   %r9		/* pt_regs->r9 */
+	xorq    %r9, %r9	/* nospec   r9 */
 	pushq   %r10		/* pt_regs->r10 */
+	xorq    %r10, %r10	/* nospec   r10 */
 	pushq   %r11		/* pt_regs->r11 */
+	xorq    %r11, %r11	/* nospec   r11*/
 	pushq	%rbx		/* pt_regs->rbx */
+	xorl    %ebx, %ebx	/* nospec   rbx*/
 	pushq	%rbp		/* pt_regs->rbp */
+	xorl    %ebp, %ebp	/* nospec   rbp*/
 	pushq	%r12		/* pt_regs->r12 */
+	xorq    %r12, %r12	/* nospec   r12*/
 	pushq	%r13		/* pt_regs->r13 */
+	xorq    %r13, %r13	/* nospec   r13*/
 	pushq	%r14		/* pt_regs->r14 */
+	xorq    %r14, %r14	/* nospec   r14*/
 	pushq	%r15		/* pt_regs->r15 */
+	xorq    %r15, %r15	/* nospec   r15*/
 	UNWIND_HINT_REGS
-	CLEAR_REGS_NOSPEC
 	ENCODE_FRAME_POINTER
 
 	/*

Re: [PATCH tip-pti 2/2] x86/entry: interleave XOR register clearing with PUSH/MOV instructions

[Dan Williams]

On Tue, Feb 6, 2018 at 1:32 PM, Dominik Brodowski
<linux@dominikbrodowski.net> wrote:
> Same as is done for syscalls, interleave XOR with PUSH or MOV
> instructions for exceptions/interrupts, in order to minimize
> the cost of the additional instructions required for register
> clearing.
>
> Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

Looks good to me.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

>
> diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
> index 75a237c95ff7..89a906c868d8 100644
> --- a/arch/x86/entry/calling.h
> +++ b/arch/x86/entry/calling.h
> @@ -102,10 +102,21 @@ For 32-bit we have the following conventions - kernel is built with
>         .endm
>
>         .macro SAVE_C_REGS offset=0
> +       /*
> +        * Save registers and sanitize registers of values that a
> +        * speculation attack might otherwise want to exploit. The
> +        * lower registers are likely clobbered well before they
> +        * could be put to use in a speculative execution gadget.
> +        * Interleave XOR with MOV for better uop scheduling:
> +        */
>         movq %r11, 6*8+\offset(%rsp)
> +       xorq %r11, %r11                         /* nospec r11 */
>         movq %r10, 7*8+\offset(%rsp)
> +       xorq %r10, %r10                         /* nospec r10 */
>         movq %r9,  8*8+\offset(%rsp)
> +       xorq %r9, %r9                           /* nospec r9 */
>         movq %r8,  9*8+\offset(%rsp)
> +       xorq %r8, %r8                           /* nospec r8 */
>         movq %rax, 10*8+\offset(%rsp)
>         movq %rcx, 11*8+\offset(%rsp)
>         movq %rdx, 12*8+\offset(%rsp)
> @@ -115,34 +126,28 @@ For 32-bit we have the following conventions - kernel is built with
>         .endm
>
>         .macro SAVE_EXTRA_REGS offset=0
> +       /*
> +        * Save registers and sanitize registers of values that a
> +        * speculation attack might otherwise want to exploit. The
> +        * lower registers are likely clobbered well before they
> +        * could be put to use in a speculative execution gadget.
> +        * Interleave XOR with MOV for better uop scheduling:
> +        */
>         movq %r15, 0*8+\offset(%rsp)
> +       xorq %r15, %r15                         /* nospec r15 */
>         movq %r14, 1*8+\offset(%rsp)
> +       xorq %r14, %r14                         /* nospec r14 */
>         movq %r13, 2*8+\offset(%rsp)
> +       xorq %r13, %r13                         /* nospec r13 */
>         movq %r12, 3*8+\offset(%rsp)
> +       xorq %r12, %r12                         /* nospec r12 */
>         movq %rbp, 4*8+\offset(%rsp)
> +       xorl %ebp, %ebp                         /* nospec rbp */
>         movq %rbx, 5*8+\offset(%rsp)
> +       xorl %ebx, %ebx                         /* nospec rbx */
>         UNWIND_HINT_REGS offset=\offset
>         .endm
>
> -       /*
> -        * Sanitize registers of values that a speculation attack
> -        * might otherwise want to exploit. The lower registers are
> -        * likely clobbered well before they could be put to use in
> -        * a speculative execution gadget:
> -        */
> -       .macro CLEAR_REGS_NOSPEC
> -       xorl %ebp, %ebp
> -       xorl %ebx, %ebx
> -       xorq %r8, %r8
> -       xorq %r9, %r9
> -       xorq %r10, %r10
> -       xorq %r11, %r11
> -       xorq %r12, %r12
> -       xorq %r13, %r13
> -       xorq %r14, %r14
> -       xorq %r15, %r15
> -       .endm
> -
>         .macro POP_EXTRA_REGS
>         popq %r15
>         popq %r14
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index 9e48002b953b..903d9088bdb3 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -575,7 +575,6 @@ END(irq_entries_start)
>         ALLOC_PT_GPREGS_ON_STACK
>         SAVE_C_REGS
>         SAVE_EXTRA_REGS
> -       CLEAR_REGS_NOSPEC
>         ENCODE_FRAME_POINTER
>
>         testb   $3, CS(%rsp)
> @@ -1134,7 +1133,6 @@ ENTRY(xen_failsafe_callback)
>         ALLOC_PT_GPREGS_ON_STACK
>         SAVE_C_REGS
>         SAVE_EXTRA_REGS
> -       CLEAR_REGS_NOSPEC
>         ENCODE_FRAME_POINTER
>         jmp     error_exit
>  END(xen_failsafe_callback)
> @@ -1180,7 +1178,6 @@ ENTRY(paranoid_entry)
>         cld
>         SAVE_C_REGS 8
>         SAVE_EXTRA_REGS 8
> -       CLEAR_REGS_NOSPEC
>         ENCODE_FRAME_POINTER 8
>         movl    $1, %ebx
>         movl    $MSR_GS_BASE, %ecx
> @@ -1233,7 +1230,6 @@ ENTRY(error_entry)
>         cld
>         SAVE_C_REGS 8
>         SAVE_EXTRA_REGS 8
> -       CLEAR_REGS_NOSPEC
>         ENCODE_FRAME_POINTER 8
>         testb   $3, CS+8(%rsp)
>         jz      .Lerror_kernelspace
> @@ -1420,18 +1416,34 @@ ENTRY(nmi)
>         pushq   (%rdx)          /* pt_regs->dx */
>         pushq   %rcx            /* pt_regs->cx */
>         pushq   %rax            /* pt_regs->ax */
> +       /*
> +        * Sanitize registers of values that a speculation attack
> +        * might otherwise want to exploit. The lower registers are
> +        * likely clobbered well before they could be put to use in
> +        * a speculative execution gadget. Interleave XOR with PUSH
> +        * for better uop scheduling:
> +        */
>         pushq   %r8             /* pt_regs->r8 */
> +       xorq    %r8, %r8        /* nospec   r8 */
>         pushq   %r9             /* pt_regs->r9 */
> +       xorq    %r9, %r9        /* nospec   r9 */
>         pushq   %r10            /* pt_regs->r10 */
> +       xorq    %r10, %r10      /* nospec   r10 */
>         pushq   %r11            /* pt_regs->r11 */
> +       xorq    %r11, %r11      /* nospec   r11*/
>         pushq   %rbx            /* pt_regs->rbx */
> +       xorl    %ebx, %ebx      /* nospec   rbx*/
>         pushq   %rbp            /* pt_regs->rbp */
> +       xorl    %ebp, %ebp      /* nospec   rbp*/
>         pushq   %r12            /* pt_regs->r12 */
> +       xorq    %r12, %r12      /* nospec   r12*/
>         pushq   %r13            /* pt_regs->r13 */
> +       xorq    %r13, %r13      /* nospec   r13*/
>         pushq   %r14            /* pt_regs->r14 */
> +       xorq    %r14, %r14      /* nospec   r14*/
>         pushq   %r15            /* pt_regs->r15 */
> +       xorq    %r15, %r15      /* nospec   r15*/
>         UNWIND_HINT_REGS
> -       CLEAR_REGS_NOSPEC
>         ENCODE_FRAME_POINTER
>
>         /*

Re: [PATCH tip-pti 2/2] x86/entry: interleave XOR register clearing with PUSH/MOV instructions

[Linus Torvalds]

On Tue, Feb 6, 2018 at 1:32 PM, Dominik Brodowski
<linux@dominikbrodowski.net> wrote:
> Same as is done for syscalls, interleave XOR with PUSH or MOV
> instructions for exceptions/interrupts, in order to minimize
> the cost of the additional instructions required for register
> clearing.

Side note: I would _really_ like to see

 (a) SAVE_{C,EXTRA}_REGS go away entirely, to be replaced by just SAVE_REGS.

     We never use them independently of each other any more.

 (b) Get rid of ALLOC_PT_GPREGS_ON_STACK entirely, and make SAVE_REGS
use pushq's instead of movs.

Doing (a) should be completely trivial.

Doing (b) looks like it needs _some_ care, because
ALLOC_PT_GPREGS_ON_STACK is not always done just before the SAVE_REGS,
the error entry code does it in in the early entry code. But honestly,
that seems mainly so that it can do

        testb   $3, CS(%rsp)                    /* If coming from
userspace, switch stacks */

before registers are saved, yet use the same CS offset as if they had
already been saved. So that _one_ stack offset in the 'idtentry' macro
would need to be fixed up.

There might be others that I don't see from just eyeballing, so it
does need some care, but wouldn't it be nice if *all* the entry code
could just use the same pushq sequences, and then put the xor's in
there?

The reason for that complexity is purely the system call fastpath case
that no longer exists, I think.

Am I missing something?

                Linus

Re: [PATCH tip-pti 1/2] x86/entry: remove SAVE_C_REGS_EXCEPT_* macros

[Linus Torvalds]

On Tue, Feb 6, 2018 at 1:25 PM, Dominik Brodowski
<linux@dominikbrodowski.net> wrote:
> The macros which save all but specific registers have been unused for
> a long time. Remove them and the SAVE_C_REGS_HELPER macro, but
> instead provide the SAVE_C_REGS macro directly.

Ack. But see my other email about even more cleanups.

At least the SAVE_{C,EXTRA}_REGS -> SAVE_REGS combination cleanup
should be totally mindless, since they are always used together and
with the same argument.

Unless I'm blind.

           Linus

Re: [PATCH tip-pti 2/2] x86/entry: interleave XOR register clearing with PUSH/MOV instructions

[Andy Lutomirski]

On Tue, Feb 6, 2018 at 10:48 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Feb 6, 2018 at 1:32 PM, Dominik Brodowski
> <linux@dominikbrodowski.net> wrote:
>> Same as is done for syscalls, interleave XOR with PUSH or MOV
>> instructions for exceptions/interrupts, in order to minimize
>> the cost of the additional instructions required for register
>> clearing.
>
> Side note: I would _really_ like to see
>
>  (a) SAVE_{C,EXTRA}_REGS go away entirely, to be replaced by just SAVE_REGS.
>
>      We never use them independently of each other any more.
>
>  (b) Get rid of ALLOC_PT_GPREGS_ON_STACK entirely, and make SAVE_REGS
> use pushq's instead of movs.

Agreed.

However, bit fat NAK to the patch as it.  There's no way I'm okay with
a macro called SAVE_C_REGS that actually saves *and clears* C regs.
Call it SAVE_AND_CLEAR_C_REGS.

>
> Doing (a) should be completely trivial.
>
> Doing (b) looks like it needs _some_ care, because
> ALLOC_PT_GPREGS_ON_STACK is not always done just before the SAVE_REGS,
> the error entry code does it in in the early entry code. But honestly,
> that seems mainly so that it can do
>
>         testb   $3, CS(%rsp)                    /* If coming from
> userspace, switch stacks */
>
> before registers are saved, yet use the same CS offset as if they had
> already been saved. So that _one_ stack offset in the 'idtentry' macro
> would need to be fixed up.
>
> There might be others that I don't see from just eyeballing, so it
> does need some care, but wouldn't it be nice if *all* the entry code
> could just use the same pushq sequences, and then put the xor's in
> there?
>
> The reason for that complexity is purely the system call fastpath case
> that no longer exists, I think.
>
> Am I missing something?

I don't think so.

idtentry could use some massive cleanups IMO.  At some point I'll find
time to do it.  We've added features to it piecemeal over time, and
the net result including the stack switch for PTI is a mess.

>
>                 Linus

Re: [PATCH tip-pti 2/2] x86/entry: interleave XOR register clearing with PUSH/MOV instructions

[Andi Kleen]

> The reason for that complexity is purely the system call fastpath case
> that no longer exists, I think.
> 
> Am I missing something?

Yes merging the macros should be fine without fast path. 

But for push, on older CPUs (older AMD, most Atoms, really old Intel big core) 
sub+mov is a lot faster than push because push has additional dependencies
causing pipeline bubbles. So you would make these cases slower if you
use PUSH.

That is no different between fast path and slow path.

-Andi

PS it was never fully clear to me why we removed the fast path. After all it 
could still be useful on the future CPUs with Spectre hardware fixes.

Re: [PATCH tip-pti 2/2] x86/entry: interleave XOR register clearing with PUSH/MOV instructions

[Linus Torvalds]

On Tue, Feb 6, 2018 at 3:54 PM, Andi Kleen <ak@linux.intel.com> wrote:
>
> But for push, on older CPUs (older AMD, most Atoms, really old Intel big core)
> sub+mov is a lot faster than push because push has additional dependencies
> causing pipeline bubbles. So you would make these cases slower if you
> use PUSH.

I refuse to optimize for old CPU's.

Also, even for old CPU's, the push sequence is *much* smaller than the
mov sequence. And really, just a single extra cache miss more than
eats up any advantage you get from decoding.

> PS it was never fully clear to me why we removed the fast path. After all it
> could still be useful on the future CPUs with Spectre hardware fixes.

The fastpath really messes up all these cleanups, and forced that
"mov" sequence and illegible code.

Plus the fastpath couldn't clear those registers anyway, since it
didn't even _save_ them - exactly because the whole point of the
fastpath was that not all registers are clobbered by the calling
conventions.

We can try to see if it's worth re-instating in a few years when
hopefully fixed CPU's will be the norm. Right now the fast path
definitely made no sense.

           Linus

Re: [PATCH tip-pti 2/2] x86/entry: interleave XOR register clearing with PUSH/MOV instructions

[Andi Kleen]

> Plus the fastpath couldn't clear those registers anyway, since it
> didn't even _save_ them - exactly because the whole point of the
> fastpath was that not all registers are clobbered by the calling
> conventions.

Fast path saves more than just register saving.  I changed the fast path
to save all registers in my earlier clearregs branches

https://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git/log/?h=spec/clearregs-3

It is still quite a bit faster than all the slow stuff the C do_syscall
code does (e.g. reloading all the arguments, setting up unnecessary
frame pointers etc.).

Just take a look at the disassembly of that function. It's really
not very optimized.

-Andi

Re: [PATCH tip-pti 2/2] x86/entry: interleave XOR register clearing with PUSH/MOV instructions

[Linus Torvalds]

On Wed, Feb 7, 2018 at 7:18 AM, Andi Kleen <ak@linux.intel.com> wrote:
>
> Fast path saves more than just register saving.  I changed the fast path
> to save all registers in my earlier clearregs branches

I know. I saw your patches. And I went "Eww".

> It is still quite a bit faster than all the slow stuff the C do_syscall
> code does (e.g. reloading all the arguments, setting up unnecessary
> frame pointers etc.).
>
> Just take a look at the disassembly of that function. It's really
> not very optimized.

Actually, only the argument reloading _really_ annoys me in do_syscall().

And I do think we should be able to fix that fairly easily by moving
it into the SYSCALLx() macros - at least for 64-bit (32-bit has nasty
issues with 64-bit arguments).

The other thing we need to do is to just pass down the system call
number as an argument instead of reloading it.

         Linus

Re: [PATCH tip-pti 2/2] x86/entry: interleave XOR register clearing with PUSH/MOV instructions

[Linus Torvalds]

On Wed, Feb 7, 2018 at 9:05 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> The other thing we need to do is to just pass down the system call
> number as an argument instead of reloading it.

.. we may also want to disable some debug things.

For example, if you enable KASAN, it does insane things for
do_syscall(). I realize that nobody is supposed to care about
performance for KASAN, but still...

So one of the issues with do_syscall() is simply that _because_ it was
the rare case only, the code simply was never even looked at. But if y
ou look at the straight-line case, do_syscall_64() really doesn't look
too bad apart from the argument reloading that I do think is largely
fixable.

              Linus