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

Andrew Cooper posted 7 patches 1 month, 2 weeks ago
[PATCH v2 6/7] x86/alternative: Relocate all insn-relative fields
Posted by Andrew Cooper 1 month, 2 weeks ago
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(-)

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;
-- 
2.39.5


Re: [PATCH v2 6/7] x86/alternative: Relocate all insn-relative fields
Posted by Jan Beulich 1 month, 2 weeks ago
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>