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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200513214856.GA27858@amd>
Date:   Wed, 13 May 2020 23:48:56 +0200
From:   Pavel Machek <pavel@...x.de>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Miroslav Benes <mbenes@...e.cz>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Ingo Molnar <mingo@...nel.org>,
        Andy Lutomirski <luto@...nel.org>, Dave Jones <dsj@...com>,
        Jann Horn <jannh@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Vince Weaver <vincent.weaver@...ne.edu>
Subject: Re: [PATCH 4.19 37/48] x86/entry/64: Fix unwind hints in register
 clearing code

On Wed 2020-05-13 11:45:03, Greg Kroah-Hartman wrote:
> From: Josh Poimboeuf <jpoimboe@...hat.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

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)