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

 e8 <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 'new_size'
amount of bytes at that location.

The amount is up to 31 instructions if desired (which is
the size of the opaque member). If there is a need to NOP
more then: a) more 'struct livepatch_func' structures need
to be present, b) we have to implement a variable size
buffer (in the future), or c) first byte an unconditional
branch skipping the to be disabled code (of course provided
there are no branch targets in the middle).

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).

And introduce a general livepatch_insn_len inline function
that would depend on platform specific instruction size
(for a unconditional branch). As such we also rename the
PATCH_INSN_SIZE to ARCH_PATCH_INSN_SIZE.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>

---
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Ross Lagerwall <ross.lagerw...@citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>

v3: First submission
v4: Fix description - e9 -> e8
    Remove the restriction of only doing 5 or 4 bytes.
    Redo the patching code to deal with variable size of new_size.
    Expand the amount of bytes we can NOP.
    Move the PATCH_INSN_SIZE definition in platform specific headers
    Move the get_len to livepatch_get_insn_len inline function.
v5: s/PATCH_INSN_SIZE/ARCH_PATCH_INSN_SIZE/
    s/arch_livepatch_insn_len/livepatch_insn_len/
    s/size_t len/unsigned int len/
    Add in commit description the c) mechanism (insert an unconditional
    branch).
---
 docs/misc/livepatch.markdown      |  7 +++++--
 xen/arch/x86/alternative.c        |  2 +-
 xen/arch/x86/livepatch.c          | 40 +++++++++++++++++++++++++--------------
 xen/common/livepatch.c            |  3 ++-
 xen/include/asm-x86/alternative.h |  1 +
 xen/include/asm-x86/livepatch.h   | 21 ++++++++++++++++++++
 xen/include/xen/livepatch.h       |  9 +++++++++
 7 files changed, 65 insertions(+), 18 deletions(-)
 create mode 100644 xen/include/asm-x86/livepatch.h

diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown
index 81f4fc9..a8e70a8 100644
--- a/docs/misc/livepatch.markdown
+++ b/docs/misc/livepatch.markdown
@@ -320,10 +320,13 @@ 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** not 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_addr` is
+   zero then `new_size` determines how many instruction bytes to NOP (up to
+   opaque size modulo smallest platform instruction - 1 byte x86 and 4 bytes 
on ARM).
 
 * `version` is to be one.
 
diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index 05e3eb8..6eaa10f 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 725b3f6..0b8642b 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -12,8 +12,7 @@
 #include <xen/livepatch.h>
 
 #include <asm/nmi.h>
-
-#define PATCH_INSN_SIZE 5
+#include <asm/livepatch.h>
 
 int arch_livepatch_quiesce(void)
 {
@@ -31,11 +30,11 @@ void arch_livepatch_revive(void)
 
 int arch_livepatch_verify_func(const struct livepatch_func *func)
 {
-    /* No NOP patching yet. */
-    if ( !func->new_size )
+    /* If NOPing only do up to maximum amount we can put in the ->opaque. */
+    if ( !func->new_addr && func->new_size > sizeof(func->opaque) )
         return -EOPNOTSUPP;
 
-    if ( func->old_size < PATCH_INSN_SIZE )
+    if ( func->old_size < ARCH_PATCH_INSN_SIZE )
         return -EINVAL;
 
     return 0;
@@ -43,23 +42,36 @@ 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;
-
-    BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->opaque));
-    BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof(val)));
+    uint8_t insn[sizeof(func->opaque)];
+    unsigned int len;
 
     old_ptr = func->old_addr;
-    memcpy(func->opaque, old_ptr, PATCH_INSN_SIZE);
+    len = livepatch_insn_len(func);
+    if ( !len )
+        return;
+
+    memcpy(func->opaque, old_ptr, len);
+    if ( func->new_addr )
+    {
+        int32_t val;
+
+        BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE != (1 + sizeof(val)));
+
+        insn[0] = 0xe9;
+        val = func->new_addr - func->old_addr - ARCH_PATCH_INSN_SIZE;
+
+        memcpy(&insn[1], &val, sizeof(val));
+    }
+    else
+        add_nops(&insn, len);
 
-    *old_ptr++ = 0xe9; /* Relative jump */
-    val = func->new_addr - func->old_addr - PATCH_INSN_SIZE;
-    memcpy(old_ptr, &val, sizeof(val));
+    memcpy(old_ptr, insn, len);
 }
 
 void arch_livepatch_revert_jmp(const struct livepatch_func *func)
 {
-    memcpy(func->old_addr, func->opaque, PATCH_INSN_SIZE);
+    memcpy(func->old_addr, func->opaque, livepatch_insn_len(func));
 }
 
 /* Serialise the CPU pipeline. */
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 967985c..218b389 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -520,7 +520,8 @@ static int prepare_payload(struct payload *payload,
             return -EOPNOTSUPP;
         }
 
-        if ( !f->new_addr || !f->new_size )
+        /* 'old_addr', 'new_addr', 'new_size' can all be zero. */
+        if ( !f->old_size )
         {
             dprintk(XENLOG_ERR, LIVEPATCH "%s: Address or size fields are 
zero!\n",
                     elf->name);
diff --git a/xen/include/asm-x86/alternative.h 
b/xen/include/asm-x86/alternative.h
index 67fc0d2..db4f08e 100644
--- a/xen/include/asm-x86/alternative.h
+++ b/xen/include/asm-x86/alternative.h
@@ -27,6 +27,7 @@ struct alt_instr {
 #define ALT_ORIG_PTR(a)     __ALT_PTR(a, instr_offset)
 #define ALT_REPL_PTR(a)     __ALT_PTR(a, repl_offset)
 
+extern void add_nops(void *insns, unsigned int len);
 /* Similar to alternative_instructions except it can be run with IRQs enabled. 
*/
 extern void apply_alternatives(const struct alt_instr *start,
                                const struct alt_instr *end);
diff --git a/xen/include/asm-x86/livepatch.h b/xen/include/asm-x86/livepatch.h
new file mode 100644
index 0000000..5e04aa1
--- /dev/null
+++ b/xen/include/asm-x86/livepatch.h
@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#ifndef __XEN_X86_LIVEPATCH_H__
+#define __XEN_X86_LIVEPATCH_H__
+
+#define ARCH_PATCH_INSN_SIZE 5
+
+#endif /* __XEN_X86_LIVEPATCH_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 46b9fc2..b714fbc 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -68,7 +68,16 @@ int arch_livepatch_secure(const void *va, unsigned int 
pages, enum va_type types
 void arch_livepatch_init(void);
 
 #include <public/sysctl.h> /* For struct livepatch_func. */
+#include <asm/livepatch.h> /* For ARCH_PATCH_INSN_SIZE. */
 int arch_livepatch_verify_func(const struct livepatch_func *func);
+
+static inline size_t livepatch_insn_len(const struct livepatch_func *func)
+{
+    if ( !func->new_addr )
+        return func->new_size;
+
+    return ARCH_PATCH_INSN_SIZE;
+}
 /*
  * These functions are called around the critical region patching live code,
  * for an architecture to take make appropratie global state adjustments.
-- 
2.5.5


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

Reply via email to