Subject: x86/stubs: Introduce place_ret() to abstract away raw 0xc3's
From: Andrew Cooper <[email-masked]>

The Indirect Target Selection speculative vulnerability means that indirect
branches (including RETs) are unsafe when in the first half of a cacheline.
This means it's not safe for logic using the stubs to write raw 0xc3's.

Introduce place_ret() which, for now, writes a raw 0xc3 but will contain
additional logic when return thunks are in use.

stub_selftest() doesn't strictly need to be converted as they only run on
boot, but doing so gets us a partial test of place_ret() too.

No functional change.

This is part of XSA-469 / CVE-2024-28956

Signed-off-by: Andrew Cooper <[email-masked]>
Reviewed-by: Roger Pau Monné <[email-masked]>

diff --git a/tools/tests/x86_emulator/x86-emulate.h b/tools/tests/x86_emulator/x86-emulate.h
index 929c1a72ae88..4c292ac338f6 100644
--- a/tools/tests/x86_emulator/x86-emulate.h
+++ b/tools/tests/x86_emulator/x86-emulate.h
@@ -77,6 +77,12 @@
 
 #define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63))
 
+static inline void *place_ret(void *ptr)
+{
+    *(uint8_t *)ptr = 0xc3;
+    return ptr + 1;
+}
+
 extern uint32_t mxcsr_mask;
 extern struct cpu_policy cpu_policy;
 
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 0ded68d06958..02ab235de7ff 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -11,9 +11,7 @@ obj-$(CONFIG_PV) += pv/
 obj-y += x86_64/
 obj-y += x86_emulate/
 
-alternative-y := alternative.init.o
-alternative-$(CONFIG_LIVEPATCH) :=
-obj-bin-y += $(alternative-y)
+obj-y += alternative.o
 obj-y += apic.o
 obj-y += bhb-thunk.o
 obj-y += bitops.o
@@ -41,7 +39,7 @@ obj-y += hypercall.o
 obj-y += i387.o
 obj-y += i8259.o
 obj-y += io_apic.o
-obj-$(CONFIG_LIVEPATCH) += alternative.o livepatch.o
+obj-$(CONFIG_LIVEPATCH) += livepatch.o
 obj-y += msi.o
 obj-y += msr.o
 obj-$(CONFIG_INDIRECT_THUNK) += indirect-thunk.o
diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index 88c90044c20d..ec451d962c10 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -137,6 +137,20 @@ void init_or_livepatch add_nops(void *insns, unsigned int len)
     }
 }
 
+/*
+ * Place a return at @ptr.  @ptr must be in the writable alias of a stub.
+ *
+ * Returns the next position to write into the stub.
+ */
+void *place_ret(void *ptr)
+{
+    uint8_t *p = ptr;
+
+    *p++ = 0xc3;
+
+    return p;
+}
+
 /*
  * text_poke - Update instructions on a live kernel or non-executed code.
  * @addr: address to modify
diff --git a/xen/arch/x86/extable.c b/xen/arch/x86/extable.c
index 705cf9eb94ca..1572efa69a00 100644
--- a/xen/arch/x86/extable.c
+++ b/xen/arch/x86/extable.c
@@ -151,20 +151,20 @@ search_exception_table(const struct cpu_user_regs *regs, unsigned long *stub_ra)
 int __init cf_check stub_selftest(void)
 {
     static const struct {
-        uint8_t opc[8];
+        uint8_t opc[7];
         uint64_t rax;
         union stub_exception_token res;
     } tests[] __initconst = {
 #define endbr64 0xf3, 0x0f, 0x1e, 0xfa
-        { .opc = { endbr64, 0x0f, 0xb9, 0xc3, 0xc3 }, /* ud1 */
+        { .opc = { endbr64, 0x0f, 0xb9, 0x90 }, /* ud1 */
           .res.fields.trapnr = X86_EXC_UD },
-        { .opc = { endbr64, 0x90, 0x02, 0x00, 0xc3 }, /* nop; add (%rax),%al */
+        { .opc = { endbr64, 0x90, 0x02, 0x00 }, /* nop; add (%rax),%al */
           .rax = 0x0123456789abcdef,
           .res.fields.trapnr = X86_EXC_GP },
-        { .opc = { endbr64, 0x02, 0x04, 0x04, 0xc3 }, /* add (%rsp,%rax),%al */
+        { .opc = { endbr64, 0x02, 0x04, 0x04 }, /* add (%rsp,%rax),%al */
           .rax = 0xfedcba9876543210UL,
           .res.fields.trapnr = X86_EXC_SS },
-        { .opc = { endbr64, 0xcc, 0xc3, 0xc3, 0xc3 }, /* int3 */
+        { .opc = { endbr64, 0xcc, 0x90, 0x90 }, /* int3 */
           .res.fields.trapnr = X86_EXC_BP },
 #undef endbr64
     };
@@ -183,6 +183,7 @@ int __init cf_check stub_selftest(void)
 
         memset(ptr, 0xcc, STUB_BUF_SIZE / 2);
         memcpy(ptr, tests[i].opc, ARRAY_SIZE(tests[i].opc));
+        place_ret(ptr + ARRAY_SIZE(tests[i].opc));
         unmap_domain_page(ptr);
 
         asm volatile ( "INDIRECT_CALL %[stb]\n"
diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h
index ef63e257ad78..28d4ec0f9c4a 100644
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative.h
@@ -31,6 +31,8 @@ struct __packed alt_instr {
 #define ALT_REPL_PTR(a)     __ALT_PTR(a, repl_offset)
 
 extern void add_nops(void *insns, unsigned int len);
+void *place_ret(void *ptr);
+
 /* Similar to alternative_instructions except it can be run with IRQs enabled. */
 extern int apply_alternatives(struct alt_instr *start, struct alt_instr *end);
 extern void alternative_instructions(void);
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 70150c272276..ff5d1c9f8634 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -76,7 +76,6 @@ static io_emul_stub_t *io_emul_stub_setup(struct priv_op_ctxt *ctxt, u8 opcode,
         0x41, 0x5c, /* pop %r12  */
         0x5d,       /* pop %rbp  */
         0x5b,       /* pop %rbx  */
-        0xc3,       /* ret       */
     };
 
     const struct stubs *this_stubs = &this_cpu(stubs);
@@ -126,11 +125,13 @@ static io_emul_stub_t *io_emul_stub_setup(struct priv_op_ctxt *ctxt, u8 opcode,
 
     APPEND_CALL(save_guest_gprs);
     APPEND_BUFF(epilogue);
+    p = place_ret(p);
 
     /* Build-time best effort attempt to catch problems. */
     BUILD_BUG_ON(STUB_BUF_SIZE / 2 <
                  (sizeof(prologue) + sizeof(epilogue) + 10 /* 2x call */ +
-                  MAX(3 /* default stub */, IOEMUL_QUIRK_STUB_BYTES)));
+                  MAX(3 /* default stub */, IOEMUL_QUIRK_STUB_BYTES) +
+                  1 /* ret */));
     /* Runtime confirmation that we haven't clobbered an adjacent stub. */
     BUG_ON(STUB_BUF_SIZE / 2 < (p - ctxt->io_emul_stub));
 
diff --git a/xen/arch/x86/x86_emulate/fpu.c b/xen/arch/x86/x86_emulate/fpu.c
index 54c8621421c6..9cc37a1d8e3c 100644
--- a/xen/arch/x86/x86_emulate/fpu.c
+++ b/xen/arch/x86/x86_emulate/fpu.c
@@ -32,36 +32,42 @@ static inline bool fpu_check_write(void)
 
 #define emulate_fpu_insn_memdst(opc, ext, arg)                          \
 do {                                                                    \
+    void *_p = get_stub(stub);                                          \
     /* ModRM: mod=0, reg=ext, rm=0, i.e. a (%rax) operand */            \
     *insn_bytes = 2;                                                    \
-    memcpy(get_stub(stub),                                              \
-           ((uint8_t[]){ opc, ((ext) & 7) << 3, 0xc3 }), 3);            \
+    memcpy(_p, ((uint8_t[]){ opc, ((ext) & 7) << 3 }), 2); _p += 2;     \
+    place_ret(_p);                                                      \
     invoke_stub("", "", "+m" (arg) : "a" (&(arg)));                     \
     put_stub(stub);                                                     \
 } while (0)
 
 #define emulate_fpu_insn_memsrc(opc, ext, arg)                          \
 do {                                                                    \
+    void *_p = get_stub(stub);                                          \
     /* ModRM: mod=0, reg=ext, rm=0, i.e. a (%rax) operand */            \
-    memcpy(get_stub(stub),                                              \
-           ((uint8_t[]){ opc, ((ext) & 7) << 3, 0xc3 }), 3);            \
+    memcpy(_p, ((uint8_t[]){ opc, ((ext) & 7) << 3 }), 2); _p += 2;     \
+    place_ret(_p);                                                      \
     invoke_stub("", "", "=m" (dummy) : "m" (arg), "a" (&(arg)));        \
     put_stub(stub);                                                     \
 } while (0)
 
 #define emulate_fpu_insn_stub(bytes...)                                 \
 do {                                                                    \
+    void *_p = get_stub(stub);                                          \
     unsigned int nr_ = sizeof((uint8_t[]){ bytes });                    \
-    memcpy(get_stub(stub), ((uint8_t[]){ bytes, 0xc3 }), nr_ + 1);      \
+    memcpy(_p, ((uint8_t[]){ bytes }), nr_); _p += nr_;                 \
+    place_ret(_p);                                                      \
     invoke_stub("", "", "=m" (dummy) : "i" (0));                        \
     put_stub(stub);                                                     \
 } while (0)
 
 #define emulate_fpu_insn_stub_eflags(bytes...)                          \
 do {                                                                    \
+    void *_p = get_stub(stub);                                          \
     unsigned int nr_ = sizeof((uint8_t[]){ bytes });                    \
     unsigned long tmp_;                                                 \
-    memcpy(get_stub(stub), ((uint8_t[]){ bytes, 0xc3 }), nr_ + 1);      \
+    memcpy(_p, ((uint8_t[]){ bytes }), nr_); _p += nr_;                 \
+    place_ret(_p);                                                      \
     invoke_stub(_PRE_EFLAGS("[eflags]", "[mask]", "[tmp]"),             \
                 _POST_EFLAGS("[eflags]", "[mask]", "[tmp]"),            \
                 [eflags] "+g" (regs->eflags), [tmp] "=&r" (tmp_)        \
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index e8cba5a27f56..a6a1c9937f5d 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1398,7 +1398,7 @@ x86_emulate(
         stb[3] = 0x91;
         stb[4] = evex.opmsk << 3;
         insn_bytes = 5;
-        stb[5] = 0xc3;
+        place_ret(&stb[5]);
 
         invoke_stub("", "", "+m" (op_mask) : "a" (&op_mask));
 
@@ -3631,7 +3631,7 @@ x86_emulate(
         }
         opc[1] = (modrm & 0x38) | 0xc0;
         insn_bytes = EVEX_PFX_BYTES + 2;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         copy_EVEX(opc, evex);
         invoke_stub("", "", "=g" (dummy) : "a" (src.val));
@@ -3698,7 +3698,7 @@ x86_emulate(
             insn_bytes = PFX_BYTES + 2;
             copy_REX_VEX(opc, rex_prefix, vex);
         }
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         ea.reg = decode_gpr(&_regs, modrm_reg);
         invoke_stub("", "", "=a" (*ea.reg) : "c" (mmvalp), "m" (*mmvalp));
@@ -3772,7 +3772,7 @@ x86_emulate(
             insn_bytes = PFX_BYTES + 2;
             copy_REX_VEX(opc, rex_prefix, vex);
         }
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         _regs.eflags &= ~EFLAGS_MASK;
         invoke_stub("",
@@ -4008,7 +4008,7 @@ x86_emulate(
         opc[1] = modrm & 0xc7;
         insn_bytes = PFX_BYTES + 2;
     simd_0f_to_gpr:
-        opc[insn_bytes - PFX_BYTES] = 0xc3;
+        place_ret(&opc[insn_bytes - PFX_BYTES]);
 
         generate_exception_if(ea.type != OP_REG, X86_EXC_UD);
 
@@ -4405,7 +4405,7 @@ x86_emulate(
             vex.w = 0;
         opc[1] = modrm & 0x38;
         insn_bytes = PFX_BYTES + 2;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         copy_REX_VEX(opc, rex_prefix, vex);
         invoke_stub("", "", "+m" (src.val) : "a" (&src.val));
@@ -4442,7 +4442,7 @@ x86_emulate(
             evex.w = 0;
         opc[1] = modrm & 0x38;
         insn_bytes = EVEX_PFX_BYTES + 2;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         copy_EVEX(opc, evex);
         invoke_stub("", "", "+m" (src.val) : "a" (&src.val));
@@ -4637,7 +4637,7 @@ x86_emulate(
 #endif /* X86EMUL_NO_SIMD */
 
     simd_0f_reg_only:
-        opc[insn_bytes - PFX_BYTES] = 0xc3;
+        place_ret(&opc[insn_bytes - PFX_BYTES]);
 
         copy_REX_VEX(opc, rex_prefix, vex);
         invoke_stub("", "", [dummy_out] "=g" (dummy) : [dummy_in] "i" (0) );
@@ -4971,7 +4971,7 @@ x86_emulate(
         if ( !mode_64bit() )
             vex.w = 0;
         opc[1] = modrm & 0xf8;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         copy_VEX(opc, vex);
         ea.reg = decode_gpr(&_regs, modrm_rm);
@@ -5014,7 +5014,7 @@ x86_emulate(
         if ( !mode_64bit() )
             vex.w = 0;
         opc[1] = modrm & 0xc7;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         copy_VEX(opc, vex);
         invoke_stub("", "", "=a" (dst.val) : [dummy] "i" (0));
@@ -5044,7 +5044,7 @@ x86_emulate(
         opc = init_prefixes(stub);
         opc[0] = b;
         opc[1] = modrm;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         copy_VEX(opc, vex);
         _regs.eflags &= ~EFLAGS_MASK;
@@ -5612,7 +5612,7 @@ x86_emulate(
         if ( !mode_64bit() )
             vex.w = 0;
         opc[1] = modrm & 0xc7;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         copy_REX_VEX(opc, rex_prefix, vex);
         invoke_stub("", "", "=a" (ea.val) : [dummy] "i" (0));
@@ -5730,7 +5730,7 @@ x86_emulate(
             opc[1] &= 0x38;
         }
         insn_bytes = PFX_BYTES + 2;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
         if ( vex.opcx == vex_none )
         {
             /* Cover for extra prefix byte. */
@@ -6010,7 +6010,7 @@ x86_emulate(
         pvex->b = !mode_64bit() || (vex.reg >> 3);
         opc[1] = 0xc0 | (~vex.reg & 7);
         pvex->reg = 0xf;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         invoke_stub("", "", "=a" (ea.val) : [dummy] "i" (0));
         put_stub(stub);
@@ -6284,7 +6284,7 @@ x86_emulate(
             evex.w = 0;
         opc[1] = modrm & 0xf8;
         insn_bytes = EVEX_PFX_BYTES + 2;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         copy_EVEX(opc, evex);
         invoke_stub("", "", "=g" (dummy) : "a" (src.val));
@@ -6383,7 +6383,7 @@ x86_emulate(
         pvex->b = 1;
         opc[1] = (modrm_reg & 7) << 3;
         pvex->reg = 0xf;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         invoke_stub("", "", "=m" (*mmvalp) : "a" (mmvalp));
 
@@ -6453,7 +6453,7 @@ x86_emulate(
         pvex->b = 1;
         opc[1] = (modrm_reg & 7) << 3;
         pvex->reg = 0xf;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         invoke_stub("", "", "+m" (*mmvalp) : "a" (mmvalp));
 
@@ -6509,7 +6509,7 @@ x86_emulate(
         pevex->b = 1;
         opc[1] = (modrm_reg & 7) << 3;
         pevex->RX = 1;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         invoke_stub("", "", "=m" (*mmvalp) : "a" (mmvalp));
 
@@ -6574,7 +6574,7 @@ x86_emulate(
         pevex->b = 1;
         opc[1] = (modrm_reg & 7) << 3;
         pevex->RX = 1;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         invoke_stub("", "", "+m" (*mmvalp) : "a" (mmvalp));
 
@@ -6588,7 +6588,7 @@ x86_emulate(
         opc[2] = 0x90;
         /* Use (%rax) as source. */
         opc[3] = evex.opmsk << 3;
-        opc[4] = 0xc3;
+        place_ret(&opc[4]);
 
         invoke_stub("", "", "+m" (op_mask) : "a" (&op_mask));
         put_stub(stub);
@@ -6664,7 +6664,7 @@ x86_emulate(
         pevex->b = 1;
         opc[1] = (modrm_reg & 7) << 3;
         pevex->RX = 1;
-        opc[2] = 0xc3;
+        place_ret(&opc[2]);
 
         invoke_stub("", "", "=m" (*mmvalp) : "a" (mmvalp));
 
@@ -6741,7 +6741,7 @@ x86_emulate(
         opc[2] = 0x90;
         /* Use (%rax) as source. */
         opc[3] = evex.opmsk << 3;
-        opc[4] = 0xc3;
+        place_ret(&opc[4]);
 
         invoke_stub("", "", "+m" (op_mask) : "a" (&op_mask));
         put_stub(stub);
@@ -6940,7 +6940,7 @@ x86_emulate(
         pvex->reg = 0xf; /* rAX */
         buf[3] = b;
         buf[4] = 0x09; /* reg=rCX r/m=(%rCX) */
-        buf[5] = 0xc3;
+        place_ret(&buf[5]);
 
         src.reg = decode_vex_gpr(vex.reg, &_regs, ctxt);
         emulate_stub([dst] "=&c" (dst.val), "[dst]" (&src.val), "a" (*src.reg));
@@ -6976,7 +6976,7 @@ x86_emulate(
         pvex->reg = 0xf; /* rAX */
         buf[3] = b;
         buf[4] = (modrm & 0x38) | 0x01; /* r/m=(%rCX) */
-        buf[5] = 0xc3;
+        place_ret(&buf[5]);
 
         dst.reg = decode_vex_gpr(vex.reg, &_regs, ctxt);
         emulate_stub("=&a" (dst.val), "c" (&src.val));
@@ -7217,7 +7217,7 @@ x86_emulate(
             evex.w = vex.w = 0;
         opc[1] = modrm & 0x38;
         opc[2] = imm1;
-        opc[3] = 0xc3;
+        place_ret(&opc[3]);
         if ( vex.opcx == vex_none )
         {
             /* Cover for extra prefix byte. */
@@ -7384,7 +7384,7 @@ x86_emulate(
             insn_bytes = PFX_BYTES + 3;
             copy_VEX(opc, vex);
         }
-        opc[3] = 0xc3;
+        place_ret(&opc[3]);
 
         /* Latch MXCSR - we may need to restore it below. */
         invoke_stub("stmxcsr %[mxcsr]", "",
@@ -7630,7 +7630,7 @@ x86_emulate(
         }
         opc[2] = imm1;
         insn_bytes = PFX_BYTES + 3;
-        opc[3] = 0xc3;
+        place_ret(&opc[3]);
         if ( vex.opcx == vex_none )
         {
             /* Cover for extra prefix byte. */
@@ -7976,7 +7976,7 @@ x86_emulate(
         pxop->reg = 0xf; /* rAX */
         buf[3] = b;
         buf[4] = (modrm & 0x38) | 0x01; /* r/m=(%rCX) */
-        buf[5] = 0xc3;
+        place_ret(&buf[5]);
 
         dst.reg = decode_vex_gpr(vex.reg, &_regs, ctxt);
         emulate_stub([dst] "=&a" (dst.val), "c" (&src.val));
@@ -8085,7 +8085,7 @@ x86_emulate(
         buf[3] = b;
         buf[4] = 0x09; /* reg=rCX r/m=(%rCX) */
         *(uint32_t *)(buf + 5) = imm1;
-        buf[9] = 0xc3;
+        place_ret(&buf[9]);
 
         emulate_stub([dst] "=&c" (dst.val), "[dst]" (&src.val));
 
@@ -8181,12 +8181,12 @@ x86_emulate(
 
         if ( evex_encoded() )
         {
-            opc[insn_bytes - EVEX_PFX_BYTES] = 0xc3;
+            place_ret(&opc[insn_bytes - EVEX_PFX_BYTES]);
             copy_EVEX(opc, evex);
         }
         else
         {
-            opc[insn_bytes - PFX_BYTES] = 0xc3;
+            place_ret(&opc[insn_bytes - PFX_BYTES]);
             copy_REX_VEX(opc, rex_prefix, vex);
         }
 
@@ -8510,7 +8510,7 @@ int x86_emul_rmw(
         pvex->reg = 0xf; /* rAX */
         buf[3] = ctxt->opcode;
         buf[4] = 0x11; /* reg=rDX r/m=(%RCX) */
-        buf[5] = 0xc3;
+        place_ret(&buf[5]);
 
         *eflags &= ~EFLAGS_MASK;
         invoke_stub("",