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.w...@oracle.com> Signed-off-by: Ross Lagerwall <ross.lagerw...@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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel