Re: [PATCH 4.19 37/48] x86/entry/64: Fix unwind hints in register clearing code

[Pavel Machek <pavel@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 3407 bytes --]

On Wed 2020-05-13 11:45:03, Greg Kroah-Hartman wrote:
> From: Josh Poimboeuf <jpoimboe@redhat.com>
> 
> commit 06a9750edcffa808494d56da939085c35904e618 upstream.
> 
> The PUSH_AND_CLEAR_REGS macro zeroes each register immediately after
> pushing it.  If an NMI or exception hits after a register is cleared,
> but before the UNWIND_HINT_REGS annotation, the ORC unwinder will
> wrongly think the previous value of the register was zero.  This can
> confuse the unwinding process and cause it to exit early.
> 
> Because ORC is simpler than DWARF, there are a limited number of unwind
> annotation states, so it's not possible to add an individual unwind hint
> after each push/clear combination.  Instead, the register clearing
> instructions need to be consolidated and moved to after the
> UNWIND_HINT_REGS annotation.

This actually makes kernel entry/exit slower, due to poor instruction
scheduling. And that is a bit of hot path... Is it strictly
neccessary? Not everyone needs ORC scheduler. Should it be somehow
optional?

Best regards,
								Pavel

> -	 * Interleave XOR with PUSH for better uop scheduling:
> -	 */
>  	.if \save_ret
>  	pushq	%rsi		/* pt_regs->si */
>  	movq	8(%rsp), %rsi	/* temporarily store the return address in %rsi */
> @@ -114,34 +107,43 @@ For 32-bit we have the following convent
>  	pushq   %rsi		/* pt_regs->si */
>  	.endif
>  	pushq	\rdx		/* pt_regs->dx */
> -	xorl	%edx, %edx	/* nospec   dx */
>  	pushq   %rcx		/* pt_regs->cx */
> -	xorl	%ecx, %ecx	/* nospec   cx */
>  	pushq   \rax		/* pt_regs->ax */
>  	pushq   %r8		/* pt_regs->r8 */
> -	xorl	%r8d, %r8d	/* nospec   r8 */
>  	pushq   %r9		/* pt_regs->r9 */
> -	xorl	%r9d, %r9d	/* nospec   r9 */
>  	pushq   %r10		/* pt_regs->r10 */
> -	xorl	%r10d, %r10d	/* nospec   r10 */
>  	pushq   %r11		/* pt_regs->r11 */
> -	xorl	%r11d, %r11d	/* 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 */
> -	xorl	%r12d, %r12d	/* nospec   r12*/
>  	pushq	%r13		/* pt_regs->r13 */
> -	xorl	%r13d, %r13d	/* nospec   r13*/
>  	pushq	%r14		/* pt_regs->r14 */
> -	xorl	%r14d, %r14d	/* nospec   r14*/
>  	pushq	%r15		/* pt_regs->r15 */
> -	xorl	%r15d, %r15d	/* nospec   r15*/
>  	UNWIND_HINT_REGS
> +
>  	.if \save_ret
>  	pushq	%rsi		/* return address on top of stack */
>  	.endif
> +
> +	/*
> +	 * 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.
> +	 */
> +	xorl	%edx,  %edx	/* nospec dx  */
> +	xorl	%ecx,  %ecx	/* nospec cx  */
> +	xorl	%r8d,  %r8d	/* nospec r8  */
> +	xorl	%r9d,  %r9d	/* nospec r9  */
> +	xorl	%r10d, %r10d	/* nospec r10 */
> +	xorl	%r11d, %r11d	/* nospec r11 */
> +	xorl	%ebx,  %ebx	/* nospec rbx */
> +	xorl	%ebp,  %ebp	/* nospec rbp */
> +	xorl	%r12d, %r12d	/* nospec r12 */
> +	xorl	%r13d, %r13d	/* nospec r13 */
> +	xorl	%r14d, %r14d	/* nospec r14 */
> +	xorl	%r15d, %r15d	/* nospec r15 */
> +
>  .endm
>  
>  .macro POP_REGS pop_rdi=1 skip_r11rcx=0
> 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]