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@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andraprs@amazon.com>
Reviewed-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Eslam Elnikety <elnikety@amazon.de>
Reviewed-by: Martin Pohlack <mpohlack@amazon.de>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>

diff --git a/.gitignore b/.gitignore
index 3ada0c4f0bb8631221ad43c06c2f9a6308783726..223bedcd2b1ebc46feb1d11d0d0a1165a3ae1d9c 100644 (file)
--- 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 6d9f72f49b27f211dd93073357ae75498cb8b9a3..fd1f5d012609fe60cb20cdb7c3334a0f3a7b1375 100644 (file)
--- 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 7ab1f822909326a9ca4ab152b6922b5d23afa0c5..a0e0d3092b826726d0d5f30a9885172b3b8e0670 100644 (file)
--- 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 69ede75d20d5c6a3f89d874c139749f80250ef4b..057a46bda3d02306d197043b0bd45838efbdff41 100644 (file)
--- 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 6831383db1a88f8e570e7c2447aad180e6201df9..938aee17ec3a49349c083aa81b8f8ce3ed0c4cc9 100644 (file)
--- 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) $^
 
 #
@@ -78,6 +80,17 @@ note.o:
                   --rename-section=.data=.livepatch.depends,alloc,load,readonly,data,contents -S $@.bin $@
        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)