[Xen-changelog] [xen staging] livepatch: Add support for apply|revert action replacement hooks

[patchbot@xxxxxxx]

commit ef87efee9d38b61624f25c1a056d386a70ba99aa
Author:     Pawel Wieczorkiewicz <wipawel@xxxxxxxxx>
AuthorDate: Tue Nov 26 10:07:54 2019 +0000
Commit:     Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
CommitDate: Fri Dec 13 14:45:32 2019 +0000

    livepatch: Add support for apply|revert action replacement hooks
    
    By default, in the quiescing zone, a livepatch payload is applied with
    apply_payload() and reverted with revert_payload() functions. Both of
    the functions receive the payload struct pointer as a parameter. The
    functions are also a place where standard 'load' and 'unload' module
    hooks are executed.
    
    To increase livepatching system's agility and provide more flexible
    long-term livepatch solution, allow to overwrite the default apply
    and revert action functions with hook-like supplied alternatives.
    The alternative functions are optional and the default functions are
    used by default.
    
    Since the alternative functions have direct access to the livepatch
    payload structure, they can better control context of the 'load' and
    'unload' hooks execution as well as exact instructions replacement
    workflows. They can be also easily extended to support extra features
    in the future.
    
    To simplify the alternative function generation move code responsible
    for payload and livepatch region registration outside of the function.
    That way it is guaranteed that the registration step occurs even for
    newly supplied functions.
    
    Signed-off-by: Pawel Wieczorkiewicz <wipawel@xxxxxxxxx>
    Reviewed-by: Petre Eftime <epetre@xxxxxxxxxx>
    Reviewed-by: Martin Pohlack <mpohlack@xxxxxxxxxx>
    Reviewed-by: Norbert Manthey <nmanthey@xxxxxxxxxx>
    Reviewed-by: Andra-Irina Paraschiv <andraprs@xxxxxxxxxx>
    Reviewed-by: Bjoern Doebel <doebel@xxxxxxxxxx>
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
    Reviewed-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
---
 docs/misc/livepatch.pandoc            |  23 ++++++++
 xen/common/livepatch.c                |  65 ++++++++++++++++++----
 xen/include/xen/livepatch_payload.h   |  10 ++++
 xen/test/livepatch/Makefile           |  10 +++-
 xen/test/livepatch/xen_action_hooks.c | 100 ++++++++++++++++++++++++++++++++++
 5 files changed, 197 insertions(+), 11 deletions(-)

diff --git a/docs/misc/livepatch.pandoc b/docs/misc/livepatch.pandoc
index 5e8e2ff6f6..3c635add92 100644
--- a/docs/misc/livepatch.pandoc
+++ b/docs/misc/livepatch.pandoc
@@ -275,6 +275,7 @@ The payload contains at least three sections:
  * `.livepatch.funcs` - which is an array of livepatch_func structures.
    and/or any of:
  * `.livepatch.hooks.{preapply,postapply,prerevert,postrevert}'
+ * `.livepatch.hooks.{apply,revert}`
    - which are a pointer to a hook function pointer.
 
  * `.livepatch.xen_depends` - which is an ELF Note that describes what Xen
@@ -356,6 +357,14 @@ met.
  * `.livepatch.hooks.{prerevert,postrevert}`
    - which are a pointer to a single hook function pointer.
 
+Finally, it optionally may also contain the address of apply or revert action
+hooks to be called instead of the default apply and revert payload actions
+(while all CPUs are kept in quiescing zone). These hooks do have access to
+payload structure.
+
+ * `.livepatch.hooks.{apply,revert}`
+   - which are a pointer to a single hook function pointer.
+
 ### Example of .livepatch.funcs
 
 A simple example of what a payload file can be:
@@ -469,6 +478,20 @@ The type definition of the function are as follow:
 
     typedef void livepatch_postcall_t(livepatch_payload_t *arg);
 
+#### .livepatch.hooks.apply and .livepatch.hooks.revert
+
+This section contains a pointer to a single function pointer to be executed
+instead of a default apply (or revert) action function. This is useful to
+replace or augment default behavior of the apply (or revert) action that
+requires all CPUs to be in the quiescing zone.
+This type of hooks do have access to payload structure.
+
+Each entry in this array is eight bytes.
+
+The type definition of the function are as follow:
+
+    typedef int livepatch_actioncall_t(livepatch_payload_t *arg);
+
 ### .livepatch.xen_depends, .livepatch.depends and .note.gnu.build-id
 
 To support dependencies checking and safe loading (to load the
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 8ca7487dd4..6c1b811c28 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -587,8 +587,11 @@ static int prepare_payload(struct payload *payload,
     LIVEPATCH_ASSIGN_MULTI_HOOK(elf, payload->unload_funcs, 
payload->n_unload_funcs, ".livepatch.hooks.unload");
 
     LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.apply.pre, 
".livepatch.hooks.preapply");
+    LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.apply.action, 
".livepatch.hooks.apply");
     LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.apply.post, 
".livepatch.hooks.postapply");
+
     LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.revert.pre, 
".livepatch.hooks.prerevert");
+    LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.revert.action, 
".livepatch.hooks.revert");
     LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.revert.post, 
".livepatch.hooks.postrevert");
 
     sec = livepatch_elf_sec_by_name(elf, ELF_BUILD_ID_NOTE);
@@ -1122,6 +1125,11 @@ static int apply_payload(struct payload *data)
 
     arch_livepatch_revive();
 
+    return 0;
+}
+
+static inline void apply_payload_tail(struct payload *data)
+{
     /*
      * We need RCU variant (which has barriers) in case we crash here.
      * The applied_list is iterated by the trap code.
@@ -1129,7 +1137,7 @@ static int apply_payload(struct payload *data)
     list_add_tail_rcu(&data->applied_list, &applied_list);
     register_virtual_region(&data->region);
 
-    return 0;
+    data->state = LIVEPATCH_STATE_APPLIED;
 }
 
 static int revert_payload(struct payload *data)
@@ -1162,6 +1170,11 @@ static int revert_payload(struct payload *data)
     ASSERT(!local_irq_is_enabled());
 
     arch_livepatch_revive();
+    return 0;
+}
+
+static inline void revert_payload_tail(struct payload *data)
+{
 
     /*
      * We need RCU variant (which has barriers) in case we crash here.
@@ -1171,7 +1184,7 @@ static int revert_payload(struct payload *data)
     unregister_virtual_region(&data->region);
 
     data->reverted = true;
-    return 0;
+    data->state = LIVEPATCH_STATE_CHECKED;
 }
 
 /*
@@ -1191,15 +1204,31 @@ static void livepatch_do_action(void)
     switch ( livepatch_work.cmd )
     {
     case LIVEPATCH_ACTION_APPLY:
-        rc = apply_payload(data);
+        if ( is_hook_enabled(data->hooks.apply.action) )
+        {
+            printk(XENLOG_INFO LIVEPATCH "%s: Calling apply action hook 
function\n", data->name);
+
+            rc = (*data->hooks.apply.action)(data);
+        }
+        else
+            rc = apply_payload(data);
+
         if ( rc == 0 )
-            data->state = LIVEPATCH_STATE_APPLIED;
+            apply_payload_tail(data);
         break;
 
     case LIVEPATCH_ACTION_REVERT:
-        rc = revert_payload(data);
+        if ( is_hook_enabled(data->hooks.revert.action) )
+        {
+            printk(XENLOG_INFO LIVEPATCH "%s: Calling revert action hook 
function\n", data->name);
+
+            rc = (*data->hooks.revert.action)(data);
+        }
+        else
+            rc = revert_payload(data);
+
         if ( rc == 0 )
-            data->state = LIVEPATCH_STATE_CHECKED;
+            revert_payload_tail(data);
         break;
 
     case LIVEPATCH_ACTION_REPLACE:
@@ -1210,9 +1239,17 @@ static void livepatch_do_action(void)
          */
         list_for_each_entry_safe_reverse ( other, tmp, &applied_list, 
applied_list )
         {
-            other->rc = revert_payload(other);
+            if ( is_hook_enabled(other->hooks.revert.action) )
+            {
+                printk(XENLOG_INFO LIVEPATCH "%s: Calling revert action hook 
function\n", other->name);
+
+                other->rc = (*other->hooks.revert.action)(other);
+            }
+            else
+                other->rc = revert_payload(other);
+
             if ( other->rc == 0 )
-                other->state = LIVEPATCH_STATE_CHECKED;
+                revert_payload_tail(other);
             else
             {
                 rc = -EINVAL;
@@ -1222,9 +1259,17 @@ static void livepatch_do_action(void)
 
         if ( rc == 0 )
         {
-            rc = apply_payload(data);
+            if ( is_hook_enabled(data->hooks.apply.action) )
+            {
+                printk(XENLOG_INFO LIVEPATCH "%s: Calling apply action hook 
function\n", data->name);
+
+                rc = (*data->hooks.apply.action)(data);
+            }
+            else
+                rc = apply_payload(data);
+
             if ( rc == 0 )
-                data->state = LIVEPATCH_STATE_APPLIED;
+                apply_payload_tail(data);
         }
         break;
 
diff --git a/xen/include/xen/livepatch_payload.h 
b/xen/include/xen/livepatch_payload.h
index cd20944cc4..ff16af0dd6 100644
--- a/xen/include/xen/livepatch_payload.h
+++ b/xen/include/xen/livepatch_payload.h
@@ -22,11 +22,13 @@ typedef void livepatch_loadcall_t(void);
 typedef void livepatch_unloadcall_t(void);
 
 typedef int livepatch_precall_t(livepatch_payload_t *arg);
+typedef int livepatch_actioncall_t(livepatch_payload_t *arg);
 typedef void livepatch_postcall_t(livepatch_payload_t *arg);
 
 struct livepatch_hooks {
     struct {
         livepatch_precall_t *const *pre;
+        livepatch_actioncall_t *const *action;
         livepatch_postcall_t *const *post;
     } apply, revert;
 };
@@ -91,6 +93,10 @@ struct payload {
     livepatch_precall_t *__attribute__((weak, used)) \
         const livepatch_preapply_data_##_fn 
__section(".livepatch.hooks.preapply") = _fn;
 
+#define LIVEPATCH_APPLY_HOOK(_fn) \
+    livepatch_actioncall_t *__attribute__((weak, used)) \
+        const livepatch_apply_data_##_fn __section(".livepatch.hooks.apply") = 
_fn;
+
 #define LIVEPATCH_POSTAPPLY_HOOK(_fn) \
     livepatch_postcall_t *__attribute__((weak, used)) \
         const livepatch_postapply_data_##_fn 
__section(".livepatch.hooks.postapply") = _fn;
@@ -99,6 +105,10 @@ struct payload {
     livepatch_precall_t *__attribute__((weak, used)) \
         const livepatch_prerevert_data_##_fn 
__section(".livepatch.hooks.prerevert") = _fn;
 
+#define LIVEPATCH_REVERT_HOOK(_fn) \
+    livepatch_actioncall_t *__attribute__((weak, used)) \
+        const livepatch_revert_data_##_fn __section(".livepatch.hooks.revert") 
= _fn;
+
 #define LIVEPATCH_POSTREVERT_HOOK(_fn) \
     livepatch_postcall_t *__attribute__((weak, used)) \
         const livepatch_postrevert_data_##_fn 
__section(".livepatch.hooks.postrevert") = _fn;
diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile
index a94bc48536..116e52e774 100644
--- a/xen/test/livepatch/Makefile
+++ b/xen/test/livepatch/Makefile
@@ -22,6 +22,7 @@ LIVEPATCH_NOP := xen_nop.livepatch
 LIVEPATCH_NO_XEN_BUILDID := xen_no_xen_buildid.livepatch
 LIVEPATCH_PREPOST_HOOKS := xen_prepost_hooks.livepatch
 LIVEPATCH_PREPOST_HOOKS_FAIL := xen_prepost_hooks_fail.livepatch
+LIVEPATCH_ACTION_HOOKS := xen_action_hooks.livepatch
 
 LIVEPATCHES += $(LIVEPATCH)
 LIVEPATCHES += $(LIVEPATCH_BYE)
@@ -30,6 +31,7 @@ LIVEPATCHES += $(LIVEPATCH_NOP)
 LIVEPATCHES += $(LIVEPATCH_NO_XEN_BUILDID)
 LIVEPATCHES += $(LIVEPATCH_PREPOST_HOOKS)
 LIVEPATCHES += $(LIVEPATCH_PREPOST_HOOKS_FAIL)
+LIVEPATCHES += $(LIVEPATCH_ACTION_HOOKS)
 
 LIVEPATCH_DEBUG_DIR ?= $(DEBUG_DIR)/xen-livepatch
 
@@ -144,6 +146,12 @@ xen_prepost_hooks_fail.o: config.h
 $(LIVEPATCH_PREPOST_HOOKS_FAIL): xen_prepost_hooks_fail.o 
xen_hello_world_func.o note.o xen_note.o
        $(LD) $(LDFLAGS) $(build_id_linker) -r -o 
$(LIVEPATCH_PREPOST_HOOKS_FAIL) $^
 
+xen_actions_hooks.o: config.h
+
+.PHONY: $(LIVEPATCH_ACTION_HOOKS)
+$(LIVEPATCH_ACTION_HOOKS): xen_action_hooks.o xen_hello_world_func.o note.o 
xen_note.o
+       $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_ACTION_HOOKS) $^
+
 .PHONY: livepatch
 livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP) 
$(LIVEPATCH_NO_XEN_BUILDID) \
-           $(LIVEPATCH_PREPOST_HOOKS) $(LIVEPATCH_PREPOST_HOOKS_FAIL)
+           $(LIVEPATCH_PREPOST_HOOKS) $(LIVEPATCH_PREPOST_HOOKS_FAIL) 
$(LIVEPATCH_ACTION_HOOKS)
diff --git a/xen/test/livepatch/xen_action_hooks.c 
b/xen/test/livepatch/xen_action_hooks.c
new file mode 100644
index 0000000000..a947afc41f
--- /dev/null
+++ b/xen/test/livepatch/xen_action_hooks.c
@@ -0,0 +1,100 @@
+/*
+ * Copyright (c) 2019 Amazon.com, Inc. or its affiliates. All rights reserved.
+ *
+ */
+
+#include "config.h"
+#include <xen/lib.h>
+#include <xen/types.h>
+#include <xen/version.h>
+#include <xen/livepatch.h>
+#include <xen/livepatch_payload.h>
+
+#include <public/sysctl.h>
+
+static const char hello_world_patch_this_fnc[] = "xen_extra_version";
+extern const char *xen_hello_world(void);
+
+static unsigned int apply_cnt;
+static unsigned int revert_cnt;
+
+static int apply_hook(livepatch_payload_t *payload)
+{
+    int i;
+
+    printk(KERN_DEBUG "%s: Hook starting.\n", __func__);
+
+    for (i = 0; i < payload->nfuncs; i++)
+    {
+        struct livepatch_func *func = &payload->funcs[i];
+
+        apply_cnt++;
+
+        printk(KERN_DEBUG "%s: applying: %s\n", __func__, func->name);
+    }
+
+    printk(KERN_DEBUG "%s: Hook done.\n", __func__);
+
+    return 0;
+}
+
+static int revert_hook(livepatch_payload_t *payload)
+{
+    int i;
+
+    printk(KERN_DEBUG "%s: Hook starting.\n", __func__);
+
+    for (i = 0; i < payload->nfuncs; i++)
+    {
+        struct livepatch_func *func = &payload->funcs[i];
+
+        revert_cnt++;
+
+        printk(KERN_DEBUG "%s: reverting: %s\n", __func__, func->name);
+    }
+
+    printk(KERN_DEBUG "%s: Hook done.\n", __func__);
+
+    return 0;
+}
+
+static void post_revert_hook(livepatch_payload_t *payload)
+{
+    int i;
+
+    printk(KERN_DEBUG "%s: Hook starting.\n", __func__);
+
+    for (i = 0; i < payload->nfuncs; i++)
+    {
+        struct livepatch_func *func = &payload->funcs[i];
+
+        printk(KERN_DEBUG "%s: reverted: %s\n", __func__, func->name);
+    }
+
+    BUG_ON(apply_cnt != 1 || revert_cnt != 1);
+    printk(KERN_DEBUG "%s: Hook done.\n", __func__);
+}
+
+LIVEPATCH_APPLY_HOOK(apply_hook);
+LIVEPATCH_REVERT_HOOK(revert_hook);
+
+LIVEPATCH_POSTREVERT_HOOK(post_revert_hook);
+
+struct livepatch_func __section(".livepatch.funcs") livepatch_xen_hello_world 
= {
+    .version = LIVEPATCH_PAYLOAD_VERSION,
+    .name = hello_world_patch_this_fnc,
+    .new_addr = xen_hello_world,
+    .old_addr = xen_extra_version,
+    .new_size = NEW_CODE_SZ,
+    .old_size = OLD_CODE_SZ,
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
generated by git-patchbot for /home/xen/git/xen.git#staging

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog