[PATCH v3 16/23] xsplice: basic build-id dependency checking.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

We now expect that the ELF payloads be built with the
--build-id.

Also the .xsplice.deps section has to have the contents
of the hypervisor (or a preceding payload) build-id.

We already have the code to verify the Elf_Note build-id
so export parts of it.

This dependency means the hypervisor MUST be compiled with
--build-id - so we gate the build of xSplice on the availability
of said functionality.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 Config.mk                  |  1 +
 docs/misc/xsplice.markdown | 78 +++++++++++++++++++++++++++++--------------
 xen/arch/x86/test/Makefile | 21 ++++++++++--
 xen/common/Kconfig         |  5 +++
 xen/common/version.c       | 41 ++++++++++++++++-------
 xen/common/xsplice.c       | 83 ++++++++++++++++++++++++++++++++++++++++++++--
 xen/include/xen/version.h  |  4 +++
 xen/include/xen/xsplice.h  |  6 ++++
 8 files changed, 197 insertions(+), 42 deletions(-)

diff --git a/Config.mk b/Config.mk
index 61186e2..ced25df 100644
--- a/Config.mk
+++ b/Config.mk
@@ -134,6 +134,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 0a5b87b..c06cd9d 100644
--- a/docs/misc/xsplice.markdown
+++ b/docs/misc/xsplice.markdown
@@ -283,9 +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.
+ *  `.note.gnu.build-id` - the build-id of this payload.
+
+### .xsplice.funcs
+
+The `.xsplice.funcs` contains an array of xsplice_patch_func structures
+which describe the functions to be patched:
 <pre>
 struct xsplice_patch_func {  
     const char *name;  
@@ -327,7 +335,7 @@ When reverting a patch, the hypervisor iterates over each `xsplice_patch_func`
 and the core code copies the data from the undo buffer (private internal copy)
 to `old_addr`.
 
-### Example
+### Example of .xsplice.funcs
 
 A simple example of what a payload file can be:
 
@@ -362,6 +370,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 an 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).
@@ -862,28 +887,6 @@ This is implemented in the Xen Project hypervisor.
 
 Only the privileged domain should be allowed to do this operation.
 
-
-# Not Yet Done
-
-This is for further development of xSplice.
-
-## Goals
-
-The design must also have a mechanism for:
-
- *  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.
- * Be able to lookup in the Xen hypervisor the symbol names of functions from the ELF payload.
- * Be able to patch .rodata, .bss, and .data sections.
- * Further safety checks (blacklist of which functions cannot be patched, check
-   the stack, etc).
- * NOP out the code sequence if `new_size` is zero.
-
 ### xSplice interdependencies
 
 xSplice patches interdependencies are tricky.
@@ -910,6 +913,31 @@ 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
+
+This is for further development of xSplice.
+
+## Goals
+
+The design must also have a mechanism for:
+
+ * Be able to lookup in the Xen hypervisor the symbol names of functions from the ELF payload.
+ * Be able to patch .rodata, .bss, and .data sections.
+ * Further safety checks (blacklist of which functions cannot be patched, check
+   the stack, etc).
+ * NOP out the code sequence if `new_size` is zero.
+
 ### Handle inlined __LINE__
 
 This problem is related to hotpatch construction
diff --git a/xen/arch/x86/test/Makefile b/xen/arch/x86/test/Makefile
index 3fe951d..de9693f 100644
--- a/xen/arch/x86/test/Makefile
+++ b/xen/arch/x86/test/Makefile
@@ -22,7 +22,7 @@ endif
 
 .PHONY: clean
 clean::
-	rm -f *.o .*.o.d $(XSPLICE) config.h
+	rm -f *.o .*.o.d $(XSPLICE) config.h build_id.o
 
 #
 # To compute these values we need the binary files: xen-syms
@@ -41,10 +41,25 @@ config.h: xen_hello_world_func.o
 	 echo "#define OLD_CODE_SZ $(OLD_CODE_SZ)"; \
 	 echo "#define OLD_CODE $(OLD_CODE)") > $@
 
+#
+# 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.
+#
+.PHONY: build_id.o
+build_id.o:
+	$(OBJCOPY) --only-section=.note $(BASEDIR)/xen-syms .$@.0
+	# Need to clear the CODE when the build_id.o is put in the .data
+	$(OBJCOPY) --set-section-flags=.note=alloc,load,data  \
+		   --rename-section=.note=.xsplice.depends .$@.0 $@
+	rm -f .$@.0
+
 .PHONY: xsplice
-xsplice: config.h
+build_id_files := build_id.o
+xsplice: config.h build_id.o
 	# Need to have these done in sequential order
 	$(MAKE) -f $(BASEDIR)/Rules.mk xen_hello_world_func.o
 	$(MAKE) -f $(BASEDIR)/Rules.mk xen_hello_world.o
-	$(LD) $(LDFLAGS) -r -o $(XSPLICE) xen_hello_world_func.o xen_hello_world.o
+	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(XSPLICE) xen_hello_world_func.o \
+		 xen_hello_world.o build_id.o
 
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 619aa9e..a313171 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -51,6 +51,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"
@@ -156,6 +160,7 @@ endmenu
 config XSPLICE
 	bool "xsplice support"
 	default y
+	depends on HAS_BUILD_ID = "y"
 	---help---
 	  Allows a running Xen hypervisor to be patched without rebooting.
 	  This is primarily used to patch an hypervisor with XSA fixes.
diff --git a/xen/common/version.c b/xen/common/version.c
index 33c09e5..e21c01e 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -69,10 +69,29 @@ const char *xen_deny(void)
 /* Defined in linker script. */
 extern const Elf_Note __note_gnu_build_id_start[], __note_gnu_build_id_end[];
 
+int xen_build_id_check(char **p, unsigned int *len, const Elf_Note *n)
+{
+    /* Check if we really have a build-id. */
+    if ( NT_GNU_BUILD_ID != n->type )
+        return -ENODATA;
+
+    /* 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;
+}
+
 int xen_build_id(char **p, unsigned int *len)
 {
     const Elf_Note *n = __note_gnu_build_id_start;
     static bool_t checked = 0;
+    int rc;
 
     if ( checked )
     {
@@ -86,23 +105,21 @@ int xen_build_id(char **p, unsigned int *len)
 
     /* Check for full Note header. */
     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;
-
-    /* Sanity check, name should be "GNU" for ld-generated build-id. */
-    if ( strncmp(ELFNOTE_NAME(n), "GNU", n->namesz) != 0 )
-        return -ENODATA;
-
-    *len = n->descsz;
-    *p = ELFNOTE_DESC(n);
+    rc = xen_build_id_check(p, len, n);
+    if ( !rc )
+        checked = 1;
 
-    checked = 1;
-    return 0;
+    return rc;
 }
 #else
+int xen_build_id_check(char **p, unsigned int *len, const Elf_Note *n)
+{
+    return -ENODATA;
+}
 int xen_build_id(char **p, unsigned int *len)
 {
     return -ENODATA;
diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c
index 34719fc..2ba5bb5 100644
--- a/xen/common/xsplice.c
+++ b/xen/common/xsplice.c
@@ -4,6 +4,7 @@
  */
 
 #include <xen/cpu.h>
+#include <xen/elf.h>
 #include <xen/guest_access.h>
 #include <xen/keyhandler.h>
 #include <xen/lib.h>
@@ -14,6 +15,7 @@
 #include <xen/softirq.h>
 #include <xen/spinlock.h>
 #include <xen/version.h>
+#include <xen/version.h>
 #include <xen/wait.h>
 #include <xen/xsplice_elf.h>
 #include <xen/xsplice.h>
@@ -51,6 +53,8 @@ struct payload {
     struct exception_table_entry *start_ex_table;
     struct exception_table_entry *stop_ex_table;
 #endif
+    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 + 1];/* Name of it. */
 };
 
@@ -639,7 +643,9 @@ static int check_special_sections(struct payload *payload,
                                   struct xsplice_elf *elf)
 {
     unsigned int i;
-    static const char *const names[] = { ".xsplice.funcs" };
+    static const char *const names[] = { ".xsplice.funcs" ,
+                                         ".xsplice.depends",
+                                         ".note.gnu.build-id"};
 
     for ( i = 0; i < ARRAY_SIZE(names); i++ )
     {
@@ -648,7 +654,7 @@ static int check_special_sections(struct payload *payload,
         sec = xsplice_elf_sec_by_name(elf, names[i]);
         if ( !sec )
         {
-            printk(XENLOG_ERR "%s: %s is missing!\n", names[i],elf->name);
+            printk(XENLOG_ERR "%s: %s is missing!\n", names[i], elf->name);
             return -EINVAL;
         }
         if ( !sec->sec->sh_size )
@@ -657,12 +663,15 @@ static int check_special_sections(struct payload *payload,
     return 0;
 }
 
+#define NT_GNU_BUILD_ID 3
+
 static int find_special_sections(struct payload *payload,
                                  struct xsplice_elf *elf)
 {
     struct xsplice_elf_sec *sec;
     unsigned int i;
     struct xsplice_patch_func *f;
+    Elf_Note *n;
 
     sec = xsplice_elf_sec_by_name(elf, ".xsplice.funcs");
     if ( sec->sec->sh_size % sizeof *payload->funcs )
@@ -689,6 +698,27 @@ static int find_special_sections(struct payload *payload,
                 return -EINVAL;
     }
 
+    sec = xsplice_elf_sec_by_name(elf, ".note.gnu.build-id");
+    if ( sec )
+    {
+        n = (Elf_Note *)sec->load_addr;
+        if ( sec->sec->sh_size <= sizeof *n )
+            return -EINVAL;
+
+        if ( xen_build_id_check(&payload->id.p, &payload->id.len, n) )
+            return -EINVAL;
+    }
+
+    sec = xsplice_elf_sec_by_name(elf, ".xsplice.depends");
+    {
+        n = (Elf_Note *)sec->load_addr;
+        if ( sec->sec->sh_size <= sizeof *n )
+            return -EINVAL;
+
+        if ( xen_build_id_check(&payload->dep.p, &payload->dep.len, n) )
+            return -EINVAL;
+    }
+
     /* Optional sections. */
     for ( i = 0; i < BUGFRAME_NR; i++ )
     {
@@ -784,6 +814,38 @@ static int load_payload_data(struct payload *payload, uint8_t *raw, ssize_t len)
  * The following functions get the CPUs into an appropriate state and
  * apply (or revert) each of the module's functions.
  */
+/* Only apply if the payload is applied on top of the correct build-id. */
+static int apply_depcheck(struct payload *payload)
+{
+    if ( !payload->dep.len )
+        return -EINVAL;
+
+    if ( list_empty(&applied_list) )
+    {
+        char *id;
+        unsigned int len;
+        int rc;
+
+        rc = xen_build_id(&id, &len);
+        if ( rc )
+            return rc;
+
+        if ( (payload->dep.len != len ) ||
+              memcmp(id, payload->dep.p, len) )
+            return -EINVAL;
+    }
+    else
+    {
+        struct payload *data = list_last_entry(&applied_list, struct payload,
+                                               applied_list);
+
+        if ( (payload->dep.len != data->id.len) ||
+             memcmp(data->id.p, payload->dep.p, data->id.len) )
+            return -EINVAL;
+    }
+
+    return 0;
+}
 
 /*
  * This function is executed having all other CPUs with no stack (we may
@@ -793,6 +855,11 @@ static int load_payload_data(struct payload *payload, uint8_t *raw, ssize_t len)
 static int apply_payload(struct payload *data)
 {
     unsigned int i;
+    int rc;
+
+    rc = apply_depcheck(data);
+    if ( rc )
+        return rc;
 
     printk(XENLOG_DEBUG "%s: Applying %u functions.\n", data->name,
            data->nfuncs);
@@ -805,6 +872,13 @@ static int apply_payload(struct payload *data)
     return 0;
 }
 
+/* Only allow reverting if this is the top of the stack. */
+static int revert_depcheck(struct payload *payload)
+{
+    return (list_last_entry_or_null(&applied_list, struct payload,
+                                    applied_list) == payload) ? 0 : -EINVAL;
+}
+
 /*
  * This function is executed having all other CPUs with no stack (we may
  * have cpu_idle on it) and IRQs disabled.
@@ -812,6 +886,11 @@ static int apply_payload(struct payload *data)
 static int revert_payload(struct payload *data)
 {
     unsigned int i;
+    int rc;
+
+    rc = revert_depcheck(data);
+    if ( rc )
+        return rc;
 
     printk(XENLOG_DEBUG "%s: Reverting.\n", data->name);
 
diff --git a/xen/include/xen/version.h b/xen/include/xen/version.h
index 466c977..7e80012 100644
--- a/xen/include/xen/version.h
+++ b/xen/include/xen/version.h
@@ -15,4 +15,8 @@ const char *xen_banner(void);
 const char *xen_deny(void);
 int xen_build_id(char **p, unsigned int *len);
 
+#include <xen/types.h>
+#include <xen/elfstructs.h>
+int xen_build_id_check(char **p, unsigned int *len, const Elf_Note *n);
+
 #endif /* __XEN_VERSION_H__ */
diff --git a/xen/include/xen/xsplice.h b/xen/include/xen/xsplice.h
index 3a9948a..061a1a1 100644
--- a/xen/include/xen/xsplice.h
+++ b/xen/include/xen/xsplice.h
@@ -24,6 +24,12 @@ struct xsplice_patch_func {
 };
 
 #ifdef CONFIG_XSPLICE
+
+struct xsplice_build_id {
+   char *p;
+   unsigned int len;
+};
+
 int xsplice_control(struct xen_sysctl_xsplice_op *);
 void do_xsplice(void);
 struct bug_frame *xsplice_find_bug(const char *eip, int *id);
-- 
2.1.0