From: Sean Christopherson <seanjc@google.com>
To: Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
x86@kernel.org
Cc: linux-kernel@vger.kernel.org, Srikanth Aithal <sraithal@amd.com>,
kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
Sean Christopherson <seanjc@google.com>
Subject: [PATCH] x86/retpoline: Don't clobber RFLAGS during srso_safe_ret()
Date: Fri, 11 Aug 2023 08:52:55 -0700 [thread overview]
Message-ID: <20230811155255.250835-1-seanjc@google.com> (raw)
Use 'lea' instead of 'add' when adjusting %rsp in srso_safe_ret() so as to
avoid clobbering flags. Drop one of the INT3 instructions to account for
the LEA consuming one more byte than the ADD.
KVM's emulator makes indirect calls into a jump table of sorts, where
the destination of each call is a small blob of code that performs fast
emulation by executing the target instruction with fixed operands.
E.g. to emulate ADC, fastop() invokes adcb_al_dl():
adcb_al_dl:
0xffffffff8105f5f0 <+0>: adc %dl,%al
0xffffffff8105f5f2 <+2>: jmp 0xffffffff81a39270 <__x86_return_thunk>
A major motivation for doing fast emulation is to leverage the CPU to
handle consumption and manipulation of arithmetic flags, i.e. RFLAGS is
both an input and output to the target of the call. fastop() collects
the RFLAGS result by pushing RFLAGS onto the stack and popping them back
into a variable (held in RDI in this case)
asm("push %[flags]; popf; " CALL_NOSPEC " ; pushf; pop %[flags]\n"
0xffffffff81062be7 <+71>: mov 0xc0(%r8),%rdx
0xffffffff81062bee <+78>: mov 0x100(%r8),%rcx
0xffffffff81062bf5 <+85>: push %rdi
0xffffffff81062bf6 <+86>: popf
0xffffffff81062bf7 <+87>: call *%rsi
0xffffffff81062bf9 <+89>: nop
0xffffffff81062bfa <+90>: nop
0xffffffff81062bfb <+91>: nop
0xffffffff81062bfc <+92>: pushf
0xffffffff81062bfd <+93>: pop %rdi
and then propagating the arithmetic flags into the vCPU's emulator state:
ctxt->eflags = (ctxt->eflags & ~EFLAGS_MASK) | (flags & EFLAGS_MASK);
0xffffffff81062be0 <+64>: and $0xfffffffffffff72a,%r9
0xffffffff81062bfe <+94>: and $0x8d5,%edi
0xffffffff81062c0d <+109>: or %rdi,%r9
0xffffffff81062c1a <+122>: mov %r9,0x10(%r8)
The failures can be most easily reproduced by running the "emulator" test
in KVM-Unit-Tests.
If you're feeling a bit of deja vu, see commit b63f20a778c8
("x86/retpoline: Don't clobber RFLAGS during CALL_NOSPEC on i386").
Fixes: fb3bd914b3ec ("x86/srso: Add a Speculative RAS Overflow mitigation")
Reported-by: Srikanth Aithal <sraithal@amd.com>
Closes: https://lore.kernel.org/all/de474347-122d-54cd-eabf-9dcc95ab9eae@amd.com
Cc: stable@vger.kernel.org
Cc: kvm@vger.kernel.org
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
Those that fail to learn from history are doomed to repeat it. :-D
arch/x86/lib/retpoline.S | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S
index 2cff585f22f2..132cedbf9e57 100644
--- a/arch/x86/lib/retpoline.S
+++ b/arch/x86/lib/retpoline.S
@@ -164,7 +164,7 @@ __EXPORT_THUNK(srso_untrain_ret_alias)
/* Needs a definition for the __x86_return_thunk alternative below. */
SYM_START(srso_safe_ret_alias, SYM_L_GLOBAL, SYM_A_NONE)
#ifdef CONFIG_CPU_SRSO
- add $8, %_ASM_SP
+ lea 8(%_ASM_SP), %_ASM_SP
UNWIND_HINT_FUNC
#endif
ANNOTATE_UNRET_SAFE
@@ -239,7 +239,7 @@ __EXPORT_THUNK(zen_untrain_ret)
* SRSO untraining sequence for Zen1/2, similar to zen_untrain_ret()
* above. On kernel entry, srso_untrain_ret() is executed which is a
*
- * movabs $0xccccccc308c48348,%rax
+ * movabs $0xccccc30824648d48,%rax
*
* and when the return thunk executes the inner label srso_safe_ret()
* later, it is a stack manipulation and a RET which is mispredicted and
@@ -252,11 +252,10 @@ SYM_START(srso_untrain_ret, SYM_L_GLOBAL, SYM_A_NONE)
.byte 0x48, 0xb8
SYM_INNER_LABEL(srso_safe_ret, SYM_L_GLOBAL)
- add $8, %_ASM_SP
+ lea 8(%_ASM_SP), %_ASM_SP
ret
int3
int3
- int3
lfence
call srso_safe_ret
int3
base-commit: 25aa0bebba72b318e71fe205bfd1236550cc9534
--
2.41.0.694.ge786442a9b-goog
next reply other threads:[~2023-08-11 15:53 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-11 15:52 Sean Christopherson [this message]
2023-08-11 17:28 ` [PATCH] x86/retpoline: Don't clobber RFLAGS during srso_safe_ret() Nathan Chancellor
2023-08-11 18:05 ` Borislav Petkov
2023-08-12 2:59 ` Sean Christopherson
2023-08-11 18:47 ` Mika Penttilä
2023-08-12 2:57 ` Sean Christopherson
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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230811155255.250835-1-seanjc@google.com \
--to=seanjc@google.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=sraithal@amd.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox