diff mbox

[v3,19/23] xsplice, symbols: Implement symbol name resolution on address. (v2)

Message ID 1455300361-13092-20-git-send-email-konrad.wilk@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Konrad Rzeszutek Wilk Feb. 12, 2016, 6:05 p.m. UTC
From: Ross Lagerwall <ross.lagerwall@citrix.com>

If in the payload we do not have the old_addr we can resolve
the virtual address based on the UNDEFined symbols.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
v1: Ross original version.
v2: Include test-case and document update.
---
 docs/misc/xsplice.markdown          |   1 -
 xen/arch/x86/Makefile               |   6 +-
 xen/arch/x86/test/Makefile          |   4 +-
 xen/arch/x86/test/xen_hello_world.c |   5 +-
 xen/common/symbols.c                |  23 ++++++
 xen/common/xsplice.c                | 151 ++++++++++++++++++++++++++++++++++++
 xen/common/xsplice_elf.c            |  19 ++++-
 xen/include/xen/symbols.h           |   2 +
 xen/include/xen/xsplice.h           |   7 ++
 9 files changed, 208 insertions(+), 10 deletions(-)

Comments

Ross Lagerwall Feb. 22, 2016, 2:57 p.m. UTC | #1
On 02/12/2016 06:05 PM, Konrad Rzeszutek Wilk wrote:
snip
>   static int load_payload_data(struct payload *payload, uint8_t *raw, ssize_t len)
>   {
>       struct xsplice_elf elf;
> @@ -831,6 +953,10 @@ static int load_payload_data(struct payload *payload, uint8_t *raw, ssize_t len)
>       if ( rc )
>           goto err_payload;
>
> +    rc = build_symbol_table(payload, &elf);
> +    if ( rc )
> +        goto err_payload;
> +
>       rc = find_special_sections(payload, &elf);
>       if ( rc )
>           goto err_payload;
> @@ -1234,6 +1360,31 @@ unsigned long search_module_extables(unsigned long addr)
>   }
>   #endif
>

build_symbol_table() needs to go after find_special_sections() because 
it uses payload->nfuncs which is only calculated in 
find_special_sections(). Why did you reorder it from how I did it?
diff mbox

Patch

diff --git a/docs/misc/xsplice.markdown b/docs/misc/xsplice.markdown
index c06cd9d..1a982f2 100644
--- a/docs/misc/xsplice.markdown
+++ b/docs/misc/xsplice.markdown
@@ -932,7 +932,6 @@  This is for further development of xSplice.
 
 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).
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 4a19ae9..5f7c57e 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -119,12 +119,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 $(build_id_linker) \
 	    $(@D)/.$(@F).1.o -o $@
diff --git a/xen/arch/x86/test/Makefile b/xen/arch/x86/test/Makefile
index de9693f..57c5189 100644
--- a/xen/arch/x86/test/Makefile
+++ b/xen/arch/x86/test/Makefile
@@ -32,14 +32,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,$<,xen_hello_world)
 config.h: NEW_CODE_SZ=$(call CODE_SZ,$(BASEDIR)/xen-syms,xen_extra_version)
 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)") > $@
 
 #
 # This target is only accessible if CONFIG_XSPLICE is defined, which
diff --git a/xen/arch/x86/test/xen_hello_world.c b/xen/arch/x86/test/xen_hello_world.c
index 6a1775b..6200fbe 100644
--- a/xen/arch/x86/test/xen_hello_world.c
+++ b/xen/arch/x86/test/xen_hello_world.c
@@ -6,10 +6,13 @@ 
 static char name[] = "xen_hello_world";
 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 = {
     .name = name,
     .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 bf5623f..406e5be 100644
--- a/xen/common/symbols.c
+++ b/xen/common/symbols.c
@@ -209,3 +209,26 @@  int xensyms_read(uint32_t *symnum, char *type,
 
     return 0;
 }
+
+uint64_t symbols_lookup_by_name(const char *symname)
+{
+    uint32_t symnum = 0;
+    uint64_t addr = 0, outaddr = 0;
+    int rc;
+    char type;
+    char name[KSYM_NAME_LEN + 1] = {0};
+
+    do {
+        rc = xensyms_read(&symnum, &type, &addr, name);
+        if ( rc )
+            break;
+
+        if ( !strcmp(name, symname) )
+        {
+            outaddr = addr;
+            break;
+        }
+    } while ( name[0] != '\0' );
+
+    return outaddr;
+}
diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c
index 3f1da13..0b42a16 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/version.h>
 #include <xen/version.h>
 #include <xen/wait.h>
@@ -55,6 +56,9 @@  struct payload {
 #endif
     struct xsplice_build_id id;          /* ELFNOTE_DESC(.note.gnu.build-id) of the payload. */
     struct xsplice_build_id dep;         /* ELFNOTE_DESC(.xsplice.depends). */
+    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 + 1];/* Name of it. */
 };
 
@@ -232,6 +236,8 @@  static void free_payload(struct payload *data)
     payload_cnt--;
     payload_version++;
     free_payload_data(data);
+    xfree(data->symtab);
+    xfree(data->strtab);
     xfree(data);
 }
 
@@ -290,6 +296,8 @@  static int xsplice_upload(xen_sysctl_xsplice_upload_t *upload)
  err_raw:
     free_xenheap_pages(raw_data, get_order_from_bytes(upload->size));
  err_data:
+    xfree(data->symtab);
+    xfree(data->strtab);
     xfree(data);
     return rc;
 }
@@ -716,6 +724,24 @@  static int find_special_sections(struct payload *payload,
         for ( j = 0; j < 24; j ++ )
             if ( f->pad[j] )
                 return -EINVAL;
+
+        /* Lookup function's old address if not already resolved. */
+        if ( !f->old_addr )
+        {
+            f->old_addr = symbols_lookup_by_name(f->name);
+            if ( !f->old_addr )
+            {
+                f->old_addr = xsplice_symbols_lookup_by_name(f->name);
+                if ( !f->old_addr )
+                {
+                    printk(XENLOG_ERR "%s: Could not resolve old address of %s\n",
+                           elf->name, f->name);
+                    return -ENOENT;
+                }
+            }
+            printk(XENLOG_DEBUG "%s: Resolved old address %s => 0x%"PRIx64"\n",
+                   elf->name, f->name, f->old_addr);
+        }
     }
 
     sec = xsplice_elf_sec_by_name(elf, ".note.gnu.build-id");
@@ -798,6 +824,102 @@  static int find_special_sections(struct payload *payload,
     return 0;
 }
 
+static bool_t is_core_symbol(struct xsplice_elf *elf,
+                             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, struct xsplice_elf *elf)
+{
+    unsigned int i, j, nsyms = 0;
+    size_t strtab_len = 0;
+    struct xsplice_symbol *symtab;
+    char *strtab;
+
+    /* Recall that 0 is always NULL. */
+    for ( i = 1; i < elf->nsym; i++ )
+    {
+        if ( is_core_symbol(elf, elf->sym + i) )
+        {
+            nsyms++;
+            strtab_len += strlen(elf->sym[i].name) + 1;
+        }
+    }
+
+    symtab = xmalloc_array(struct xsplice_symbol, nsyms);
+    if ( !symtab )
+        return -ENOMEM;
+
+    strtab = xmalloc_bytes(strtab_len);
+    if ( !strtab )
+    {
+        xfree(symtab);
+        return -ENOMEM;
+    }
+
+    nsyms = 0;
+    strtab_len = 0;
+    for ( i = 1; i < elf->nsym; i++ )
+    {
+        if ( is_core_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].flags = 0;
+            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 == payload->funcs[j].new_addr )
+            {
+                found = 1;
+                break;
+            }
+        }
+
+        if ( !found )
+        {
+            if ( xsplice_symbols_lookup_by_name(symtab[i].name) )
+            {
+                printk(XENLOG_ERR "%s: duplicate new symbol: %s\n", elf->name,
+                       symtab[i].name);
+                xfree(symtab);
+                xfree(strtab);
+                return -EEXIST;
+            }
+            printk(XENLOG_DEBUG "%s: new symbol %s\n", elf->name,
+                   symtab[i].name);
+        }
+        else
+        {
+            printk(XENLOG_DEBUG "%s: overriding symbol %s\n", elf->name,
+                   symtab[i].name);
+        }
+    }
+
+    payload->symtab = symtab;
+    payload->strtab = strtab;
+    payload->nsyms = nsyms;
+
+    return 0;
+}
+
 static int load_payload_data(struct payload *payload, uint8_t *raw, ssize_t len)
 {
     struct xsplice_elf elf;
@@ -831,6 +953,10 @@  static int load_payload_data(struct payload *payload, uint8_t *raw, ssize_t len)
     if ( rc )
         goto err_payload;
 
+    rc = build_symbol_table(payload, &elf);
+    if ( rc )
+        goto err_payload;
+
     rc = find_special_sections(payload, &elf);
     if ( rc )
         goto err_payload;
@@ -1234,6 +1360,31 @@  unsigned long search_module_extables(unsigned long addr)
 }
 #endif
 
+uint64_t xsplice_symbols_lookup_by_name(const char *symname)
+{
+    struct payload *data;
+    unsigned int i;
+    uint64_t value = 0;
+
+    spin_lock(&payload_lock);
+
+    list_for_each_entry ( data, &payload_list, list )
+    {
+        for ( i = 0; i < data->nsyms; i++ )
+        {
+            if ( !strcmp(data->symtab[i].name, symname) )
+            {
+                value = data->symtab[i].value;
+                goto out;
+            }
+        }
+    }
+
+out:
+    spin_unlock(&payload_lock);
+    return value;
+}
+
 static int __init xsplice_init(void)
 {
     register_keyhandler('x', xsplice_printall, "print xsplicing info", 1);
diff --git a/xen/common/xsplice_elf.c b/xen/common/xsplice_elf.c
index ad70797..dade9fd 100644
--- a/xen/common/xsplice_elf.c
+++ b/xen/common/xsplice_elf.c
@@ -1,5 +1,6 @@ 
 #include <xen/errno.h>
 #include <xen/lib.h>
+#include <xen/symbols.h>
 #include <xen/xsplice_elf.h>
 #include <xen/xsplice.h>
 
@@ -223,9 +224,21 @@  int xsplice_elf_resolve_symbols(struct xsplice_elf *elf)
                 return_(-EINVAL);
                 break;
             case SHN_UNDEF:
-                printk(XENLOG_ERR "%s: Unknown symbol: %s\n", elf->name,
-                       elf->sym[i].name);
-                return_(-ENOENT);
+                elf->sym[i].sym->st_value = symbols_lookup_by_name(elf->sym[i].name);
+                if ( !elf->sym[i].sym->st_value )
+                {
+                    elf->sym[i].sym->st_value =
+                        xsplice_symbols_lookup_by_name(elf->sym[i].name);
+                    if ( !elf->sym[i].sym->st_value )
+                    {
+                        printk(XENLOG_ERR "%s: Unknown symbol: %s\n", elf->name,
+                               elf->sym[i].name);
+                        return_(-ENOENT);
+                    }
+                }
+                printk(XENLOG_DEBUG "%s: Undefined symbol resolved: %s => 0x%"PRIx64"\n",
+                       elf->name, elf->sym[i].name,
+                       elf->sym[i].sym->st_value);
                 break;
             case SHN_ABS:
                 printk(XENLOG_DEBUG "%s: Absolute symbol: %s => 0x%"PRIx64"\n",
diff --git a/xen/include/xen/symbols.h b/xen/include/xen/symbols.h
index 1fa0537..f8ea1dc 100644
--- a/xen/include/xen/symbols.h
+++ b/xen/include/xen/symbols.h
@@ -14,4 +14,6 @@  const char *symbols_lookup(unsigned long addr,
 int xensyms_read(uint32_t *symnum, char *type,
                  uint64_t *address, char *name);
 
+uint64_t 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 061a1a1..1045213 100644
--- a/xen/include/xen/xsplice.h
+++ b/xen/include/xen/xsplice.h
@@ -30,12 +30,19 @@  struct xsplice_build_id {
    unsigned int len;
 };
 
+struct xsplice_symbol {
+    const char *name;
+    uint64_t value;
+    size_t size;
+};
+
 int xsplice_control(struct xen_sysctl_xsplice_op *);
 void do_xsplice(void);
 struct bug_frame *xsplice_find_bug(const char *eip, int *id);
 bool_t is_module(const void *addr);
 bool_t is_active_module_text(unsigned long addr);
 unsigned long search_module_extables(unsigned long addr);
+uint64_t xsplice_symbols_lookup_by_name(const char *symname);
 
 /* Arch hooks */
 int xsplice_verify_elf(struct xsplice_elf *elf, uint8_t *data);