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

[Xen-changelog] [xen staging] livepatch: Always check hypervisor build ID upon livepatch upload



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

    livepatch: Always check hypervisor build ID upon livepatch upload
    
    This change is part of a independant stacked livepatch modules
    feature. This feature allows to bypass dependencies between modules
    upon loading, but still verifies Xen build ID matching.
    
    In order to prevent (up)loading any livepatches built for different
    hypervisor version as indicated by the Xen Build ID, add checking for
    the payload's vs Xen's build id match.
    
    To achieve that embed into every livepatch another section with a
    dedicated hypervisor build id in it. After the payload is loaded and
    the .livepatch.xen_depends section becomes available, perform the
    check and reject the payload if there is no match.
    
    Signed-off-by: Pawel Wieczorkiewicz <wipawel@xxxxxxxxx>
    Reviewed-by: Andra-Irina Paraschiv <andraprs@xxxxxxxxxx>
    Reviewed-by: Bjoern Doebel <doebel@xxxxxxxxx>
    Reviewed-by: Eslam Elnikety <elnikety@xxxxxxxxx>
    Reviewed-by: Martin Pohlack <mpohlack@xxxxxxxxx>
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
    Reviewed-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
---
 .gitignore                  |  1 +
 docs/misc/livepatch.pandoc  | 28 +++++++++++++++++++--------
 xen/common/livepatch.c      | 47 +++++++++++++++++++++++++++++++++++++++++++++
 xen/include/xen/livepatch.h |  7 ++++---
 xen/test/livepatch/Makefile | 31 +++++++++++++++++++++++++-----
 5 files changed, 98 insertions(+), 16 deletions(-)

diff --git a/.gitignore b/.gitignore
index 3ada0c4f0b..223bedcd2b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -312,6 +312,7 @@ xen/test/livepatch/xen_bye_world.livepatch
 xen/test/livepatch/xen_hello_world.livepatch
 xen/test/livepatch/xen_nop.livepatch
 xen/test/livepatch/xen_replace_world.livepatch
+xen/test/livepatch/xen_no_xen_buildid.livepatch
 xen/tools/kconfig/.tmp_gtkcheck
 xen/tools/kconfig/.tmp_qtcheck
 xen/tools/symbols
diff --git a/docs/misc/livepatch.pandoc b/docs/misc/livepatch.pandoc
index 6d9f72f49b..fd1f5d0126 100644
--- a/docs/misc/livepatch.pandoc
+++ b/docs/misc/livepatch.pandoc
@@ -270,6 +270,8 @@ like what the Linux kernel module loader does.
 The payload contains at least three sections:
 
  * `.livepatch.funcs` - which is an array of livepatch_func structures.
+ * `.livepatch.xen_depends` - which is an ELF Note that describes what Xen
+    build-id the payload depends on. **MUST** have one.
  * `.livepatch.depends` - which is an ELF Note that describes what the payload
     depends on. **MUST** have one.
  *  `.note.gnu.build-id` - the build-id of this payload. **MUST** have one.
@@ -383,16 +385,16 @@ The type definition of the function are as follow:
     typedef void (*livepatch_loadcall_t)(void);
     typedef void (*livepatch_unloadcall_t)(void);
 
-### .livepatch.depends and .note.gnu.build-id
+### .livepatch.xen_depends, .livepatch.depends and .note.gnu.build-id
 
 To support dependencies checking and safe loading (to load the
 appropiate payload against the right hypervisor) there is a need
 to embbed an build-id dependency.
 
-This is done by the payload containing an section `.livepatch.depends`
-which follows the format of an ELF Note. The contents of this
-(name, and description) are specific to the linker utilized to
-build the hypevisor and payload.
+This is done by the payload containing sections `.livepatch.xen_depends`
+and `.livepatch.depends` which follow the format of an ELF Note.
+The contents of these (name, and description) are specific to the linker
+utilized to build the hypevisor and payload.
 
 If GNU linker is used then the name is `GNU` and the description
 is a NT_GNU_BUILD_ID type ID. The description can be an SHA1
@@ -400,6 +402,13 @@ checksum, MD5 checksum or any unique value.
 
 The size of these structures varies with the `--build-id` linker option.
 
+There are two kinds of build-id dependencies:
+
+ * Xen build-id dependency (.livepatch.xen_depends section)
+ * previous payload build-id dependency (.livepatch.depends section)
+
+See "Live patch interdependencies" for more information.
+
 ## Hypercalls
 
 We will employ the sub operations of the system management hypercall (sysctl).
@@ -894,13 +903,16 @@ but is more complex to implement.
 The second option which requires an build-id of the hypervisor
 is implemented in the Xen hypervisor.
 
-Specifically each payload has two build-id ELF notes:
+Specifically each payload has three build-id ELF notes:
  * The build-id of the payload itself (generated via --build-id).
+ * The build-id of the Xen hypervisor it depends on (extracted from the
+   hypervisor during build time).
  * The build-id of the payload it depends on (extracted from the
    the previous payload or hypervisor during build time).
 
-This means that the very first payload depends on the hypervisor
-build-id.
+This means that every payload depends on the hypervisor build-id and on
+the build-id of the previous payload in the stack.
+The very first payload depends on the hypervisor build-id only.
 
 # Not Yet Done
 
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 7ab1f82290..a0e0d3092b 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -74,6 +74,7 @@ struct payload {
     unsigned int nsyms;                  /* Nr of entries in .strtab and 
symbols. */
     struct livepatch_build_id id;        /* ELFNOTE_DESC(.note.gnu.build-id) 
of the payload. */
     struct livepatch_build_id dep;       /* ELFNOTE_DESC(.livepatch.depends). 
*/
+    struct livepatch_build_id xen_dep;   /* 
ELFNOTE_DESC(.livepatch.xen_depends). */
     livepatch_loadcall_t *const *load_funcs;   /* The array of funcs to call 
after */
     livepatch_unloadcall_t *const *unload_funcs;/* load and unload of the 
payload. */
     unsigned int n_load_funcs;           /* Nr of the funcs to load and 
execute. */
@@ -476,11 +477,34 @@ static bool section_ok(const struct livepatch_elf *elf,
     return true;
 }
 
+static int xen_build_id_dep(const struct payload *payload)
+{
+    const void *id = NULL;
+    unsigned int len = 0;
+    int rc;
+
+    ASSERT(payload->xen_dep.len);
+    ASSERT(payload->xen_dep.p);
+
+    rc = xen_build_id(&id, &len);
+    if ( rc )
+        return rc;
+
+    if ( payload->xen_dep.len != len || memcmp(id, payload->xen_dep.p, len) ) {
+        printk(XENLOG_ERR LIVEPATCH "%s: check against hypervisor build-id 
failed\n",
+               payload->name);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 static int check_special_sections(const struct livepatch_elf *elf)
 {
     unsigned int i;
     static const char *const names[] = { ELF_LIVEPATCH_FUNC,
                                          ELF_LIVEPATCH_DEPENDS,
+                                         ELF_LIVEPATCH_XEN_DEPENDS,
                                          ELF_BUILD_ID_NOTE};
     DECLARE_BITMAP(found, ARRAY_SIZE(names)) = { 0 };
 
@@ -632,6 +656,22 @@ static int prepare_payload(struct payload *payload,
             return -EINVAL;
     }
 
+    sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_XEN_DEPENDS);
+    if ( sec )
+    {
+        n = sec->load_addr;
+
+        if ( sec->sec->sh_size <= sizeof(*n) )
+            return -EINVAL;
+
+        if ( xen_build_id_check(n, sec->sec->sh_size,
+                                &payload->xen_dep.p, &payload->xen_dep.len) )
+            return -EINVAL;
+
+        if ( !payload->xen_dep.len || !payload->xen_dep.p )
+            return -EINVAL;
+    }
+
     /* Setup the virtual region with proper data. */
     region = &payload->region;
 
@@ -882,6 +922,10 @@ static int load_payload_data(struct payload *payload, void 
*raw, size_t len)
     if ( rc )
         goto out;
 
+    rc = xen_build_id_dep(payload);
+    if ( rc )
+        goto out;
+
     rc = build_symbol_table(payload, &elf);
     if ( rc )
         goto out;
@@ -1663,6 +1707,9 @@ static void livepatch_printall(unsigned char key)
 
         if ( data->dep.len )
             printk("depend-on=%*phN\n", data->dep.len, data->dep.p);
+
+        if ( data->xen_dep.len )
+            printk("depend-on-xen=%*phN\n", data->xen_dep.len, 
data->xen_dep.p);
     }
 
     spin_unlock(&payload_lock);
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 69ede75d20..057a46bda3 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -29,9 +29,10 @@ struct xen_sysctl_livepatch_op;
 /* Convenience define for printk. */
 #define LIVEPATCH             "livepatch: "
 /* ELF payload special section names. */
-#define ELF_LIVEPATCH_FUNC    ".livepatch.funcs"
-#define ELF_LIVEPATCH_DEPENDS ".livepatch.depends"
-#define ELF_BUILD_ID_NOTE      ".note.gnu.build-id"
+#define ELF_LIVEPATCH_FUNC        ".livepatch.funcs"
+#define ELF_LIVEPATCH_DEPENDS     ".livepatch.depends"
+#define ELF_LIVEPATCH_XEN_DEPENDS ".livepatch.xen_depends"
+#define ELF_BUILD_ID_NOTE         ".note.gnu.build-id"
 /* 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 6831383db1..938aee17ec 100644
--- a/xen/test/livepatch/Makefile
+++ b/xen/test/livepatch/Makefile
@@ -19,11 +19,13 @@ LIVEPATCH := xen_hello_world.livepatch
 LIVEPATCH_BYE := xen_bye_world.livepatch
 LIVEPATCH_REPLACE := xen_replace_world.livepatch
 LIVEPATCH_NOP := xen_nop.livepatch
+LIVEPATCH_NO_XEN_BUILDID := xen_no_xen_buildid.livepatch
 
 LIVEPATCHES += $(LIVEPATCH)
 LIVEPATCHES += $(LIVEPATCH_BYE)
 LIVEPATCHES += $(LIVEPATCH_REPLACE)
 LIVEPATCHES += $(LIVEPATCH_NOP)
+LIVEPATCHES += $(LIVEPATCH_NO_XEN_BUILDID)
 
 LIVEPATCH_DEBUG_DIR ?= $(DEBUG_DIR)/xen-livepatch
 
@@ -59,7 +61,7 @@ config.h: xen_hello_world_func.o
 xen_hello_world.o: config.h
 
 .PHONY: $(LIVEPATCH)
-$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o
+$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o xen_note.o
        $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH) $^
 
 #
@@ -79,6 +81,17 @@ note.o:
        rm -f $@.bin
 
 #
+# Append .livepatch.xen_depends section
+# with Xen build-id derived from xen-syms.
+#
+.PHONY: xen_note.o
+xen_note.o:
+       $(OBJCOPY) -O binary --only-section=.note.gnu.build-id 
$(BASEDIR)/xen-syms $@.bin
+       $(OBJCOPY) $(OBJCOPY_MAGIC) \
+                  
--rename-section=.data=.livepatch.xen_depends,alloc,load,readonly,data,contents 
-S $@.bin $@
+       rm -f $@.bin
+
+#
 # Extract the build-id of the xen_hello_world.livepatch
 # (which xen_bye_world will depend on).
 #
@@ -92,20 +105,28 @@ hello_world_note.o: $(LIVEPATCH)
 xen_bye_world.o: config.h
 
 .PHONY: $(LIVEPATCH_BYE)
-$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o hello_world_note.o
+$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o hello_world_note.o 
xen_note.o
        $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_BYE) $^
 
 xen_replace_world.o: config.h
 
 .PHONY: $(LIVEPATCH_REPLACE)
-$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o note.o
+$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o note.o 
xen_note.o
        $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_REPLACE) $^
 
 xen_nop.o: config.h
 
 .PHONY: $(LIVEPATCH_NOP)
-$(LIVEPATCH_NOP): xen_nop.o note.o
+$(LIVEPATCH_NOP): xen_nop.o note.o xen_note.o
        $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_NOP) $^
 
+# This one always fails upon upload, because it deliberately
+# does not have a .livepatch.xen_depends (xen_note.o) section.
+xen_no_xen_buildid.o: config.h
+
+.PHONY: $(LIVEPATCH_NO_XEN_BUILDID)
+$(LIVEPATCH_NO_XEN_BUILDID): xen_nop.o note.o
+       $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_NO_XEN_BUILDID) $^
+
 .PHONY: livepatch
-livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP)
+livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP) 
$(LIVEPATCH_NO_XEN_BUILDID)
--
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®.