From: Ross Lagerwall <ross.lagerw...@citrix.com> If in the payload we do not have the old_addr we can resolve the virtual address based on the UNDEFined symbols.
We also use an boolean flag: new_symbol to track symbols. The usual case this is used is by: * A payload may introduce a new symbol * A payload may override an existing symbol (introduced in Xen or another payload) * Overriding symbols must exist in the symtab for backtraces. * A payload must always link against the object which defines the new symbol. Considering that payloads may be loaded in any order it would be incorrect to link against a payload which simply overrides a symbol because you could end up with a chain of jumps which is inefficient and may result in the expected function not being executed. Also we include a local definition block in the symbols.c file. Signed-off-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com> Signed-off-by: Ross Lagerwall <ross.lagerw...@citrix.com> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com> --- Cc: Keir Fraser <k...@xen.org> Cc: Jan Beulich <jbeul...@suse.com> Cc: Andrew Cooper <andrew.coop...@citrix.com> v1: Ross original version. v2: Include test-case and document update. v2: s/size_t/ssize_t/ Include core_text_size, core_text calculation v4: Cast on dprintk to uint64_t to make ELF 32bit build. v6: Rebase where the spinlock is no more recursive. Drop the spinlock usage in xsplice_symbols_lookup_by_name v7: Add Andrew's Reviewed-by Initialize addr and symname to zero as xensyms_read uses it. --- --- xen/arch/x86/Makefile | 16 +++- xen/arch/x86/test/Makefile | 4 +- xen/arch/x86/test/xen_hello_world.c | 5 +- xen/common/symbols.c | 34 ++++++++ xen/common/xsplice.c | 156 +++++++++++++++++++++++++++++++++++- xen/common/xsplice_elf.c | 19 ++++- xen/include/xen/symbols.h | 2 + xen/include/xen/xsplice.h | 8 ++ 8 files changed, 232 insertions(+), 12 deletions(-) diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 6c2d5fa..57c93e1 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -72,6 +72,12 @@ efi-y := $(shell if [ ! -r $(BASEDIR)/include/xen/compile.h -o \ -O $(BASEDIR)/include/xen/compile.h ]; then \ echo '$(TARGET).efi'; fi) +ifdef CONFIG_XSPLICE +all_symbols = --all-symbols +else +all_symbols = +endif + $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 ./boot/mkelf32 $(TARGET)-syms $(TARGET) 0x100000 \ `$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'` @@ -111,12 +117,14 @@ $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \ $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0 $(NM) -pa --format=sysv $(@D)/.$(@F).0 \ - | $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).0.S + | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort \ + >$(@D)/.$(@F).0.S $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \ $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1 $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ - | $(BASEDIR)/tools/symbols --sysv --sort --warn-dup >$(@D)/.$(@F).1.S + | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort --warn-dup \ + >$(@D)/.$(@F).1.S $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \ $(@D)/.$(@F).1.o -o $@ @@ -140,14 +148,14 @@ $(TARGET).efi: prelink-efi.o efi.lds efi/relocs-dummy.o $(BASEDIR)/common/symbol $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).$(base).0 &&) : $(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).0) >$(@D)/.$(@F).0r.S $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).0 \ - | $(guard) $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).0s.S + | $(guard) $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0s.S $(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o $(foreach base, $(VIRT_BASE) $(ALT_BASE), \ $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< \ $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o -o $(@D)/.$(@F).$(base).1 &&) : $(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).1) >$(@D)/.$(@F).1r.S $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).1 \ - | $(guard) $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).1s.S + | $(guard) $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1s.S $(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o $(guard) $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds -N $< \ $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o -o $@ diff --git a/xen/arch/x86/test/Makefile b/xen/arch/x86/test/Makefile index b9cf13c..baa4820 100644 --- a/xen/arch/x86/test/Makefile +++ b/xen/arch/x86/test/Makefile @@ -26,14 +26,12 @@ clean:: # the last entry in the build target. # .PHONY: config.h -config.h: OLD_CODE=$(call CODE_ADDR,$(BASEDIR)/xen-syms,xen_extra_version) config.h: OLD_CODE_SZ=$(call CODE_SZ,$(BASEDIR)/xen-syms,xen_extra_version) config.h: NEW_CODE_SZ=$(call CODE_SZ,$<,xen_hello_world) config.h: xen_hello_world_func.o (set -e; \ echo "#define NEW_CODE_SZ $(NEW_CODE_SZ)"; \ - echo "#define OLD_CODE_SZ $(OLD_CODE_SZ)"; \ - echo "#define OLD_CODE $(OLD_CODE)") > $@ + echo "#define OLD_CODE_SZ $(OLD_CODE_SZ)") > $@ .PHONY: xsplice xsplice: config.h diff --git a/xen/arch/x86/test/xen_hello_world.c b/xen/arch/x86/test/xen_hello_world.c index 21831e9..22fe2e7 100644 --- a/xen/arch/x86/test/xen_hello_world.c +++ b/xen/arch/x86/test/xen_hello_world.c @@ -10,11 +10,14 @@ static char hello_world_patch_this_fnc[] = "xen_extra_version"; extern const char *xen_hello_world(void); +/* External symbol. */ +extern const char *xen_extra_version(void); + struct xsplice_patch_func __section(".xsplice.funcs") xsplice_xen_hello_world = { .version = XSPLICE_PAYLOAD_VERSION, .name = hello_world_patch_this_fnc, .new_addr = (unsigned long)(xen_hello_world), - .old_addr = OLD_CODE, + .old_addr = (unsigned long)(xen_extra_version), .new_size = NEW_CODE_SZ, .old_size = OLD_CODE_SZ, }; diff --git a/xen/common/symbols.c b/xen/common/symbols.c index b18ddcd1..781765d 100644 --- a/xen/common/symbols.c +++ b/xen/common/symbols.c @@ -207,3 +207,37 @@ int xensyms_read(uint32_t *symnum, char *type, return 0; } + +unsigned long symbols_lookup_by_name(const char *symname) +{ + char name[KSYM_NAME_LEN + 1] = { 0 }; + uint32_t symnum = 0; + char type; + uint64_t addr = 0; + int rc; + + if ( symname == '\0' ) + return 0; + + do { + rc = xensyms_read(&symnum, &type, &addr, name); + if ( rc ) + break; + + if ( !strcmp(name, symname) ) + return addr; + + } while ( name[0] != '\0' ); + + return 0; +} + +/* + * 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/xsplice.c b/xen/common/xsplice.c index 9a53cf4..27f4822 100644 --- a/xen/common/xsplice.c +++ b/xen/common/xsplice.c @@ -14,6 +14,7 @@ #include <xen/smp.h> #include <xen/softirq.h> #include <xen/spinlock.h> +#include <xen/symbols.h> #include <xen/vmap.h> #include <xen/wait.h> #include <xen/xsplice_elf.h> @@ -51,6 +52,9 @@ struct payload { struct list_head applied_list; /* Linked to 'applied_list'. */ struct xsplice_patch_func_internal *funcs; /* The array of functions to patch. */ unsigned int nfuncs; /* Nr of functions to patch. */ + struct xsplice_symbol *symtab; /* All symbols. */ + char *strtab; /* Pointer to .strtab. */ + unsigned int nsyms; /* Nr of entries in .strtab and symbols. */ char name[XEN_XSPLICE_NAME_SIZE]; /* Name of it. */ }; @@ -114,6 +118,28 @@ static int verify_payload(const xen_sysctl_xsplice_upload_t *upload, char *n) return 0; } +unsigned long xsplice_symbols_lookup_by_name(const char *symname) +{ + struct payload *data; + + ASSERT(spin_is_locked(&payload_lock)); + list_for_each_entry ( data, &payload_list, list ) + { + unsigned int i; + + for ( i = 0; i < data->nsyms; i++ ) + { + if ( !data->symtab[i].new_symbol ) + continue; + + if ( !strcmp(data->symtab[i].name, symname) ) + return data->symtab[i].value; + } + } + + return 0; +} + static struct payload *find_payload(const char *name) { struct payload *data, *found = NULL; @@ -376,11 +402,129 @@ static int prepare_payload(struct payload *payload, for ( j = 0; j < ARRAY_SIZE(f->u.pad); j++ ) if ( f->u.pad[j] ) return -EINVAL; + + /* Lookup function's old address if not already resolved. */ + if ( !f->old_addr ) + { + f->old_addr = (void *)symbols_lookup_by_name(f->name); + if ( !f->old_addr ) + { + f->old_addr = (void *)xsplice_symbols_lookup_by_name(f->name); + if ( !f->old_addr ) + { + printk(XENLOG_DEBUG XSPLICE "%s: Could not resolve old address of %s\n", + elf->name, f->name); + return -ENOENT; + } + } + dprintk(XENLOG_DEBUG, XSPLICE "%s: Resolved old address %s => %p\n", + elf->name, f->name, f->old_addr); + } } return 0; } +static bool_t is_payload_symbol(const struct xsplice_elf *elf, + const struct xsplice_elf_sym *sym) +{ + if ( sym->sym->st_shndx == SHN_UNDEF || + sym->sym->st_shndx >= elf->hdr->e_shnum ) + return 0; + + return (elf->sec[sym->sym->st_shndx].sec->sh_flags & SHF_ALLOC) && + (ELF64_ST_TYPE(sym->sym->st_info) == STT_OBJECT || + ELF64_ST_TYPE(sym->sym->st_info) == STT_FUNC); +} + +static int build_symbol_table(struct payload *payload, + const struct xsplice_elf *elf) +{ + unsigned int i, j, nsyms = 0; + size_t strtab_len = 0; + struct xsplice_symbol *symtab; + char *strtab; + + ASSERT(payload->nfuncs); + + /* Recall that section @0 is always NULL. */ + for ( i = 1; i < elf->nsym; i++ ) + { + if ( is_payload_symbol(elf, elf->sym + i) ) + { + nsyms++; + strtab_len += strlen(elf->sym[i].name) + 1; + } + } + + symtab = xmalloc_array(struct xsplice_symbol, nsyms); + strtab = xmalloc_array(char, strtab_len); + + if ( !strtab || !symtab ) + { + xfree(strtab); + xfree(symtab); + return -ENOMEM; + } + + nsyms = 0; + strtab_len = 0; + for ( i = 1; i < elf->nsym; i++ ) + { + if ( is_payload_symbol(elf, elf->sym + i) ) + { + symtab[nsyms].name = strtab + strtab_len; + symtab[nsyms].size = elf->sym[i].sym->st_size; + symtab[nsyms].value = elf->sym[i].sym->st_value; + symtab[nsyms].new_symbol = 0; /* To be checked below. */ + strtab_len += strlcpy(strtab + strtab_len, elf->sym[i].name, + KSYM_NAME_LEN) + 1; + nsyms++; + } + } + + for ( i = 0; i < nsyms; i++ ) + { + bool_t found = 0; + + for ( j = 0; j < payload->nfuncs; j++ ) + { + if ( symtab[i].value == (unsigned long)payload->funcs[j].new_addr ) + { + found = 1; + break; + } + } + + if ( !found ) + { + if ( xsplice_symbols_lookup_by_name(symtab[i].name) ) + { + printk(XENLOG_DEBUG XSPLICE "%s: duplicate new symbol: %s\n", + elf->name, symtab[i].name); + xfree(symtab); + xfree(strtab); + return -EEXIST; + } + symtab[i].new_symbol = 1; + dprintk(XENLOG_DEBUG, XSPLICE "%s: new symbol %s\n", + elf->name, symtab[i].name); + } + else + { + /* new_symbol is not set. */ + dprintk(XENLOG_DEBUG, XSPLICE "%s: overriding symbol %s\n", + elf->name, symtab[i].name); + } + } + + payload->symtab = symtab; + payload->strtab = strtab; + payload->nsyms = nsyms; + + return 0; +} + static void free_payload(struct payload *data) { ASSERT(spin_is_locked(&payload_lock)); @@ -388,6 +532,8 @@ static void free_payload(struct payload *data) payload_cnt--; payload_version++; free_payload_data(data); + xfree(data->symtab); + xfree(data->strtab); xfree(data); } @@ -420,6 +566,10 @@ static int load_payload_data(struct payload *payload, void *raw, size_t len) if ( rc ) goto out; + rc = build_symbol_table(payload, &elf); + if ( rc ) + goto out; + rc = secure_payload(payload, &elf); out: @@ -491,8 +641,12 @@ static int xsplice_upload(xen_sysctl_xsplice_upload_t *upload) vfree(raw_data); - if ( rc ) + if ( rc && data ) + { + xfree(data->symtab); + xfree(data->strtab); xfree(data); + } return rc; } diff --git a/xen/common/xsplice_elf.c b/xen/common/xsplice_elf.c index 7262fc2..09b8c93 100644 --- a/xen/common/xsplice_elf.c +++ b/xen/common/xsplice_elf.c @@ -4,6 +4,7 @@ #include <xen/errno.h> #include <xen/lib.h> +#include <xen/symbols.h> #include <xen/xsplice_elf.h> #include <xen/xsplice.h> @@ -274,9 +275,21 @@ int xsplice_elf_resolve_symbols(struct xsplice_elf *elf) break; case SHN_UNDEF: - printk(XENLOG_DEBUG XSPLICE "%s: Unknown symbol: %s\n", - elf->name, elf->sym[i].name); - rc = -ENOENT; + elf->sym[i].sym->st_value = (unsigned long)symbols_lookup_by_name(elf->sym[i].name); + if ( !elf->sym[i].sym->st_value ) + { + elf->sym[i].sym->st_value = (unsigned long) + xsplice_symbols_lookup_by_name(elf->sym[i].name); + if ( !elf->sym[i].sym->st_value ) + { + printk(XENLOG_DEBUG XSPLICE "%s: Unknown symbol: %s\n", + elf->name, elf->sym[i].name); + rc = -ENOENT; + break; + } + } + dprintk(XENLOG_DEBUG, XSPLICE "%s: Undefined symbol resolved: %s => %#"PRIxElfAddr"\n", + elf->name, elf->sym[i].name, elf->sym[i].sym->st_value); break; case SHN_ABS: diff --git a/xen/include/xen/symbols.h b/xen/include/xen/symbols.h index f58e611..d9bd35f 100644 --- a/xen/include/xen/symbols.h +++ b/xen/include/xen/symbols.h @@ -23,4 +23,6 @@ const char *symbols_lookup(unsigned long addr, int xensyms_read(uint32_t *symnum, char *type, uint64_t *address, char *name); +unsigned long symbols_lookup_by_name(const char *symname); + #endif /*_XEN_SYMBOLS_H*/ diff --git a/xen/include/xen/xsplice.h b/xen/include/xen/xsplice.h index 8486887..a0436da 100644 --- a/xen/include/xen/xsplice.h +++ b/xen/include/xen/xsplice.h @@ -57,8 +57,16 @@ struct xsplice_patch_func_internal { /* Convenience define for printk. */ #define XSPLICE "xsplice: " +struct xsplice_symbol { + const char *name; + uint64_t value; + size_t size; + bool_t new_symbol; +}; + int xsplice_op(struct xen_sysctl_xsplice_op *); void check_for_xsplice_work(void); +unsigned long xsplice_symbols_lookup_by_name(const char *symname); /* Arch hooks. */ int arch_xsplice_verify_elf(const struct xsplice_elf *elf); -- 2.5.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel