All of
 help / color / mirror / Atom feed
From: Andy Lutomirski <>
To:, LKML <>
Cc: Juergen Gross <>, "H. Peter Anvin" <>,
	Brian Gerst <>,
	Dave Hansen <>,
	Dominik Brodowski <>,
	Ingo Molnar <>, Borislav Petkov <>,, Andy Lutomirski <>,
	Greg KH <>,,
	Thomas Gleixner <>,
	Linus Torvalds <>,
	Boris Ostrovsky <>
Subject: [PATCH] x86/entry/64: Remove %ebx handling from error_entry/exit
Date: Sun, 22 Jul 2018 11:05:09 -0700	[thread overview]
Message-ID: <b5010a090d3586b2d6e06c7ad3ec5542d1241c45.1532282627.git.luto__32605.7672886693$1532282655$gmane$> (raw)

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:

        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.

I suspect that malicious userspace could use this bug to crash the
kernel even without the offending patch applied, though.

[Historical note: I wrote this patch as a cleanup before I was aware
 of the bug it fixed.]

[Note to stable maintainers: this should probably get applied to all
 kernels.  If you're nervous about that, a more conservative fix to
 add xorl %ebx,%ebx; incl %ebx before the jump to error_exit should
 also fix the problem.]

Cc: Brian Gerst <>
Cc: Borislav Petkov <>
Cc: Dominik Brodowski <>
Cc: Ingo Molnar <>
Cc: "H. Peter Anvin" <>
Cc: Thomas Gleixner <>
Cc: Boris Ostrovsky <>
Cc: Juergen Gross <>
Fixes: 3ac6d8c787b8 ("x86/entry/64: Clear registers for exceptions/interrupts, to reduce speculation attack surface")
Reported-and-tested-by: "M. Vefa Bicakci" <>
Signed-off-by: Andy Lutomirski <>

I could also submit the conservative fix tagged for -stable and respin
this on top of it.  Ingo, Greg, what do you prefer?

 arch/x86/entry/entry_64.S | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 73a522d53b53..8ae7ffda8f98 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -981,7 +981,7 @@ ENTRY(\sym)
 	call	\do_sym
-	jmp	error_exit			/* %ebx: no swapgs flag */
+	jmp	error_exit
@@ -1222,7 +1222,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
@@ -1269,7 +1268,6 @@ ENTRY(error_entry)
 	 * for these here too.
-	incl	%ebx
 	leaq	native_irq_return_iret(%rip), %rcx
 	cmpq	%rcx, RIP+8(%rsp)
 	je	.Lerror_bad_iret
@@ -1303,28 +1301,20 @@ 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
- * 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
- */
-	testl	%ebx, %ebx
-	jnz	retint_kernel
+	testb	$3, CS(%rsp)
+	jz	retint_kernel
 	jmp	retint_user

Xen-devel mailing list

             reply	other threads:[~2018-07-22 18:05 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-22 18:05 Andy Lutomirski [this message]
2018-07-22 18:05 Andy Lutomirski
2018-07-23  7:25 ` Greg KH
2018-07-24  2:31   ` Andy Lutomirski
2018-07-24  2:31   ` Andy Lutomirski
2018-07-23  7:25 ` Greg KH
2018-08-07 22:57 backport of XSA-274 patch to 4.9.x kernel (could use a review) Andy Lutomirski
2018-08-08 17:35 ` [PATCH] x86/entry/64: Remove %ebx handling from error_entry/exit Sarah Newman
2018-08-08 17:35   ` Sarah Newman
2018-08-09 12:41   ` David Woodhouse
2018-08-09 12:41   ` [Xen-devel] " David Woodhouse
2018-08-10  7:23     ` Sarah Newman
2018-08-10  7:23     ` [Xen-devel] " Sarah Newman
2018-08-16 15:19       ` Greg KH
2018-08-16 15:35         ` Andy Lutomirski
2018-08-16 15:19       ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='b5010a090d3586b2d6e06c7ad3ec5542d1241c45.1532282627.git.luto__32605.7672886693$1532282655$gmane$' \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH] x86/entry/64: Remove %ebx handling from error_entry/exit' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.