diff --git a/.gitignore b/.gitignore index 4a81f430b8..88cec1d10c 100644 --- a/.gitignore +++ b/.gitignore @@ -248,6 +248,7 @@ xen/arch/x86/efi/disabled xen/arch/x86/efi/mkreloc xen/arch/x86/test/config.h xen/arch/x86/test/xen_hello_world.xsplice +xen/arch/x86/test/xen_bye_world.xsplice xen/arch/*/efi/boot.c xen/arch/*/efi/compat.c xen/arch/*/efi/efi.h diff --git a/Config.mk b/Config.mk index 0213498bc5..79321dc51f 100644 --- a/Config.mk +++ b/Config.mk @@ -143,6 +143,7 @@ ifeq ($(call ld-ver-build-id,$(LD)),n) build_id_linker := else CFLAGS += -DBUILD_ID +export XEN_HAS_BUILD_ID=y build_id_linker := --build-id=sha1 endif diff --git a/docs/misc/xsplice.markdown b/docs/misc/xsplice.markdown index 35ebc28dfd..4a98be1803 100644 --- a/docs/misc/xsplice.markdown +++ b/docs/misc/xsplice.markdown @@ -283,8 +283,17 @@ The xSplice core code loads the payload as a standard ELF binary, relocates it and handles the architecture-specifc sections as needed. This process is much like what the Linux kernel module loader does. -The payload contains a section (xsplice_patch_func) with an array of structures -describing the functions to be patched: +The payload contains at least three sections: + + * `.xsplice.funcs` - which is an array of xsplice_patch_func structures. + * `.xsplice.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. + +### .xsplice.funcs + +The `.xsplice.funcs` contains an array of xsplice_patch_func structures +which describe the functions to be patched:
 struct xsplice_patch_func {  
@@ -368,6 +377,23 @@ struct xsplice_patch_func xsplice_hello_world = {
 
 Code must be compiled with -fPIC.
 
+### .xsplice.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 `.xsplice.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.
+
+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
+checksum, MD5 checksum or any unique value.
+
+The size of these structures varies with the --build-id linker option.
+
 ## Hypercalls
 
 We will employ the sub operations of the system management hypercall (sysctl).
@@ -853,6 +879,42 @@ This is implemented in the Xen Project hypervisor.
 
 Only the privileged domain should be allowed to do this operation.
 
+### xSplice interdependencies
+
+xSplice patches interdependencies are tricky.
+
+There are the ways this can be addressed:
+ * A single large patch that subsumes and replaces all previous ones.
+   Over the life-time of patching the hypervisor this large patch
+   grows to accumulate all the code changes.
+ * Hotpatch stack - where an mechanism exists that loads the hotpatches
+   in the same order they were built in. We would need an build-id
+   of the hypevisor to make sure the hot-patches are build against the
+   correct build.
+ * Payload containing the old code to check against that. That allows
+   the hotpatches to be loaded indepedently (if they don't overlap) - or
+   if the old code also containst previously patched code - even if they
+   overlap.
+
+The disadvantage of the first large patch is that it can grow over
+time and not provide an bisection mechanism to identify faulty patches.
+
+The hot-patch stack puts stricts requirements on the order of the patches
+being loaded and requires an hypervisor build-id to match against.
+
+The old code allows much more flexibility and an additional guard,
+but is more complex to implement.
+
+The second option which requires an build-id of the hypervisor
+is implemented in the Xen Project hypervisor.
+
+Specifically each payload has two build-id ELF notes:
+ * The build-id of the payload itself (generated via --build-id).
+ * 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.
 
 # Not Yet Done
 
@@ -872,13 +934,6 @@ The implementation must also have a mechanism for (in no particular order):
  * NOP out the code sequence if `new_size` is zero.
  * Deal with other relocation types:  R_X86_64_[8,16,32,32S], R_X86_64_PC[8,16,64]
    in payload file.
- * An dependency mechanism for the payloads. To use that information to load:
-    - The appropiate payload. To verify that payload is built against the
-      hypervisor. This can be done via the `build-id`
-      or via providing an copy of the old code - so that the hypervisor can
-       verify it against the code in memory.
-    - To construct an appropiate order of payloads to load in case they
-      depend on each other.
 
 ### Handle inlined __LINE__
 
@@ -943,32 +998,6 @@ the function itself.
 Similar considerations are true to a lesser extent for __FILE__, but it
 could be argued that file renaming should be done outside of hotpatches.
 
-### xSplice interdependencies
-
-xSplice patches interdependencies are tricky.
-
-There are the ways this can be addressed:
- * A single large patch that subsumes and replaces all previous ones.
-   Over the life-time of patching the hypervisor this large patch
-   grows to accumulate all the code changes.
- * Hotpatch stack - where an mechanism exists that loads the hotpatches
-   in the same order they were built in. We would need an build-id
-   of the hypevisor to make sure the hot-patches are build against the
-   correct build.
- * Payload containing the old code to check against that. That allows
-   the hotpatches to be loaded indepedently (if they don't overlap) - or
-   if the old code also containst previously patched code - even if they
-   overlap.
-
-The disadvantage of the first large patch is that it can grow over
-time and not provide an bisection mechanism to identify faulty patches.
-
-The hot-patch stack puts stricts requirements on the order of the patches
-being loaded and requires an hypervisor build-id to match against.
-
-The old code allows much more flexibility and an additional guard,
-but is more complex to implement.
-
 ## Signature checking requirements.
 
 The signature checking requires that the layout of the data in memory
diff --git a/xen/arch/x86/test/Makefile b/xen/arch/x86/test/Makefile
index 9c852ed489..3a25ca39a7 100644
--- a/xen/arch/x86/test/Makefile
+++ b/xen/arch/x86/test/Makefile
@@ -6,13 +6,16 @@ CODE_SZ=$(shell nm --defined -S $(1) | grep $(2) | awk '{ print "0x"$$2}')
 .PHONY: default
 
 XSPLICE := xen_hello_world.xsplice
+XSPLICE_BYE := xen_bye_world.xsplice
 
 default: xsplice
 
 install: xsplice
 	$(INSTALL_DATA) $(XSPLICE) $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE)
+	$(INSTALL_DATA) $(XSPLICE_BYE) $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE_BYE)
 uninstall:
 	rm -f $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE)
+	rm -f $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE_BYE)
 
 .PHONY: clean
 clean::
@@ -33,8 +36,41 @@ config.h: xen_hello_world_func.o
 xen_hello_world.o: config.h
 
 .PHONY: $(XSPLICE)
-$(XSPLICE): xen_hello_world_func.o xen_hello_world.o
-	$(LD) $(LDFLAGS) -r -o $(XSPLICE) $^
+$(XSPLICE): xen_hello_world_func.o xen_hello_world.o note.o
+	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(XSPLICE) $^
+
+#
+# This target is only accessible if CONFIG_XSPLICE is defined, which
+# depends on $(build_id_linker) being available. Hence we do not
+# need any checks.
+#
+# N.B. The reason we don't use arch/x86/note.o is that it may
+# not be built (it is for EFI builds), and that we do not have
+# the note.o.bin to muck with (as it gets deleted)
+#
+.PHONY: note.o
+note.o:
+	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(BASEDIR)/xen-syms $@.bin
+	$(OBJCOPY) -I binary -O elf64-x86-64 -B i386:x86-64 \
+		   --rename-section=.data=.xsplice.depends -S $@.bin $@
+	rm -f $@.bin
+
+#
+# Extract the build-id of the xen_hello_world.xsplice
+# (which xen_bye_world will depend on).
+#
+.PHONY: hello_world_note.o
+hello_world_note.o: $(XSPLICE)
+	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(XSPLICE) $@.bin
+	$(OBJCOPY)  -I binary -O elf64-x86-64 -B i386:x86-64 \
+		   --rename-section=.data=.xsplice.depends -S $@.bin $@
+	rm -f $@.bin
+
+xen_bye_world.o: config.h
+
+.PHONY: $(XSPLICE_BYE)
+$(XSPLICE_BYE): xen_bye_world_func.o xen_bye_world.o hello_world_note.o
+	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(XSPLICE_BYE) $^
 
 .PHONY: xsplice
-xsplice: $(XSPLICE)
+xsplice: $(XSPLICE) $(XSPLICE_BYE)
diff --git a/xen/arch/x86/test/xen_bye_world.c b/xen/arch/x86/test/xen_bye_world.c
new file mode 100644
index 0000000000..f93f969c7f
--- /dev/null
+++ b/xen/arch/x86/test/xen_bye_world.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include "config.h"
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+static char bye_world_patch_this_fnc[] = "xen_extra_version";
+extern const char *xen_bye_world(void);
+
+struct xsplice_patch_func __section(".xsplice.funcs") xsplice_xen_bye_world = {
+    .version = XSPLICE_PAYLOAD_VERSION,
+    .name = bye_world_patch_this_fnc,
+    .new_addr = xen_bye_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:
+ */
diff --git a/xen/arch/x86/test/xen_bye_world_func.c b/xen/arch/x86/test/xen_bye_world_func.c
new file mode 100644
index 0000000000..32ef3415d3
--- /dev/null
+++ b/xen/arch/x86/test/xen_bye_world_func.c
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include 
+
+/* Our replacement function for xen_hello_world. */
+const char *xen_bye_world(void)
+{
+    return "Bye World!";
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index e4f86c2906..91ea904203 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -60,6 +60,10 @@ config HAS_GDBSX
 config HAS_IOPORTS
 	bool
 
+config HAS_BUILD_ID
+	string
+	option env="XEN_HAS_BUILD_ID"
+
 # Enable/Disable kexec support
 config KEXEC
 	bool "kexec support"
@@ -192,7 +196,7 @@ endmenu
 config XSPLICE
 	bool "xSplice live patching support (TECH PREVIEW)"
 	default n
-	depends on X86
+	depends on X86 && HAS_BUILD_ID = "y"
 	---help---
 	  Allows a running Xen hypervisor to be dynamically patched using
 	  binary patches without rebooting. This is primarily used to binarily
diff --git a/xen/common/version.c b/xen/common/version.c
index 30578a6c76..0f96849f0d 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -86,9 +86,41 @@ int xen_build_id(const void **p, unsigned int *len)
 /* Defined in linker script. */
 extern const Elf_Note __note_gnu_build_id_start[], __note_gnu_build_id_end[];
 
+int xen_build_id_check(const Elf_Note *n, unsigned int n_sz,
+                       const void **p, unsigned int *len)
+{
+    /* Check if we really have a build-id. */
+    if ( NT_GNU_BUILD_ID != n->type )
+        return -ENODATA;
+
+    if ( n_sz <= sizeof(*n) )
+        return -EINVAL;
+
+    if ( n->namesz + n->descsz < n->namesz )
+        return -EINVAL;
+
+    if ( n->namesz < 4 /* GNU\0 */)
+        return -EINVAL;
+
+    if ( n->namesz + n->descsz > n_sz - sizeof(*n) )
+        return -EINVAL;
+
+    /* Sanity check, name should be "GNU" for ld-generated build-id. */
+    if ( strncmp(ELFNOTE_NAME(n), "GNU", n->namesz) != 0 )
+        return -ENODATA;
+
+    if ( len )
+        *len = n->descsz;
+    if ( p )
+        *p = ELFNOTE_DESC(n);
+
+    return 0;
+}
+
 static int __init xen_build_init(void)
 {
     const Elf_Note *n = __note_gnu_build_id_start;
+    unsigned int sz;
 
     /* --build-id invoked with wrong parameters. */
     if ( __note_gnu_build_id_end <= &n[0] )
@@ -98,18 +130,9 @@ static int __init xen_build_init(void)
     if ( &n[1] > __note_gnu_build_id_end )
         return -ENODATA;;
 
-    /* Check if we really have a build-id. */
-    if ( NT_GNU_BUILD_ID != n->type )
-        return -ENODATA;
+    sz = (void *)__note_gnu_build_id_end - (void *)n;
 
-    /* Sanity check, name should be "GNU" for ld-generated build-id. */
-    if ( strncmp(ELFNOTE_NAME(n), "GNU", n->namesz) != 0 )
-        return -ENODATA;
-
-    build_id_len = n->descsz;
-    build_id_p = ELFNOTE_DESC(n);
-
-    return 0;
+    return xen_build_id_check(n, sz, &build_id_p, &build_id_len);
 }
 __initcall(xen_build_init);
 #endif
diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c
index 696880eddd..4b73b97213 100644
--- a/xen/common/xsplice.c
+++ b/xen/common/xsplice.c
@@ -4,6 +4,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -42,6 +43,12 @@ static LIST_HEAD(applied_list);
 static unsigned int payload_cnt;
 static unsigned int payload_version = 1;
 
+/* To contain the ELF Note header. */
+struct xsplice_build_id {
+   const void *p;
+   unsigned int len;
+};
+
 struct payload {
     uint32_t state;                      /* One of the XSPLICE_STATE_*. */
     int32_t rc;                          /* 0 or -XEN_EXX. */
@@ -61,6 +68,8 @@ struct payload {
     struct virtual_region region;        /* symbol, bug.frame patching and
                                             exception table (x86). */
     unsigned int nsyms;                  /* Nr of entries in .strtab and symbols. */
+    struct xsplice_build_id id;          /* ELFNOTE_DESC(.note.gnu.build-id) of the payload. */
+    struct xsplice_build_id dep;         /* ELFNOTE_DESC(.xsplice.depends). */
     char name[XEN_XSPLICE_NAME_SIZE];    /* Name of it. */
 };
 
@@ -407,7 +416,9 @@ static int secure_payload(struct payload *payload, struct xsplice_elf *elf)
 static int check_special_sections(const struct xsplice_elf *elf)
 {
     unsigned int i;
-    static const char *const names[] = { ELF_XSPLICE_FUNC };
+    static const char *const names[] = { ELF_XSPLICE_FUNC,
+                                         ELF_XSPLICE_DEPENDS,
+                                         ELF_BUILD_ID_NOTE};
     DECLARE_BITMAP(found, ARRAY_SIZE(names)) = { 0 };
 
     for ( i = 0; i < ARRAY_SIZE(names); i++ )
@@ -447,6 +458,7 @@ static int prepare_payload(struct payload *payload,
     unsigned int i;
     struct xsplice_patch_func *f;
     struct virtual_region *region;
+    const Elf_Note *n;
 
     sec = xsplice_elf_sec_by_name(elf, ELF_XSPLICE_FUNC);
     ASSERT(sec);
@@ -503,6 +515,37 @@ static int prepare_payload(struct payload *payload,
         }
     }
 
+    sec = xsplice_elf_sec_by_name(elf, ELF_BUILD_ID_NOTE);
+    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->id.p, &payload->id.len) )
+            return -EINVAL;
+
+        if ( !payload->id.len || !payload->id.p )
+            return -EINVAL;
+    }
+
+    sec = xsplice_elf_sec_by_name(elf, ELF_XSPLICE_DEPENDS);
+    {
+        n = sec->load_addr;
+
+        if ( sec->sec->sh_size <= sizeof(*n) )
+            return -EINVAL;
+
+        if ( xen_build_id_check(n, sec->sec->sh_size,
+                                &payload->dep.p, &payload->dep.len) )
+            return -EINVAL;
+
+        if ( !payload->dep.len || !payload->dep.p )
+            return -EINVAL;
+    }
+
     /* Setup the virtual region with proper data. */
     region = &payload->region;
 
@@ -1232,6 +1275,55 @@ void check_for_xsplice_work(void)
     }
 }
 
+/*
+ * Only allow dependent payload is applied on top of the correct
+ * build-id.
+ *
+ * This enforces an stacking order - the first payload MUST be against the
+ * hypervisor. The second against the first payload, and so on.
+ *
+ * Unless the 'internal' parameter is used - in which case we only
+ * check against the hypervisor.
+ */
+static int build_id_dep(struct payload *payload, bool_t internal)
+{
+    const void *id = NULL;
+    unsigned int len = 0;
+    int rc;
+    const char *name = "hypervisor";
+
+    ASSERT(payload->dep.len && payload->dep.p);
+
+    /* First time user is against hypervisor. */
+    if ( internal )
+    {
+        rc = xen_build_id(&id, &len);
+        if ( rc )
+            return rc;
+    }
+    else
+    {
+        /* We should be against the last applied one. */
+        const struct payload *data;
+
+        data = list_last_entry(&applied_list, struct payload, applied_list);
+
+        id = data->id.p;
+        len = data->id.len;
+        name = data->name;
+    }
+
+    if ( payload->dep.len != len ||
+         memcmp(id, payload->dep.p, len) )
+    {
+        dprintk(XENLOG_ERR, "%s%s: check against %s build-id failed!\n",
+                XSPLICE, payload->name, name);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 static int xsplice_action(xen_sysctl_xsplice_action_t *action)
 {
     struct payload *data;
@@ -1271,6 +1363,18 @@ static int xsplice_action(xen_sysctl_xsplice_action_t *action)
     case XSPLICE_ACTION_REVERT:
         if ( data->state == XSPLICE_STATE_APPLIED )
         {
+            const struct payload *p;
+
+            p = list_last_entry(&applied_list, struct payload, applied_list);
+            ASSERT(p);
+            /* We should be the last applied one. */
+            if ( p != data )
+            {
+                dprintk(XENLOG_ERR, "%s%s: can't unload. Top is %s!\n",
+                        XSPLICE, data->name, p->name);
+                rc = -EBUSY;
+                break;
+            }
             data->rc = -EAGAIN;
             rc = schedule_work(data, action->cmd, action->timeout);
         }
@@ -1279,6 +1383,9 @@ static int xsplice_action(xen_sysctl_xsplice_action_t *action)
     case XSPLICE_ACTION_APPLY:
         if ( data->state == XSPLICE_STATE_CHECKED )
         {
+            rc = build_id_dep(data, !!list_empty(&applied_list));
+            if ( rc )
+                break;
             data->rc = -EAGAIN;
             rc = schedule_work(data, action->cmd, action->timeout);
         }
@@ -1287,6 +1394,9 @@ static int xsplice_action(xen_sysctl_xsplice_action_t *action)
     case XSPLICE_ACTION_REPLACE:
         if ( data->state == XSPLICE_STATE_CHECKED )
         {
+            rc = build_id_dep(data, 1 /* against hypervisor. */);
+            if ( rc )
+                break;
             data->rc = -EAGAIN;
             rc = schedule_work(data, action->cmd, action->timeout);
         }
@@ -1391,6 +1501,11 @@ static void xsplice_printall(unsigned char key)
                 }
             }
         }
+        if ( data->id.len )
+            printk("build-id=%*phN\n", data->id.len, data->id.p);
+
+        if ( data->dep.len )
+            printk("depend-on=%*phN\n", data->dep.len, data->dep.p);
     }
 
     spin_unlock(&payload_lock);
diff --git a/xen/include/xen/xsplice.h b/xen/include/xen/xsplice.h
index 849f58c839..5ffb69d975 100644
--- a/xen/include/xen/xsplice.h
+++ b/xen/include/xen/xsplice.h
@@ -28,6 +28,8 @@ struct xen_sysctl_xsplice_op;
 #define XSPLICE             "xsplice: "
 /* ELF payload special section names. */
 #define ELF_XSPLICE_FUNC    ".xsplice.funcs"
+#define ELF_XSPLICE_DEPENDS ".xsplice.depends"
+#define ELF_BUILD_ID_NOTE   ".note.gnu.build-id"
 
 struct xsplice_symbol {
     const char *name;
@@ -40,6 +42,8 @@ int xsplice_op(struct xen_sysctl_xsplice_op *);
 void check_for_xsplice_work(void);
 unsigned long xsplice_symbols_lookup_by_name(const char *symname);
 bool_t is_patch(const void *addr);
+int xen_build_id_check(const Elf_Note *n, unsigned int n_sz,
+                       const void **p, unsigned int *len);
 
 /* Arch hooks. */
 int arch_xsplice_verify_elf(const struct xsplice_elf *elf);