diff mbox series

[v2,6/7] x86/alternative: Relocate all insn-relative fields

Message ID 20241002152725.1841575-7-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series x86/alternatives: Adjust all insn-relative fields | expand

Commit Message

Andrew Cooper Oct. 2, 2024, 3:27 p.m. UTC
Right now, relocation of displacements is restricted to finding 0xe8/e9 as the
first byte of the replacement, but this is overly restrictive.

Use x86_decode_lite() to find and adjust all insn-relative fields.

As with disp8's not leaving the replacemnet block, some disp32's don't either.
e.g. the RSB stuffing loop.  These stay unmodified.

For now, leave the altcall devirtualisation alone.  These require more care to
transform into the new scheme.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/alternative.c | 46 +++++++++++++++++++++++++++++++-------
 1 file changed, 38 insertions(+), 8 deletions(-)

Comments

Jan Beulich Oct. 8, 2024, 11:33 a.m. UTC | #1
On 02.10.2024 17:27, Andrew Cooper wrote:
> Right now, relocation of displacements is restricted to finding 0xe8/e9 as the
> first byte of the replacement, but this is overly restrictive.
> 
> Use x86_decode_lite() to find and adjust all insn-relative fields.
> 
> As with disp8's not leaving the replacemnet block, some disp32's don't either.
> e.g. the RSB stuffing loop.  These stay unmodified.
> 
> For now, leave the altcall devirtualisation alone.  These require more care to
> transform into the new scheme.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff mbox series

Patch

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index b745f112154a..da1a4af56377 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -264,10 +264,31 @@  static int init_or_livepatch _apply_alternatives(struct alt_instr *start,
 
         memcpy(buf, repl, a->repl_len);
 
+        /* Walk buf[] and adjust any insn-relative operands. */
+        if ( a->repl_len )
         {
-            /* 0xe8/0xe9 are relative branches; fix the offset. */
-            if ( a->repl_len >= 5 && (*buf & 0xfe) == 0xe8 )
+            uint8_t *ip = buf, *end = ip + a->repl_len;
+
+            for ( x86_decode_lite_t res; ip < end; ip += res.len )
             {
+                int32_t *d32;
+                const uint8_t *target;
+
+                res = x86_decode_lite(ip, end);
+
+                if ( res.len == 0 )
+                {
+                    printk("Alt for %ps [%*ph]\n"
+                           "  Unable to decode instruction at +%lu in alternative\n",
+                           ALT_ORIG_PTR(a), a->repl_len, repl, ip - repl);
+                    return -EINVAL;
+                }
+
+                if ( res.rel_sz != 4 )
+                    continue;
+
+                d32 = res.rel;
+
                 /*
                  * Detect the special case of indirect-to-direct branch patching:
                  * - replacement is a direct CALL/JMP (opcodes 0xE8/0xE9; already
@@ -337,14 +358,23 @@  static int init_or_livepatch _apply_alternatives(struct alt_instr *start,
                          */
                         goto skip_this_alternative;
                     }
+
+                    continue;
+                }
+
+                target = ip + res.len + *d32;
+
+                if ( target >= buf && target <= end )
+                {
+                    /*
+                     * Target doesn't leave the replacement block.  e.g. RSB
+                     * stuffing.  Leave it unmodified.
+                     */
+                    continue;
                 }
-                else if ( force && system_state < SYS_STATE_active )
-                    ASSERT_UNREACHABLE();
-                else
-                    *(int32_t *)(buf + 1) += repl - orig;
+
+                *d32 += repl - orig;
             }
-            else if ( force && system_state < SYS_STATE_active )
-                ASSERT_UNREACHABLE();
         }
 
         a->priv = 1;