xsplice,symbols: Implement symbol name resolution on address.
authorRoss Lagerwall <ross.lagerwall@citrix.com>
Wed, 20 Apr 2016 20:19:27 +0000 (16:19 -0400)
committerKonrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Fri, 29 Apr 2016 07:58:36 +0000 (03:58 -0400)
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.

Since the payload we get is an relocatable image (partial linked ELF file)
we have to match up the symbols. We follow the ELF visibility rules for that
and for local symbols do what bintutils ld does.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
xen/arch/x86/Makefile
xen/arch/x86/platform_hypercall.c
xen/arch/x86/test/Makefile
xen/arch/x86/test/xen_hello_world.c
xen/common/symbols.c
xen/common/xsplice.c
xen/common/xsplice_elf.c
xen/include/xen/elfstructs.h
xen/include/xen/symbols.h
xen/include/xen/xsplice.h

index f8f6eeb260df7d0f0fb73c96b31f3f1f786ccb7a..fdf4202bfaa6e9455b15e7820307188e30bc7b30 100644 (file)
@@ -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 $@
index 39fa808fde138755e6fa91f7b638713d0f6a2a98..780f22d5d2f36465a244924395779efd719a53f2 100644 (file)
@@ -798,12 +798,13 @@ ret_t do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
         static char name[KSYM_NAME_LEN + 1]; /* protected by xenpf_lock */
         XEN_GUEST_HANDLE(char) nameh;
         uint32_t namelen, copylen;
+        unsigned long addr;
 
         guest_from_compat_handle(nameh, op->u.symdata.name);
 
         ret = xensyms_read(&op->u.symdata.symnum, &op->u.symdata.type,
-                           &op->u.symdata.address, name);
-
+                           &addr, name);
+        op->u.symdata.address = addr;
         namelen = strlen(name) + 1;
 
         if ( namelen > op->u.symdata.namelen )
index 8a05b3371e4cf2efb7da23d126747e2bd9713bed..9c852ed4896b5ab652b2001de531260ce50a6d7c 100644 (file)
@@ -23,14 +23,12 @@ clean::
 # and xen_hello_world_func.o to be already compiled.
 #
 .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)") > $@
 
 xen_hello_world.o: config.h
 
index f42b25cb58048f8ca9adb659bbdd07005daa0a9f..8afd66e35be74719201f3cfe53b7c1beabcbde42 100644 (file)
@@ -5,6 +5,7 @@
 
 #include "config.h"
 #include <xen/types.h>
+#include <xen/version.h>
 #include <xen/xsplice.h>
 
 #include <public/sysctl.h>
@@ -16,7 +17,7 @@ struct xsplice_patch_func __section(".xsplice.funcs") xsplice_xen_hello_world =
     .version = XSPLICE_PAYLOAD_VERSION,
     .name = hello_world_patch_this_fnc,
     .new_addr = xen_hello_world,
-    .old_addr = (void *)OLD_CODE,
+    .old_addr = xen_extra_version,
     .new_size = NEW_CODE_SZ,
     .old_size = OLD_CODE_SZ,
 };
index b18ddcd109a5c2f34e80fefe838b1792b78ceffc..e02a2afa8626a711307779212d7d8507f83635ea 100644 (file)
@@ -170,7 +170,7 @@ static char symbols_get_symbol_type(unsigned int off)
 }
 
 int xensyms_read(uint32_t *symnum, char *type,
-                 uint64_t *address, char *name)
+                 unsigned long *address, char *name)
 {
     /*
      * Symbols are most likely accessed sequentially so we remember position
@@ -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];
+    uint32_t symnum = 0;
+    char type;
+    unsigned long addr;
+    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:
+ */
index d41c08c3a284f41aa54e3df7c27714b1b81dd7f1..3d56c04946e12de24fe6cf37b4fe032454ea4afa 100644 (file)
@@ -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>
@@ -53,6 +54,9 @@ struct payload {
     struct list_head applied_list;       /* Linked to 'applied_list'. */
     struct xsplice_patch_func *funcs;    /* The array of functions to patch. */
     unsigned int nfuncs;                 /* Nr of functions to patch. */
+    const struct xsplice_symbol *symtab; /* All symbols. */
+    const char *strtab;                  /* Pointer to .strtab. */
+    unsigned int nsyms;                  /* Nr of entries in .strtab and symbols. */
     char name[XEN_XSPLICE_NAME_SIZE];    /* Name of it. */
 };
 
@@ -113,6 +117,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)
+{
+    const 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;
@@ -374,8 +400,149 @@ static int prepare_payload(struct payload *payload,
         rc = arch_xsplice_verify_func(f);
         if ( rc )
             return rc;
+
+        /* 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 )
+                {
+                    dprintk(XENLOG_ERR, 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;
+
+    /*
+     * The payload is not a final image as we dynmically link against it.
+     * As such the linker has left symbols we don't care about and which
+     * binutils would have removed had it be a final image. Hence we:
+     * - For SHF_ALLOC - ignore symbols referring to sections that are not
+     *   loaded.
+     */
+    if ( !(elf->sec[sym->sym->st_shndx].sec->sh_flags & SHF_ALLOC) )
+        return 0;
+
+    /* - And ignore empty symbols (\0). */
+    if ( *sym->name == '\0' )
+        return 0;
+
+    /*
+     * - For SHF_MERGE - ignore local symbols referring to mergeable sections.
+     *    (ld squashes them all in one section and discards the symbols) when
+     *    those symbols start with '.L' (like .LCx). Those are intermediate
+     *    artifacts of assembly.
+     *
+     * See elf_link_input_bfd and _bfd_elf_is_local_label_name in binutils.
+     */
+    if ( (elf->sec[sym->sym->st_shndx].sec->sh_flags & SHF_MERGE) &&
+         !strncmp(sym->name, ".L", 2) )
+        return 0;
+
+    return 1;
+}
+
+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].value = elf->sym[i].sym->st_value;
+            symtab[nsyms].new_symbol = 0; /* May be overwritten 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) )
+            {
+                dprintk(XENLOG_ERR, 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;
 }
 
@@ -386,6 +553,8 @@ static void free_payload(struct payload *data)
     payload_cnt--;
     payload_version++;
     free_payload_data(data);
+    xfree((void *)data->symtab);
+    xfree((void *)data->strtab);
     xfree(data);
 }
 
@@ -418,6 +587,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:
@@ -477,8 +650,12 @@ static int xsplice_upload(xen_sysctl_xsplice_upload_t *upload)
 
     vfree(raw_data);
 
-    if ( rc )
+    if ( rc && data )
+    {
+        xfree((void *)data->symtab);
+        xfree((void *)data->strtab);
         xfree(data);
+    }
 
     return rc;
 }
index f51a161f83d4b0297f34d617a415177910c15922..e403a0ef4529c13a01666f800817c3fedf84776c 100644 (file)
@@ -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>
 
@@ -273,9 +274,20 @@ int xsplice_elf_resolve_symbols(struct xsplice_elf *elf)
             break;
 
         case SHN_UNDEF:
-            dprintk(XENLOG_ERR, XSPLICE "%s: Unknown symbol: %s\n",
-                    elf->name, elf->sym[i].name);
-            rc = -ENOENT;
+            st_value = symbols_lookup_by_name(elf->sym[i].name);
+            if ( !st_value )
+            {
+                st_value = xsplice_symbols_lookup_by_name(elf->sym[i].name);
+                if ( !st_value )
+                {
+                    dprintk(XENLOG_ERR, 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, st_value);
             break;
 
         case SHN_ABS:
index 2b9bd3fbb9a338be5e5cbe9cf9213a255ba86869..ab9b1ea5fb0218ad0231dd26b2aee8b52dfd84f2 100644 (file)
@@ -263,6 +263,7 @@ typedef struct {
 #define SHF_WRITE      0x1             /* Writable */
 #define SHF_ALLOC      0x2             /* occupies memory */
 #define SHF_EXECINSTR  0x4             /* executable */
+#define SHF_MERGE      0x10            /* mergeable */
 #define SHF_MASKPROC   0xf0000000      /* reserved bits for processor */
                                        /*  specific section attributes */
 
index f58e611f834e36b962b8d0804d65172fc7a37591..015fcb46fb07525399dc7d918fe8b17cbab677e5 100644 (file)
@@ -21,6 +21,8 @@ const char *symbols_lookup(unsigned long addr,
                            char *namebuf);
 
 int xensyms_read(uint32_t *symnum, char *type,
-                 uint64_t *address, char *name);
+                 unsigned long *address, char *name);
+
+unsigned long symbols_lookup_by_name(const char *symname);
 
 #endif /*_XEN_SYMBOLS_H*/
index c9723e444f74c13508100a1aac5856c600e24502..0a6020df6a0535d3bf2164702d4d1dd59be89592 100644 (file)
@@ -19,8 +19,15 @@ struct xen_sysctl_xsplice_op;
 /* ELF payload special section names. */
 #define ELF_XSPLICE_FUNC    ".xsplice.funcs"
 
+struct xsplice_symbol {
+    const char *name;
+    unsigned long value;
+    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);