[Xen-devel] [PATCH v3 7/9] livepatch: NOP if func->new_[addr, size] is zero.

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

The NOP functionality will NOP any of the code at
the 'old_addr' or at 'name' if the 'new_addr' and 'new_size'
are both zero. The purpose of this is to NOP out calls, such as:

 e9 <4-bytes-offset>

(5 byte insn), or on ARM a 4 byte insn for branching.

We need the EIP of where we need to the NOP, and that can
be provided via the `old_addr` or `name`.

If the `old_addr` is provided we will NOP
5 instructions (on x86) at that location.

If `name` is provided with the symbol+0x<offset/<len>
we make sure that <len> is 5 (on x86) and upon retrieving the
EIP based on `name` will NOP that location.

While at it, also unify the code on x86 patching so
it is a bit simpler (instead of two seperate writes
just make it one memcpy).

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Cc: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

v3: First submission
---
 docs/misc/livepatch.markdown      |  6 ++++--
 xen/arch/x86/alternative.c        |  2 +-
 xen/arch/x86/livepatch.c          | 23 ++++++++++++++---------
 xen/common/livepatch.c            | 12 ++++++++----
 xen/include/asm-x86/alternative.h |  1 +
 5 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown
index f1a5147..590757d 100644
--- a/docs/misc/livepatch.markdown
+++ b/docs/misc/livepatch.markdown
@@ -320,10 +320,12 @@ The size of the structure is 64 bytes on 64-bit 
hypervisors. It will be
 
 * `new_addr` is the address of the function that is replacing the old
   function. The address is filled in during relocation. The value **MUST** be
-  the address of the new function in the file.
+  either the address of the new function in the file, or zero if we are NOPing 
out
+  at `old_addr` (and `new_size` **MUST** be zero).
 
 * `old_size` and `new_size` contain the sizes of the respective functions in 
bytes.
-   The value of `old_size` **MUST** not be zero.
+   The value of `old_size` **MUST** not be zero. If the value of `new_size` is 
_zero_
+   the code at location of `old_addr` (or `name` when resolved) will be NOPed 
out.
 
 * `version` is to be one.
 
diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index be40b13..fd8528e 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -101,7 +101,7 @@ static void __init arch_init_ideal_nops(void)
 }
 
 /* Use this to add nops to a buffer, then text_poke the whole buffer. */
-static void init_or_livepatch add_nops(void *insns, unsigned int len)
+void init_or_livepatch add_nops(void *insns, unsigned int len)
 {
     while ( len > 0 )
     {
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index df64b00..cabd0c1 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -34,10 +34,6 @@ int arch_verify_insn_length(unsigned long len)
 
 int arch_livepatch_verify_func(const struct livepatch_func *func)
 {
-    /* No NOP patching yet. */
-    if ( !func->new_size )
-        return -EOPNOTSUPP;
-
     if ( func->old_size < PATCH_INSN_SIZE )
         return -EINVAL;
 
@@ -46,18 +42,27 @@ int arch_livepatch_verify_func(const struct livepatch_func 
*func)
 
 void arch_livepatch_apply_jmp(struct livepatch_func *func)
 {
-    int32_t val;
     uint8_t *old_ptr;
+    uint8_t insn[PATCH_INSN_SIZE];
 
     BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->opaque));
-    BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof(val)));
 
     old_ptr = func->old_addr;
     memcpy(func->opaque, old_ptr, PATCH_INSN_SIZE);
 
-    *old_ptr++ = 0xe9; /* Relative jump */
-    val = func->new_addr - func->old_addr - PATCH_INSN_SIZE;
-    memcpy(old_ptr, &val, sizeof(val));
+    if ( func->new_size )
+    {
+        int32_t val;
+
+        BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof(val)));
+
+        insn[0] = 0xe9;
+        val = func->new_addr - func->old_addr - PATCH_INSN_SIZE;
+        memcpy(&insn[1], &val, sizeof(val));
+    } else
+        add_nops(&insn, PATCH_INSN_SIZE);
+
+    memcpy(old_ptr, insn, PATCH_INSN_SIZE);
 }
 
 void arch_livepatch_revert_jmp(const struct livepatch_func *func)
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index e752949..6f82a9e 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -561,11 +561,15 @@ static int prepare_payload(struct payload *payload,
             return -EOPNOTSUPP;
         }
 
-        if ( !f->new_addr || !f->new_size )
+        /* If both are zero then we are NOPing. */
+        if ( (!f->new_addr || !f->new_size) )
         {
-            dprintk(XENLOG_ERR, LIVEPATCH "%s: Address or size fields are 
zero!\n",
-                    elf->name);
-            return -EINVAL;
+            if ( f->new_addr || f->new_size )
+            {
+                dprintk(XENLOG_ERR, LIVEPATCH "%s: Address or size fields are 
zero!\n",
+                        elf->name);
+                return -EINVAL;
+            }
         }
 
         rc = arch_livepatch_verify_func(f);
diff --git a/xen/include/asm-x86/alternative.h 
b/xen/include/asm-x86/alternative.h
index bce959f..acaeded 100644
--- a/xen/include/asm-x86/alternative.h
+++ b/xen/include/asm-x86/alternative.h
@@ -23,6 +23,7 @@ struct alt_instr {
     u8  replacementlen;     /* length of new instruction, <= instrlen */
 };
 
+extern void add_nops(void *insns, unsigned int len);
 /* Similar to apply_alternatives except it can be run with IRQs enabled. */
 extern void apply_alternatives_nocheck(struct alt_instr *start,
                                        struct alt_instr *end);
-- 
2.4.11


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel