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

Pavel Machek
2020-05-13T21:49:00Z

[-- 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 --]