[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] [xen staging] livepatch: Do not enforce ELF_LIVEPATCH_FUNC section presence



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

    livepatch: Do not enforce ELF_LIVEPATCH_FUNC section presence
    
    With default implementation the ELF_LIVEPATCH_FUNC section containing
    all functions to be replaced or added must be part of the livepatch
    payload, otherwise the payload is rejected (with -EINVAL).
    
    However, with the extended hooks implementation, a livepatch may be
    constructed of only hooks to perform certain actions without any code
    to be added or replaced.
    Therefore, do not always expect the functions section and allow it to
    be missing, provided there is at least one section containing hooks
    present. The functions section, when present in a payload, must be a
    single, non-empty section.
    
    Check also all extended hooks sections if they are a single, non-empty
    sections each.
    
    At least one of the functions or hooks section must be present in a
    valid payload.
    
    Signed-off-by: Pawel Wieczorkiewicz <wipawel@xxxxxxxxx>
    Reviewed-by: Andra-Irina Paraschiv <andraprs@xxxxxxxxxx>
    Reviewed-by: Bjoern Doebel <doebel@xxxxxxxxx>
    Reviewed-by: Martin Pohlack <mpohlack@xxxxxxxxx>
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
    Reviewed-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
---
 xen/common/livepatch.c                       | 147 +++++++++++++++++++--------
 xen/include/xen/livepatch.h                  |   8 ++
 xen/test/livepatch/Makefile                  |   9 +-
 xen/test/livepatch/xen_action_hooks_nofunc.c |  86 ++++++++++++++++
 4 files changed, 206 insertions(+), 44 deletions(-)

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 6c1b811c28..add7da7fa9 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -467,8 +467,7 @@ static int xen_build_id_dep(const struct payload *payload)
 static int check_special_sections(const struct livepatch_elf *elf)
 {
     unsigned int i;
-    static const char *const names[] = { ELF_LIVEPATCH_FUNC,
-                                         ELF_LIVEPATCH_DEPENDS,
+    static const char *const names[] = { ELF_LIVEPATCH_DEPENDS,
                                          ELF_LIVEPATCH_XEN_DEPENDS,
                                          ELF_BUILD_ID_NOTE};
     DECLARE_BITMAP(found, ARRAY_SIZE(names)) = { 0 };
@@ -503,6 +502,64 @@ static int check_special_sections(const struct 
livepatch_elf *elf)
     return 0;
 }
 
+static int check_patching_sections(const struct livepatch_elf *elf)
+{
+    unsigned int i;
+    static const char *const names[] = { ELF_LIVEPATCH_FUNC,
+                                         ELF_LIVEPATCH_LOAD_HOOKS,
+                                         ELF_LIVEPATCH_UNLOAD_HOOKS,
+                                         ELF_LIVEPATCH_PREAPPLY_HOOK,
+                                         ELF_LIVEPATCH_APPLY_HOOK,
+                                         ELF_LIVEPATCH_POSTAPPLY_HOOK,
+                                         ELF_LIVEPATCH_PREREVERT_HOOK,
+                                         ELF_LIVEPATCH_REVERT_HOOK,
+                                         ELF_LIVEPATCH_POSTREVERT_HOOK};
+    DECLARE_BITMAP(found, ARRAY_SIZE(names)) = { 0 };
+
+    /*
+     * The patching sections are optional, but at least one
+     * must be present. Otherwise, there is nothing to do.
+     * All the existing sections must not be empty and must
+     * be present at most once.
+     */
+    for ( i = 0; i < ARRAY_SIZE(names); i++ )
+    {
+        const struct livepatch_elf_sec *sec;
+
+        sec = livepatch_elf_sec_by_name(elf, names[i]);
+        if ( !sec )
+        {
+            dprintk(XENLOG_DEBUG, LIVEPATCH "%s: %s is missing\n",
+                    elf->name, names[i]);
+            continue; /* This section is optional */
+        }
+
+        if ( !sec->sec->sh_size )
+        {
+            printk(XENLOG_ERR LIVEPATCH "%s: %s is empty\n",
+                   elf->name, names[i]);
+            return -EINVAL;
+        }
+
+        if ( test_and_set_bit(i, found) )
+        {
+            printk(XENLOG_ERR LIVEPATCH "%s: %s was seen more than once\n",
+                   elf->name, names[i]);
+            return -EINVAL;
+        }
+    }
+
+    /* Checking if at least one section is present. */
+    if ( bitmap_empty(found, ARRAY_SIZE(names)) )
+    {
+        printk(XENLOG_ERR LIVEPATCH "%s: Nothing to patch. Aborting...\n",
+               elf->name);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 /*
  * Lookup specified section and when exists assign its address to a specified 
hook.
  * Perform section pointer and size validation: single hook sections must 
contain a
@@ -542,57 +599,59 @@ static int prepare_payload(struct payload *payload,
     const Elf_Note *n;
 
     sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_FUNC);
-    ASSERT(sec);
-    if ( !section_ok(elf, sec, sizeof(*payload->funcs)) )
-        return -EINVAL;
-
-    payload->funcs = sec->load_addr;
-    payload->nfuncs = sec->sec->sh_size / sizeof(*payload->funcs);
-
-    for ( i = 0; i < payload->nfuncs; i++ )
+    if ( sec )
     {
-        int rc;
+        if ( !section_ok(elf, sec, sizeof(*payload->funcs)) )
+            return -EINVAL;
 
-        f = &(payload->funcs[i]);
+        payload->funcs = sec->load_addr;
+        payload->nfuncs = sec->sec->sh_size / sizeof(*payload->funcs);
 
-        if ( f->version != LIVEPATCH_PAYLOAD_VERSION )
+        for ( i = 0; i < payload->nfuncs; i++ )
         {
-            printk(XENLOG_ERR LIVEPATCH "%s: Wrong version (%u). Expected 
%d\n",
-                   elf->name, f->version, LIVEPATCH_PAYLOAD_VERSION);
-            return -EOPNOTSUPP;
-        }
+            int rc;
 
-        /* 'old_addr', 'new_addr', 'new_size' can all be zero. */
-        if ( !f->old_size )
-        {
-            printk(XENLOG_ERR LIVEPATCH "%s: Address or size fields are 
zero\n",
-                   elf->name);
-            return -EINVAL;
-        }
+            f = &(payload->funcs[i]);
 
-        rc = arch_livepatch_verify_func(f);
-        if ( rc )
-            return rc;
+            if ( f->version != LIVEPATCH_PAYLOAD_VERSION )
+            {
+                printk(XENLOG_ERR LIVEPATCH "%s: Wrong version (%u). Expected 
%d\n",
+                       elf->name, f->version, LIVEPATCH_PAYLOAD_VERSION);
+                return -EOPNOTSUPP;
+            }
 
-        rc = resolve_old_address(f, elf);
-        if ( rc )
-            return rc;
+            /* 'old_addr', 'new_addr', 'new_size' can all be zero. */
+            if ( !f->old_size )
+            {
+                printk(XENLOG_ERR LIVEPATCH "%s: Address or size fields are 
zero\n",
+                       elf->name);
+                return -EINVAL;
+            }
 
-        rc = livepatch_verify_distance(f);
-        if ( rc )
-            return rc;
+            rc = arch_livepatch_verify_func(f);
+            if ( rc )
+                return rc;
+
+            rc = resolve_old_address(f, elf);
+            if ( rc )
+                return rc;
+
+            rc = livepatch_verify_distance(f);
+            if ( rc )
+                return rc;
+        }
     }
 
-    LIVEPATCH_ASSIGN_MULTI_HOOK(elf, payload->load_funcs, 
payload->n_load_funcs, ".livepatch.hooks.load");
-    LIVEPATCH_ASSIGN_MULTI_HOOK(elf, payload->unload_funcs, 
payload->n_unload_funcs, ".livepatch.hooks.unload");
+    LIVEPATCH_ASSIGN_MULTI_HOOK(elf, payload->load_funcs, 
payload->n_load_funcs, ELF_LIVEPATCH_LOAD_HOOKS);
+    LIVEPATCH_ASSIGN_MULTI_HOOK(elf, payload->unload_funcs, 
payload->n_unload_funcs, ELF_LIVEPATCH_UNLOAD_HOOKS);
 
-    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.apply.pre, 
ELF_LIVEPATCH_PREAPPLY_HOOK);
+    LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.apply.action, 
ELF_LIVEPATCH_APPLY_HOOK);
+    LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.apply.post, 
ELF_LIVEPATCH_POSTAPPLY_HOOK);
 
-    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");
+    LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.revert.pre, 
ELF_LIVEPATCH_PREREVERT_HOOK);
+    LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.revert.action, 
ELF_LIVEPATCH_REVERT_HOOK);
+    LIVEPATCH_ASSIGN_SINGLE_HOOK(elf, payload->hooks.revert.post, 
ELF_LIVEPATCH_POSTREVERT_HOOK);
 
     sec = livepatch_elf_sec_by_name(elf, ELF_BUILD_ID_NOTE);
     if ( sec )
@@ -786,8 +845,6 @@ static int build_symbol_table(struct payload *payload,
     struct livepatch_symbol *symtab;
     char *strtab;
 
-    ASSERT(payload->nfuncs);
-
     /* Recall that section @0 is always NULL. */
     for ( i = 1; i < elf->nsym; i++ )
     {
@@ -904,6 +961,10 @@ static int load_payload_data(struct payload *payload, void 
*raw, size_t len)
     if ( rc )
         goto out;
 
+    rc = check_patching_sections(&elf);
+    if ( rc )
+        goto out;
+
     rc = prepare_payload(payload, &elf);
     if ( rc )
         goto out;
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 057a46bda3..3a91626a79 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -33,6 +33,14 @@ struct xen_sysctl_livepatch_op;
 #define ELF_LIVEPATCH_DEPENDS     ".livepatch.depends"
 #define ELF_LIVEPATCH_XEN_DEPENDS ".livepatch.xen_depends"
 #define ELF_BUILD_ID_NOTE         ".note.gnu.build-id"
+#define ELF_LIVEPATCH_LOAD_HOOKS      ".livepatch.hooks.load"
+#define ELF_LIVEPATCH_UNLOAD_HOOKS    ".livepatch.hooks.unload"
+#define ELF_LIVEPATCH_PREAPPLY_HOOK   ".livepatch.hooks.preapply"
+#define ELF_LIVEPATCH_APPLY_HOOK      ".livepatch.hooks.apply"
+#define ELF_LIVEPATCH_POSTAPPLY_HOOK  ".livepatch.hooks.postapply"
+#define ELF_LIVEPATCH_PREREVERT_HOOK  ".livepatch.hooks.prerevert"
+#define ELF_LIVEPATCH_REVERT_HOOK     ".livepatch.hooks.revert"
+#define ELF_LIVEPATCH_POSTREVERT_HOOK ".livepatch.hooks.postrevert"
 /* Arbitrary limit for payload size and .bss section size. */
 #define LIVEPATCH_MAX_SIZE     MB(2)
 
diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile
index 116e52e774..bbc6bdaf64 100644
--- a/xen/test/livepatch/Makefile
+++ b/xen/test/livepatch/Makefile
@@ -23,6 +23,7 @@ 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
+LIVEPATCH_ACTION_HOOKS_NOFUNC := xen_action_hooks_nofunc.livepatch
 
 LIVEPATCHES += $(LIVEPATCH)
 LIVEPATCHES += $(LIVEPATCH_BYE)
@@ -32,6 +33,7 @@ LIVEPATCHES += $(LIVEPATCH_NO_XEN_BUILDID)
 LIVEPATCHES += $(LIVEPATCH_PREPOST_HOOKS)
 LIVEPATCHES += $(LIVEPATCH_PREPOST_HOOKS_FAIL)
 LIVEPATCHES += $(LIVEPATCH_ACTION_HOOKS)
+LIVEPATCHES += $(LIVEPATCH_ACTION_HOOKS_NOFUNC)
 
 LIVEPATCH_DEBUG_DIR ?= $(DEBUG_DIR)/xen-livepatch
 
@@ -152,6 +154,11 @@ xen_actions_hooks.o: config.h
 $(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_ACTION_HOOKS_NOFUNC)
+$(LIVEPATCH_ACTION_HOOKS_NOFUNC): xen_action_hooks_nofunc.o note.o xen_note.o
+       $(LD) $(LDFLAGS) $(build_id_linker) -r -o 
$(LIVEPATCH_ACTION_HOOKS_NOFUNC) $^
+
 .PHONY: livepatch
 livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP) 
$(LIVEPATCH_NO_XEN_BUILDID) \
-           $(LIVEPATCH_PREPOST_HOOKS) $(LIVEPATCH_PREPOST_HOOKS_FAIL) 
$(LIVEPATCH_ACTION_HOOKS)
+           $(LIVEPATCH_PREPOST_HOOKS) $(LIVEPATCH_PREPOST_HOOKS_FAIL) 
$(LIVEPATCH_ACTION_HOOKS) \
+           $(LIVEPATCH_ACTION_HOOKS_NOFUNC)
diff --git a/xen/test/livepatch/xen_action_hooks_nofunc.c 
b/xen/test/livepatch/xen_action_hooks_nofunc.c
new file mode 100644
index 0000000000..2b4e90436f
--- /dev/null
+++ b/xen/test/livepatch/xen_action_hooks_nofunc.c
@@ -0,0 +1,86 @@
+/*
+ * 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 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 > 0 || revert_cnt > 0);
+    printk(KERN_DEBUG "%s: Hook done.\n", __func__);
+}
+
+LIVEPATCH_APPLY_HOOK(apply_hook);
+LIVEPATCH_REVERT_HOOK(revert_hook);
+
+LIVEPATCH_POSTREVERT_HOOK(post_revert_hook);
+
+/*
+ * 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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.