[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] backport of XSA-274 patch to 4.9.x kernel (could use a review)


  • To: xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Chris Brannon <cmb@xxxxxxxxx>
  • Date: Mon, 06 Aug 2018 12:10:52 -0700
  • Delivery-date: Mon, 06 Aug 2018 19:10:59 +0000
  • Dkim-filter: OpenDKIM Filter v2.11.0 mail.xen.prgmr.com E265A28C00B
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

I just got the following patch from a colleague.  It's a backport of
the XSA 274 kernel patch to 4.9.x kernels.  The kernel patch given in
the XSA would not apply cleanly.  Would someone mind reviewing it?  It
would be much appreciated.

commit b3681dd548d06deb2e1573890829dff4b15abf46 upstream.

This version applies to v4.9.

error_entry and error_exit communicate the user vs kernel status of
the frame using %ebx.  This is unnecessary -- the information is in
regs->cs.  Just use regs->cs.

This makes error_entry simpler and makes error_exit more robust.

It also fixes a nasty bug.  Before all the Spectre nonsense, The
xen_failsafe_callback entry point returned like this:

        ALLOC_PT_GPREGS_ON_STACK
        SAVE_C_REGS
        SAVE_EXTRA_REGS
        ENCODE_FRAME_POINTER
        jmp     error_exit

And it did not go through error_entry.  This was bogus: RBX
contained garbage, and error_exit expected a flag in RBX.
Fortunately, it generally contained *nonzero* garbage, so the
correct code path was used.  As part of the Spectre fixes, code was
added to clear RBX to mitigate certain speculation attacks.  Now,
depending on kernel configuration, RBX got zeroed and, when running
some Wine workloads, the kernel crashes.  This was introduced by:

    commit 3ac6d8c787b8 ("x86/entry/64: Clear registers for
    exceptions/interrupts, to reduce speculation attack surface")

With this patch applied, RBX is no longer needed as a flag, and the
problem goes away.

Cc: Brian Gerst <brgerst@xxxxxxxxx>
Cc: Borislav Petkov <bp@xxxxxxxxx>
Cc: Dominik Brodowski <linux@xxxxxxxxxxxxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
Cc: Juergen Gross <jgross@xxxxxxxx>
Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx
Cc: x86@xxxxxxxxxx
Cc: stable@xxxxxxxxxxxxxxx
Cc: Andy Lutomirski <luto@xxxxxxxxxx>
Fixes: 3ac6d8c787b8 ("x86/entry/64: Clear registers for exceptions/interrupts, 
to reduce speculation attack surface")
Reported-and-tested-by: "M. Vefa Bicakci" <m.v.b@xxxxxxxxxx>
Signed-off-by: Sarah Newman <srn@xxxxxxxxx>
---
 arch/x86/entry/entry_64.S | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index d58d8dc..0dab47a 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -774,7 +774,7 @@ ENTRY(\sym)
 
        call    \do_sym
 
-       jmp     error_exit                      /* %ebx: no swapgs flag */
+       jmp     error_exit
        .endif
 END(\sym)
 .endm
@@ -1043,7 +1043,6 @@ END(paranoid_exit)
 
 /*
  * Save all registers in pt_regs, and switch gs if needed.
- * Return: EBX=0: came from user mode; EBX=1: otherwise
  */
 ENTRY(error_entry)
        cld
@@ -1087,7 +1086,6 @@ ENTRY(error_entry)
         * for these here too.
         */
 .Lerror_kernelspace:
-       incl    %ebx
        leaq    native_irq_return_iret(%rip), %rcx
        cmpq    %rcx, RIP+8(%rsp)
        je      .Lerror_bad_iret
@@ -1119,28 +1117,19 @@ ENTRY(error_entry)
 
        /*
         * Pretend that the exception came from user mode: set up pt_regs
-        * as if we faulted immediately after IRET and clear EBX so that
-        * error_exit knows that we will be returning to user mode.
+        * as if we faulted immediately after IRET.
         */
        mov     %rsp, %rdi
        call    fixup_bad_iret
        mov     %rax, %rsp
-       decl    %ebx
        jmp     .Lerror_entry_from_usermode_after_swapgs
 END(error_entry)
 
-
-/*
- * On entry, EBX is a "return to kernel mode" flag:
- *   1: already in kernel mode, don't need SWAPGS
- *   0: user gsbase is loaded, we need SWAPGS and standard preparation for 
return to usermode
- */
 ENTRY(error_exit)
-       movl    %ebx, %eax
        DISABLE_INTERRUPTS(CLBR_NONE)
        TRACE_IRQS_OFF
-       testl   %eax, %eax
-       jnz     retint_kernel
+       testb   $3, CS(%rsp)
+       jz      retint_kernel
        jmp     retint_user
 END(error_exit)
 
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.