[Xen-devel] [PATCH 1/2] xen/livepatch: Add a return value to load hooks

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

One use of load hooks is to perform a safety check of the system in its
quiesced state.  Any non-zero return value from a load hook will abort the
apply attempt.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
CC: Juergen Gross <jgross@xxxxxxxx>

For several years, the following patch in the series has been shipped in every
XenServer livepatch, implemented as a void load hook which modifies its return
address to skip to the end of apply_payload().  This method is distinctly less
hacky.
---
 xen/common/livepatch.c               | 30 +++++++++++++++++++-----------
 xen/include/xen/livepatch_payload.h  |  2 +-
 xen/test/livepatch/xen_hello_world.c | 12 +++++++++---
 3 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 7caa30c202..962647616a 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -1076,25 +1076,33 @@ static int apply_payload(struct payload *data)
      * temporarily disable the spin locks IRQ state checks.
      */
     spin_debug_disable();
-    for ( i = 0; i < data->n_load_funcs; i++ )
-        data->load_funcs[i]();
+    for ( i = 0; !rc && i < data->n_load_funcs; i++ )
+        rc = data->load_funcs[i]();
     spin_debug_enable();
 
+    if ( rc )
+        printk(XENLOG_ERR LIVEPATCH "%s: load_funcs[%u] failed: %d\n",
+               data->name, i, rc);
+
     ASSERT(!local_irq_is_enabled());
 
-    for ( i = 0; i < data->nfuncs; i++ )
-        arch_livepatch_apply(&data->funcs[i]);
+    if ( !rc )
+        for ( i = 0; i < data->nfuncs; i++ )
+            arch_livepatch_apply(&data->funcs[i]);
 
     arch_livepatch_revive();
 
-    /*
-     * We need RCU variant (which has barriers) in case we crash here.
-     * The applied_list is iterated by the trap code.
-     */
-    list_add_tail_rcu(&data->applied_list, &applied_list);
-    register_virtual_region(&data->region);
+    if ( !rc )
+    {
+        /*
+         * We need RCU variant (which has barriers) in case we crash here.
+         * The applied_list is iterated by the trap code.
+         */
+        list_add_tail_rcu(&data->applied_list, &applied_list);
+        register_virtual_region(&data->region);
+    }
 
-    return 0;
+    return rc;
 }
 
 static int revert_payload(struct payload *data)
diff --git a/xen/include/xen/livepatch_payload.h 
b/xen/include/xen/livepatch_payload.h
index 4a1a96d054..3788b52ddf 100644
--- a/xen/include/xen/livepatch_payload.h
+++ b/xen/include/xen/livepatch_payload.h
@@ -9,7 +9,7 @@
  * The following definitions are to be used in patches. They are taken
  * from kpatch.
  */
-typedef void livepatch_loadcall_t(void);
+typedef int livepatch_loadcall_t(void);
 typedef void livepatch_unloadcall_t(void);
 
 /*
diff --git a/xen/test/livepatch/xen_hello_world.c 
b/xen/test/livepatch/xen_hello_world.c
index 02f3f85dc0..d720001f07 100644
--- a/xen/test/livepatch/xen_hello_world.c
+++ b/xen/test/livepatch/xen_hello_world.c
@@ -16,9 +16,11 @@ static const char hello_world_patch_this_fnc[] = 
"xen_extra_version";
 extern const char *xen_hello_world(void);
 static unsigned int cnt;
 
-static void apply_hook(void)
+static int apply_hook(void)
 {
     printk(KERN_DEBUG "Hook executing.\n");
+
+    return 0;
 }
 
 static void revert_hook(void)
@@ -26,10 +28,14 @@ static void revert_hook(void)
     printk(KERN_DEBUG "Hook unloaded.\n");
 }
 
-static void  hi_func(void)
+static int hi_func(void)
 {
     printk(KERN_DEBUG "%s: Hi! (called %u times)\n", __func__, ++cnt);
+
+    return 0;
 };
+/* Safe to cast away the return value for this trivial case. */
+void (void_hi_func)(void) __attribute__((alias("hi_func")));
 
 static void check_fnc(void)
 {
@@ -43,7 +49,7 @@ LIVEPATCH_UNLOAD_HOOK(revert_hook);
 /* Imbalance here. Two load and three unload. */
 
 LIVEPATCH_LOAD_HOOK(hi_func);
-LIVEPATCH_UNLOAD_HOOK(hi_func);
+LIVEPATCH_UNLOAD_HOOK(void_hi_func);
 
 LIVEPATCH_UNLOAD_HOOK(check_fnc);
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel