openSUSE Build Service

Overview Repositories Revisions Requests Users Attributes Meta

File xsa456-1.patch of Package xen

# Commit 8e186f98ce0e35d1754ec9299da41ec98873b65c
# Date 2024-04-09 16:37:30 +0100
# Author Andrew Cooper <andrew.cooper3@citrix.com>
# Committer Andrew Cooper <andrew.cooper3@citrix.com>
x86: Use indirect calls in reset-stack infrastructure

Mixing up JMP and CALL indirect targets leads a very fun form of speculative
type confusion.  A target which is expecting to be called CALLed needs a
return address on the stack, and an indirect JMP doesn't place one there.

An indirect JMP which predicts to a target intending to be CALLed can end up
with a RET speculatively executing with a value from the JMPers stack frame.

There are several ways get indirect JMPs in Xen.

* From tailcall optimisations.  These are safe because the compiler has
   arranged the stack to point at the callee's return address.

* From jump tables.  These are unsafe, but Xen is built with -fno-jump-tables
   to work around several compiler issues.

* From reset_stack_and_jump_ind(), which is particularly unsafe.  Because of
   the additional stack adjustment made, the value picked up off the stack is
   regs->r15 of the next vCPU to run.

In order to mitigate this type confusion, we want to make all indirect targets
be CALL targets, and remove the use of indirect JMP except via tailcall
optimisation.

Luckily due to XSA-348, all C target functions of reset_stack_and_jump_ind()
are noreturn.  {svm,vmx}_do_resume() exits via reset_stack_and_jump(); a
direct JMP with entirely different prediction properties.  idle_loop() is an
infinite loop which eventually exits via reset_stack_and_jump_ind() from a new
schedule.  i.e. These paths are all fine having one extra return address on
the stack.

This allows us change reset_stack_and_jump_ind() to reset_stack_and_call_ind()
in order to mitigate the speculative type confusion.

This is part of XSA-456 / CVE-2024-2201.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1848,12 +1848,12 @@ void context_switch(struct vcpu *prev, s
     /* Ensure that the vcpu has an up-to-date time base. */
     update_vcpu_system_time(next);
 
-    reset_stack_and_jump_ind(nextd->arch.ctxt_switch->tail);
+    reset_stack_and_call_ind(nextd->arch.ctxt_switch->tail);
 }
 
 void continue_running(struct vcpu *same)
 {
-    reset_stack_and_jump_ind(same->domain->arch.ctxt_switch->tail);
+    reset_stack_and_call_ind(same->domain->arch.ctxt_switch->tail);
 }
 
 int __sync_local_execstate(void)
--- a/xen/include/asm-x86/current.h
+++ b/xen/include/asm-x86/current.h
@@ -180,8 +180,8 @@ unsigned long get_stack_dump_bottom (uns
     switch_stack_and_jump(fn, "jmp %c", "i")
 
 /* The constraint may only specify non-call-clobbered registers. */
-#define reset_stack_and_jump_ind(fn)                                    \
-    switch_stack_and_jump(fn, "INDIRECT_JMP %", "b")
+#define reset_stack_and_call_ind(fn)                                    \
+    switch_stack_and_jump(fn, "INDIRECT_CALL %", "b")
 
 /*
  * Which VCPU's state is currently running on each CPU?

Places

File xsa456-1.patch of Package xen

Places