Re: [PATCH v9 11/27] xsplice: Implement payload loading

[Konrad Rzeszutek Wilk <konrad@kernel.org>]

On Wed, Apr 27, 2016 at 02:28:09AM -0600, Jan Beulich wrote:
> >>> On 27.04.16 at 05:28, <konrad.wilk@oracle.com> wrote:
> >> > +static int move_payload(struct payload *payload, struct xsplice_elf *elf)
> >> > +{
> > ..snip..
> >> > +    for ( i = 1; i < elf->hdr->e_shnum; i++ )
> >> > +    {
> >> > +        if ( elf->sec[i].sec->sh_flags & SHF_ALLOC )
> >> > +        {
> >> > +            uint8_t *buf;
> >> 
> >> Perhaps void * again? And missing a blank line afterwards.
> >> 
> >> > +            if ( (elf->sec[i].sec->sh_flags & SHF_EXECINSTR) )
> >> > +                buf = text_buf;
> >> > +            else if ( (elf->sec[i].sec->sh_flags & SHF_WRITE) )
> >> > +                buf = rw_buf;
> >> > +             else
> >> 
> >> The indentation here is still one off.
> > 
> > I am not seeing it. I deleted the line and added it back using
> > spaces just in case. But I really don't see the indentation isse
> > you are seeing?
> 
> Just count the number of blanks - I count 12 ahead of the "else if"
> but 13 ahead of the bare "else". And it's still the same in the

I finally see it now! I had been looking at the 'else if' and 'buf'
but not the 'else'. Argh.

..snip..
> > +static int move_payload(struct payload *payload, struct xsplice_elf *elf)
> > +{
> > +    void *text_buf, *ro_buf, *rw_buf;
> > +    unsigned int i;
> > +    size_t size = 0;
> > +    unsigned int *offset;
> > +    int rc = 0;
> > +
> > +    offset = xmalloc_array(unsigned int, elf->hdr->e_shnum);
> > +    if ( !offset )
> > +        return -ENOMEM;
> > +
> > +    /* Compute size of different regions. */
> > +    for ( i = 1; i < elf->hdr->e_shnum; i++ )
> > +    {
> > +        if ( (elf->sec[i].sec->sh_flags & (SHF_ALLOC|SHF_EXECINSTR)) ==
> > +             (SHF_ALLOC|SHF_EXECINSTR) )
> > +            calc_section(&elf->sec[i], &payload->text_size, &offset[i]);
> > +        else if ( (elf->sec[i].sec->sh_flags & SHF_ALLOC) &&
> > +                  !(elf->sec[i].sec->sh_flags & SHF_EXECINSTR) &&
> > +                  (elf->sec[i].sec->sh_flags & SHF_WRITE) )
> > +            calc_section(&elf->sec[i], &payload->rw_size, &offset[i]);
> > +        else if ( (elf->sec[i].sec->sh_flags & SHF_ALLOC) &&
> > +                  !(elf->sec[i].sec->sh_flags & SHF_EXECINSTR) &&
> > +                  !(elf->sec[i].sec->sh_flags & SHF_WRITE) )
> > +            calc_section(&elf->sec[i], &payload->ro_size, &offset[i]);
> > +        else if ( !elf->sec[i].sec->sh_flags ||
> > +                  (elf->sec[i].sec->sh_flags & SHF_EXECINSTR) ||
> > +                  (elf->sec[i].sec->sh_flags & SHF_MASKPROC) )
> > +            /*
> > +             * Do nothing. These are .rel.text, rel.*, .symtab, .strtab,
> > +             * and .shstrtab. For the non-relocate we allocate and copy these
> > +             * via other means - and the .rel we can ignore as we only use it
> > +             * once during loading.
> > +             */
> > +            offset[i] = UINT_MAX;
> > +        else if ( (elf->sec[i].sec->sh_flags & SHF_ALLOC) &&
> > +                  (elf->sec[i].sec->sh_type == SHT_NOBITS) )
> > +        {
> > +            dprintk(XENLOG_DEBUG, XSPLICE "%s: Not supporting %s section!\n",
> > +                    elf->name, elf->sec[i].name);
> > +            rc = -EOPNOTSUPP;
> > +            goto out;
> > +        }
> > +        else /* Such as .comment, or .debug_str. */
> > +        {
> > +            dprintk(XENLOG_DEBUG, XSPLICE "%s: Ignoring %s section!\n",
> > +                    elf->name, elf->sec[i].name);
> > +            offset[i] = UINT_MAX;
> > +        }
> 
> See earlier reply regarding this entire loop body.


So I modified it and added some more comments in
xsplice_elf_resolve_symbols to make it clear that some of these
are just plain ignored. 

Here is the inline patch:

From 99b2bb4969f1a8cc82fbd80885572a99204f95f4 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 27 Apr 2016 09:01:51 -0400
Subject: [PATCH] xsplice: Implement payload loading

Add support for loading xsplice payloads. This is somewhat similar to
the Linux kernel module loader, implementing the following steps:
- Verify the elf file.
- Parse the elf file.
- Allocate a region of memory mapped within a free area of
  [xen_virt_end, XEN_VIRT_END].
- Copy allocated sections into the new region. Split them in three
  regions - .text, .data, and .rodata. MUST have at least .text.
- Resolve section symbols. All other symbols must be absolute addresses.
  (Note that patch titled "xsplice,symbols: Implement symbol name resolution
   on address" implements that)
- Perform relocations.
- Secure the the regions (.text,.data,.rodata) with proper permissions.

We capitalize on the vmalloc callback API (see patch titled:
"rm/x86/vmap: Add v[z|m]alloc_xen, and vm_init_type") to allocate
a region of memory within the [xen_virt_end, XEN_VIRT_END] for the code.

We also use the "x86/mm: Introduce modify_xen_mappings()"
to change the virtual address page-table permissions.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Julien Grall <julien.grall@arm.com>

---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v2: - Change the 'xsplice_patch_func' structure layout/size.
    - Add more error checking. Fix memory leak.
    - Move elf_resolve and elf_perform relocs in elf file.
    - Print the payload address and pages in keyhandler.
v3:
    - Make it build under ARM
    - Build it without using the return_ macro.
    - Add fixes from Ross.
    - Add the _return macro back - but only use it during debug builds.
    - Remove the macro, prefix arch_ on arch specific calls.
v4:
    - Move alloc_payload to arch specific file.
    - Use void* instead of uint8_t, use const
    - Add copyrights
    - Unroll the vmap code to add ASSERT. Change while to not incur
      potential long error loop
   - Use vmalloc/vfree cb APIs
   - Secure .text pages to be RX instead of RWX.
v5:
  - Fix allocation of virtual addresses only allowing one page to be allocated.
  - Create .text, .data, and .rodata regions with different permissions.
  - Make the find_space_t not a typedef to pointer to a function.
  - Allocate memory in here.
v6: Drop parentheses on typedefs.
  - s/an xSplice/a xSplice/
  - Rebase on "vmap: Add vmalloc_cb"
  - Rebase on "vmap: Add vmalloc_type and vm_init_type"
  - s/uint8_t/void/ on load_addr
  - Set xsplice_elf on stack without using memset.
v7:
  - Changed the check on delta = elf->hdr->e_shoff + elf->hdr->e_shnum * elf->hdr->e_shentsize;
    The sections can be right at the back of the file (different linker!), so the failing conditional
    for 'if (delta >= elf->len)' is incorrect and should have been '>'.
  - Changed dprintk(XENLOG_DEBUG to XENLOG_ERR, then back to DEBUG. Converted
    some of the printk to dprintk.
  - Rebase on " arm/x86/vmap: Add vmalloc_xen, vfree_xen and vm_init_type"
  - Changed some of the printk XENLOG_ERR to XENLOG_DEBUG
  - Check the idx in the relocation to make sure it is within bounds and
    implemented.
  - Use "x86/mm: Introduce modify_xen_mappings()"
  - Introduce PRIxElfAddr
  - Check for overflow in R_X86_64_PC32
  - Return -EOPNOTSUPP if we don't support types in ELF64_R_TYPE
v8:
  - Change dprintk and printk XENLOG_DEBUG to XENLOG_ERR
  - Convert four of the printks in dprintk.
v9:
  - Rebase on different spinlock usage in xsplice_upload.
  - Do proper bound and overflow checking.
  - Added 'const' on [text,ro,rw]_addr.
  - Made 'calc_section' and 'move_payload' use an dynamically
    allocated array for computed offsets instead of modifying sh_entsize.
  - Remove arch_xsplice_[alloc_payload|free] and use vzalloc_xen and
    vfree.
  - Collapse for loop in move_payload.
  - Move xsplice.o in Makefile
  - Add more checks in arch_xsplice_perform_rela (r_offset and
     sh_size % sh_entsize)
  - Use int32_t and int64_t in arch_xsplice_perform_rela.
  - Tighten the list of sh_flags we check
  - Use intermediate on 'buf' so that we can do 'const void *'
  - Use intermediate in xsplice_elf_resolve_symbols for 'const' of elf->sym.
  - Fail if (and only) SHF_ALLOC and SHT_NOBITS section is seen.
v10:
   - Dropped Andrew's Reviewed-by
   - Expand arch_xsplice_verify_elf to check EI_CLASS and EI_ABIVERSION
   - In arch_xsplice_perform_rela drop check against !rela->sec->sh_entsize,
     add extra checks against r_offset + sizeof(type) neccessating
     an extra goto statement.
   - Make arch_xsplice_init be __init.
   - In free_payload_data check against ->pages instead of ->text_addr.
   - In move_payload use 'void *' instead of 'uint8_t *', use xmalloc_array
     for offset, expand on the 'Do Nothing' comment and the 'Ignoring';
     Use vmalloc instead of vzalloc - which means for .bss we also use
     memset; drop the unary + when calculating address for rw_buf;
     Fix indention (I hope? I don't see an issue); also use offset[i] =UINT_MAX
     for sections we are not going to allocate or memcpy - and assert if
     we do hit those.
   - In xsplice_elf_resolve_symbols move check against
     !(elf->sec[idx].sec->sh_flags & SHF_ALLOC) back to what it was
     in v8.
   - In xsplice_elf_perform_relocs drop comment about first ELF
     symbol.
   - Different if/else if in upload function.
v10 - patch posted as inline reply within v9 patchset:
   - Add check for EI_DATA and ditch EI_ABIVERSION out of platform specific checks.
   - In elf_resolve_symbols use const ElfSym and temporary variable (st_value)
     which is used to set value in const (and only in one place will we
     unconst the sym); change %#"PRIx.." to #x or #lx types as we don't use Elf
     types anymore.
   - Make the r_offset check be within switch statements to guard against NONE
     relocations.
   - Fix the 'else if' spacing issue;
   - Use different message when idx in symbols is out of bounds.
---
 xen/arch/arm/Makefile         |   1 +
 xen/arch/arm/xsplice.c        |  46 ++++++++
 xen/arch/x86/Makefile         |   1 +
 xen/arch/x86/xsplice.c        | 173 ++++++++++++++++++++++++++++++
 xen/common/xsplice.c          | 242 +++++++++++++++++++++++++++++++++++++++++-
 xen/common/xsplice_elf.c      | 118 ++++++++++++++++++++
 xen/include/xen/elfstructs.h  |   4 +
 xen/include/xen/xsplice.h     |  24 +++++
 xen/include/xen/xsplice_elf.h |  11 +-
 9 files changed, 615 insertions(+), 5 deletions(-)
 create mode 100644 xen/arch/arm/xsplice.c
 create mode 100644 xen/arch/x86/xsplice.c

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 0328b50..eae5cb3 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -40,6 +40,7 @@ obj-y += device.o
 obj-y += decode.o
 obj-y += processor.o
 obj-y += smc.o
+obj-$(CONFIG_XSPLICE) += xsplice.o
 
 #obj-bin-y += ....o
 
diff --git a/xen/arch/arm/xsplice.c b/xen/arch/arm/xsplice.c
new file mode 100644
index 0000000..8cb7767
--- /dev/null
+++ b/xen/arch/arm/xsplice.c
@@ -0,0 +1,46 @@
+/*
+ *  Copyright (C) 2016 Citrix Systems R&D Ltd.
+ */
+#include <xen/errno.h>
+#include <xen/init.h>
+#include <xen/lib.h>
+#include <xen/xsplice_elf.h>
+#include <xen/xsplice.h>
+
+int arch_xsplice_verify_elf(const struct xsplice_elf *elf)
+{
+    return -ENOSYS;
+}
+
+int arch_xsplice_perform_rel(struct xsplice_elf *elf,
+                             const struct xsplice_elf_sec *base,
+                             const struct xsplice_elf_sec *rela)
+{
+    return -ENOSYS;
+}
+
+int arch_xsplice_perform_rela(struct xsplice_elf *elf,
+                              const struct xsplice_elf_sec *base,
+                              const struct xsplice_elf_sec *rela)
+{
+    return -ENOSYS;
+}
+
+int arch_xsplice_secure(const void *va, unsigned int pages, enum va_type type)
+{
+    return -ENOSYS;
+}
+
+void __init arch_xsplice_init(void)
+{
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 729065b..f74fd2c 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -61,6 +61,7 @@ obj-y += x86_emulate.o
 obj-y += tboot.o
 obj-y += hpet.o
 obj-y += vm_event.o
+obj-$(CONFIG_XSPLICE) += xsplice.o
 obj-y += xstate.o
 
 obj-$(crash_debug) += gdbstub.o
diff --git a/xen/arch/x86/xsplice.c b/xen/arch/x86/xsplice.c
new file mode 100644
index 0000000..82618f7
--- /dev/null
+++ b/xen/arch/x86/xsplice.c
@@ -0,0 +1,173 @@
+/*
+ * Copyright (C) 2016 Citrix Systems R&D Ltd.
+ */
+
+#include <xen/errno.h>
+#include <xen/init.h>
+#include <xen/lib.h>
+#include <xen/mm.h>
+#include <xen/pfn.h>
+#include <xen/vmap.h>
+#include <xen/xsplice_elf.h>
+#include <xen/xsplice.h>
+
+int arch_xsplice_verify_elf(const struct xsplice_elf *elf)
+{
+
+    const Elf_Ehdr *hdr = elf->hdr;
+
+    if ( hdr->e_machine != EM_X86_64 ||
+         hdr->e_ident[EI_CLASS] != ELFCLASS64 ||
+         hdr->e_ident[EI_DATA] != ELFDATA2LSB )
+    {
+        dprintk(XENLOG_ERR, XSPLICE "%s: Unsupported ELF Machine type!\n",
+                elf->name);
+        return -EOPNOTSUPP;
+    }
+
+    return 0;
+}
+
+int arch_xsplice_perform_rel(struct xsplice_elf *elf,
+                             const struct xsplice_elf_sec *base,
+                             const struct xsplice_elf_sec *rela)
+{
+    dprintk(XENLOG_ERR, XSPLICE "%s: SHT_REL relocation unsupported\n",
+            elf->name);
+    return -EOPNOTSUPP;
+}
+
+int arch_xsplice_perform_rela(struct xsplice_elf *elf,
+                              const struct xsplice_elf_sec *base,
+                              const struct xsplice_elf_sec *rela)
+{
+    const Elf_RelA *r;
+    unsigned int symndx, i;
+    uint64_t val;
+    uint8_t *dest;
+
+    /* Nothing to do. */
+    if ( !rela->sec->sh_size )
+        return 0;
+
+    if ( rela->sec->sh_entsize < sizeof(Elf_RelA) ||
+         rela->sec->sh_size % rela->sec->sh_entsize )
+    {
+        dprintk(XENLOG_ERR, XSPLICE "%s: Section relative header is corrupted!\n",
+                elf->name);
+        return -EINVAL;
+    }
+
+    for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ )
+    {
+        r = rela->data + i * rela->sec->sh_entsize;
+
+        symndx = ELF64_R_SYM(r->r_info);
+
+        if ( symndx > elf->nsym )
+        {
+            dprintk(XENLOG_ERR, XSPLICE "%s: Relative relocation wants symbol@%u which is past end!\n",
+                    elf->name, symndx);
+            return -EINVAL;
+        }
+
+        dest = base->load_addr + r->r_offset;
+        val = r->r_addend + elf->sym[symndx].sym->st_value;
+
+        switch ( ELF64_R_TYPE(r->r_info) )
+        {
+        case R_X86_64_NONE:
+            break;
+
+        case R_X86_64_64:
+            if ( r->r_offset >= base->sec->sh_size ||
+                (r->r_offset + sizeof(uint64_t)) > base->sec->sh_size )
+                goto bad_offset;
+
+            *(uint64_t *)dest = val;
+            break;
+
+        case R_X86_64_PLT32:
+            /*
+             * Xen uses -fpic which normally uses PLT relocations
+             * except that it sets visibility to hidden which means
+             * that they are not used.  However, when gcc cannot
+             * inline memcpy it emits memcpy with default visibility
+             * which then creates a PLT relocation.  It can just be
+             * treated the same as R_X86_64_PC32.
+             */
+        case R_X86_64_PC32:
+            if ( r->r_offset >= base->sec->sh_size ||
+                (r->r_offset + sizeof(uint32_t)) > base->sec->sh_size )
+                goto bad_offset;
+
+            val -= (uint64_t)dest;
+            *(int32_t *)dest = val;
+            if ( (int64_t)val != *(int32_t *)dest )
+            {
+                dprintk(XENLOG_ERR, XSPLICE "%s: Overflow in relocation %u in %s for %s!\n",
+                        elf->name, i, rela->name, base->name);
+                return -EOVERFLOW;
+            }
+            break;
+
+        default:
+            dprintk(XENLOG_ERR, XSPLICE "%s: Unhandled relocation %lu\n",
+                    elf->name, ELF64_R_TYPE(r->r_info));
+            return -EOPNOTSUPP;
+        }
+    }
+
+    return 0;
+
+ bad_offset:
+    dprintk(XENLOG_ERR, XSPLICE "%s: Relative relocation offset is past %s section!\n",
+            elf->name, base->name);
+    return -EINVAL;
+}
+
+/*
+ * Once the resolving symbols, performing relocations, etc is complete
+ * we secure the memory by putting in the proper page table attributes
+ * for the desired type.
+ */
+int arch_xsplice_secure(const void *va, unsigned int pages, enum va_type type)
+{
+    unsigned long start = (unsigned long)va;
+    unsigned int flag;
+
+    ASSERT(va);
+    ASSERT(pages);
+
+    if ( type == XSPLICE_VA_RX )
+        flag = PAGE_HYPERVISOR_RX;
+    else if ( type == XSPLICE_VA_RW )
+        flag = PAGE_HYPERVISOR_RW;
+    else
+        flag = PAGE_HYPERVISOR_RO;
+
+    modify_xen_mappings(start, start + pages * PAGE_SIZE, flag);
+
+    return 0;
+}
+
+void __init arch_xsplice_init(void)
+{
+    void *start, *end;
+
+    start = (void *)xen_virt_end;
+    end = (void *)(XEN_VIRT_END - NR_CPUS * PAGE_SIZE);
+
+    BUG_ON(end <= start);
+
+    vm_init_type(VMAP_XEN, start, end);
+}
+/*
+ * 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 73e50f0..65545c3 100644
--- a/xen/common/xsplice.c
+++ b/xen/common/xsplice.c
@@ -13,6 +13,7 @@
 #include <xen/smp.h>
 #include <xen/spinlock.h>
 #include <xen/vmap.h>
+#include <xen/xsplice_elf.h>
 #include <xen/xsplice.h>
 
 #include <asm/event.h>
@@ -29,6 +30,13 @@ struct payload {
     uint32_t state;                      /* One of the XSPLICE_STATE_*. */
     int32_t rc;                          /* 0 or -XEN_EXX. */
     struct list_head list;               /* Linked to 'payload_list'. */
+    const void *text_addr;               /* Virtual address of .text. */
+    size_t text_size;                    /* .. and its size. */
+    const void *rw_addr;                 /* Virtual address of .data. */
+    size_t rw_size;                      /* .. and its size (if any). */
+    const void *ro_addr;                 /* Virtual address of .rodata. */
+    size_t ro_size;                      /* .. and its size (if any). */
+    unsigned int pages;                  /* Total pages for [text,rw,ro]_addr */
     char name[XEN_XSPLICE_NAME_SIZE];    /* Name of it. */
 };
 
@@ -83,19 +91,232 @@ static struct payload *find_payload(const char *name)
     return found;
 }
 
+/*
+ * Functions related to XEN_SYSCTL_XSPLICE_UPLOAD (see xsplice_upload), and
+ * freeing payload (XEN_SYSCTL_XSPLICE_ACTION:XSPLICE_ACTION_UNLOAD).
+ */
+
+static void free_payload_data(struct payload *payload)
+{
+    /* Set to zero until "move_payload". */
+    if ( !payload->pages )
+        return;
+
+    vfree((void *)payload->text_addr);
+
+    payload->pages = 0;
+}
+
+/*
+* calc_section computes the size (taking into account section alignment).
+*
+* Furthermore the offset is set with the offset from the start of the virtual
+* address space for the payload (using passed in size). This is used in
+* move_payload to figure out the destination location (load_addr).
+*/
+static void calc_section(const struct xsplice_elf_sec *sec, size_t *size,
+                         unsigned int *offset)
+{
+    const Elf_Shdr *s = sec->sec;
+    size_t align_size;
+
+    align_size = ROUNDUP(*size, s->sh_addralign);
+    *offset = align_size;
+    *size = s->sh_size + align_size;
+}
+
+static int move_payload(struct payload *payload, struct xsplice_elf *elf)
+{
+    void *text_buf, *ro_buf, *rw_buf;
+    unsigned int i;
+    size_t size = 0;
+    unsigned int *offset;
+    int rc = 0;
+
+    offset = xmalloc_array(unsigned int, elf->hdr->e_shnum);
+    if ( !offset )
+        return -ENOMEM;
+
+    /* Compute size of different regions. */
+    for ( i = 1; i < elf->hdr->e_shnum; i++ )
+    {
+        if ( (elf->sec[i].sec->sh_flags & SHF_ALLOC) &&
+             (elf->sec[i].sec->sh_flags & SHF_EXECINSTR) &&
+             !(elf->sec[i].sec->sh_flags & SHF_WRITE) )
+            calc_section(&elf->sec[i], &payload->text_size, &offset[i]);
+        else if ( (elf->sec[i].sec->sh_flags & SHF_ALLOC) &&
+                  !(elf->sec[i].sec->sh_flags & SHF_EXECINSTR) &&
+                  (elf->sec[i].sec->sh_flags & SHF_WRITE) )
+            calc_section(&elf->sec[i], &payload->rw_size, &offset[i]);
+        else if ( (elf->sec[i].sec->sh_flags & SHF_ALLOC) &&
+                  !(elf->sec[i].sec->sh_flags & SHF_EXECINSTR) &&
+                  !(elf->sec[i].sec->sh_flags & SHF_WRITE) )
+            calc_section(&elf->sec[i], &payload->ro_size, &offset[i]);
+        else if ( !elf->sec[i].sec->sh_flags ||
+                  (elf->sec[i].sec->sh_flags & SHF_EXECINSTR) ||
+                  (elf->sec[i].sec->sh_flags & SHF_MASKPROC) )
+            /*
+             * Do nothing. These are .rel.text, rel.*, .symtab, .strtab,
+             * and .shstrtab. For the non-relocate we allocate and copy these
+             * via other means - and the .rel we can ignore as we only use it
+             * once during loading.
+             */
+            offset[i] = UINT_MAX;
+        else if ( (elf->sec[i].sec->sh_flags & SHF_ALLOC) &&
+                  (elf->sec[i].sec->sh_type == SHT_NOBITS) )
+        {
+            dprintk(XENLOG_DEBUG, XSPLICE "%s: Not supporting %s section!\n",
+                    elf->name, elf->sec[i].name);
+            rc = -EOPNOTSUPP;
+            goto out;
+        }
+        else /* Such as .comment, or .debug_str. */
+        {
+            dprintk(XENLOG_DEBUG, XSPLICE "%s: Ignoring %s section!\n",
+                    elf->name, elf->sec[i].name);
+            offset[i] = UINT_MAX;
+        }
+    }
+
+    /*
+     * Total of all three regions - RX, RW, and RO. We have to have
+     * keep them in seperate pages so we PAGE_ALIGN the RX and RW to have
+     * them on seperate pages. The last one will by default fall on its
+     * own page.
+     */
+    size = PAGE_ALIGN(payload->text_size) + PAGE_ALIGN(payload->rw_size) +
+                      payload->ro_size;
+
+    size = PFN_UP(size); /* Nr of pages. */
+    text_buf = vmalloc_xen(size * PAGE_SIZE);
+    if ( !text_buf )
+    {
+        dprintk(XENLOG_ERR, XSPLICE "%s: Could not allocate memory for payload!\n",
+                elf->name);
+        rc = -ENOMEM;
+        goto out;
+    }
+    rw_buf = text_buf + PAGE_ALIGN(payload->text_size);
+    ro_buf = rw_buf + PAGE_ALIGN(payload->rw_size);
+
+    payload->pages = size;
+    payload->text_addr = text_buf;
+    payload->rw_addr = rw_buf;
+    payload->ro_addr = ro_buf;
+
+    for ( i = 1; i < elf->hdr->e_shnum; i++ )
+    {
+        if ( elf->sec[i].sec->sh_flags & SHF_ALLOC )
+        {
+            void *buf;
+
+            if ( elf->sec[i].sec->sh_flags & SHF_EXECINSTR )
+                buf = text_buf;
+            else if ( elf->sec[i].sec->sh_flags & SHF_WRITE )
+                buf = rw_buf;
+            else
+                buf = ro_buf;
+
+            ASSERT(offset[i] != UINT_MAX);
+
+            elf->sec[i].load_addr = buf + offset[i];
+
+            /* Don't copy NOBITS - such as BSS. */
+            if ( elf->sec[i].sec->sh_type != SHT_NOBITS )
+            {
+                memcpy(elf->sec[i].load_addr, elf->sec[i].data,
+                       elf->sec[i].sec->sh_size);
+                dprintk(XENLOG_DEBUG, XSPLICE "%s: Loaded %s at %p\n",
+                        elf->name, elf->sec[i].name, elf->sec[i].load_addr);
+            }
+            else
+                memset(elf->sec[i].load_addr, 0, elf->sec[i].sec->sh_size);
+        }
+    }
+
+ out:
+    xfree(offset);
+
+    return rc;
+}
+
+static int secure_payload(struct payload *payload, struct xsplice_elf *elf)
+{
+    int rc;
+    unsigned int text_pages, rw_pages, ro_pages;
+
+    text_pages = PFN_UP(payload->text_size);
+    ASSERT(text_pages);
+
+    rc = arch_xsplice_secure(payload->text_addr, text_pages, XSPLICE_VA_RX);
+    if ( rc )
+        return rc;
+
+    rw_pages = PFN_UP(payload->rw_size);
+    if ( rw_pages )
+    {
+        rc = arch_xsplice_secure(payload->rw_addr, rw_pages, XSPLICE_VA_RW);
+        if ( rc )
+            return rc;
+    }
+
+    ro_pages = PFN_UP(payload->ro_size);
+    if ( ro_pages )
+        rc = arch_xsplice_secure(payload->ro_addr, ro_pages, XSPLICE_VA_RO);
+
+    ASSERT(ro_pages + rw_pages + text_pages == payload->pages);
+
+    return rc;
+}
+
 static void free_payload(struct payload *data)
 {
     ASSERT(spin_is_locked(&payload_lock));
     list_del(&data->list);
     payload_cnt--;
     payload_version++;
+    free_payload_data(data);
     xfree(data);
 }
 
+static int load_payload_data(struct payload *payload, void *raw, size_t len)
+{
+    struct xsplice_elf elf = { .name = payload->name, .len = len };
+    int rc = 0;
+
+    rc = xsplice_elf_load(&elf, raw);
+    if ( rc )
+        goto out;
+
+    rc = move_payload(payload, &elf);
+    if ( rc )
+        goto out;
+
+    rc = xsplice_elf_resolve_symbols(&elf);
+    if ( rc )
+        goto out;
+
+    rc = xsplice_elf_perform_relocs(&elf);
+    if ( rc )
+        goto out;
+
+    rc = secure_payload(payload, &elf);
+
+ out:
+    if ( rc )
+        free_payload_data(payload);
+
+    /* Free our temporary data structure. */
+    xsplice_elf_free(&elf);
+
+    return rc;
+}
+
 static int xsplice_upload(xen_sysctl_xsplice_upload_t *upload)
 {
     struct payload *data, *found;
     char n[XEN_XSPLICE_NAME_SIZE];
+    void *raw_data;
     int rc;
 
     rc = verify_payload(upload, n);
@@ -103,6 +324,7 @@ static int xsplice_upload(xen_sysctl_xsplice_upload_t *upload)
         return rc;
 
     data = xzalloc(struct payload);
+    raw_data = vmalloc(upload->size);
 
     spin_lock(&payload_lock);
 
@@ -111,11 +333,18 @@ static int xsplice_upload(xen_sysctl_xsplice_upload_t *upload)
         rc = PTR_ERR(found);
     else if ( found )
         rc = -EEXIST;
-    else if ( !data )
+    else if ( !data || !raw_data )
         rc = -ENOMEM;
+    else if ( __copy_from_guest(raw_data, upload->payload, upload->size) )
+        rc = -EFAULT;
     else
     {
         memcpy(data->name, n, strlen(n));
+
+        rc = load_payload_data(data, raw_data, upload->size);
+        if ( rc )
+            goto out;
+
         data->state = XSPLICE_STATE_CHECKED;
         INIT_LIST_HEAD(&data->list);
 
@@ -123,8 +352,12 @@ static int xsplice_upload(xen_sysctl_xsplice_upload_t *upload)
         payload_cnt++;
         payload_version++;
     }
+
+ out:
     spin_unlock(&payload_lock);
 
+    vfree(raw_data);
+
     if ( rc )
         xfree(data);
 
@@ -359,8 +592,9 @@ static void xsplice_printall(unsigned char key)
     }
 
     list_for_each_entry ( data, &payload_list, list )
-        printk(" name=%s state=%s(%d)\n", data->name,
-               state2str(data->state), data->state);
+        printk(" name=%s state=%s(%d) %p (.data=%p, .rodata=%p) using %u pages.\n",
+               data->name, state2str(data->state), data->state, data->text_addr,
+               data->rw_addr, data->ro_addr, data->pages);
 
     spin_unlock(&payload_lock);
 }
@@ -368,6 +602,8 @@ static void xsplice_printall(unsigned char key)
 static int __init xsplice_init(void)
 {
     register_keyhandler('x', xsplice_printall, "print xsplicing info", 1);
+
+    arch_xsplice_init();
     return 0;
 }
 __initcall(xsplice_init);
diff --git a/xen/common/xsplice_elf.c b/xen/common/xsplice_elf.c
index 8136ab7..c5b7192 100644
--- a/xen/common/xsplice_elf.c
+++ b/xen/common/xsplice_elf.c
@@ -100,6 +100,7 @@ static int elf_resolve_sections(struct xsplice_elf *elf, const void *data)
 
             elf->symtab = &sec[i];
 
+            elf->symtab_idx = i;
             /*
              * elf->symtab->sec->sh_link would point to the right section
              * but we hadn't finished parsing all the sections.
@@ -250,9 +251,122 @@ static int elf_get_sym(struct xsplice_elf *elf, const void *data)
     return 0;
 }
 
+int xsplice_elf_resolve_symbols(struct xsplice_elf *elf)
+{
+    unsigned int i;
+    int rc = 0;
+
+    ASSERT(elf->sym);
+
+    for ( i = 1; i < elf->nsym; i++ )
+    {
+        unsigned int idx = elf->sym[i].sym->st_shndx;
+        const Elf_Sym *sym = elf->sym[i].sym;
+        unsigned long st_value = sym->st_value;
+
+        switch ( idx )
+        {
+        case SHN_COMMON:
+            dprintk(XENLOG_ERR, XSPLICE "%s: Unexpected common symbol: %s\n",
+                    elf->name, elf->sym[i].name);
+            rc = -EINVAL;
+            break;
+
+        case SHN_UNDEF:
+            dprintk(XENLOG_ERR, XSPLICE "%s: Unknown symbol: %s\n",
+                    elf->name, elf->sym[i].name);
+            rc = -ENOENT;
+            break;
+
+        case SHN_ABS:
+            dprintk(XENLOG_DEBUG, XSPLICE "%s: Absolute symbol: %s => %#"PRIxElfAddr"\n",
+                    elf->name, elf->sym[i].name, sym->st_value);
+            break;
+
+        default:
+            /* SHN_COMMON and SHN_ABS are above. */
+            if ( idx >= SHN_LORESERVE )
+                rc = -EOPNOTSUPP;
+            else if ( idx >= elf->hdr->e_shnum )
+                rc = -EINVAL;
+
+            if ( rc )
+            {
+                dprintk(XENLOG_ERR, XSPLICE "%s: Out of bounds symbol section %#x\n",
+                        elf->name, idx);
+                break;
+            }
+
+            /* Matches 'move_payload' which ignores such sections. */
+            if ( !(elf->sec[idx].sec->sh_flags & SHF_ALLOC) )
+                break;
+
+            st_value += (unsigned long)elf->sec[idx].load_addr;
+            if ( elf->sym[i].name )
+                dprintk(XENLOG_DEBUG, XSPLICE "%s: Symbol resolved: %s => %#lx (%s)\n",
+                       elf->name, elf->sym[i].name,
+                       st_value, elf->sec[idx].name);
+        }
+
+        if ( rc )
+            break;
+
+        ((Elf_Sym *)sym)->st_value = st_value;
+    }
+
+    return rc;
+}
+
+int xsplice_elf_perform_relocs(struct xsplice_elf *elf)
+{
+    struct xsplice_elf_sec *r, *base;
+    unsigned int i;
+    int rc = 0;
+
+    ASSERT(elf->sym);
+
+    for ( i = 1; i < elf->hdr->e_shnum; i++ )
+    {
+        r = &elf->sec[i];
+
+        if ( (r->sec->sh_type != SHT_RELA) &&
+             (r->sec->sh_type != SHT_REL) )
+            continue;
+
+         /* Is it a valid relocation section? */
+         if ( r->sec->sh_info >= elf->hdr->e_shnum )
+            continue;
+
+         base = &elf->sec[r->sec->sh_info];
+
+         /* Don't relocate non-allocated sections. */
+         if ( !(base->sec->sh_flags & SHF_ALLOC) )
+            continue;
+
+        if ( r->sec->sh_link != elf->symtab_idx )
+        {
+            dprintk(XENLOG_ERR, XSPLICE "%s: Relative link of %s is incorrect (%d, expected=%d)\n",
+                    elf->name, r->name, r->sec->sh_link, elf->symtab_idx);
+            rc = -EINVAL;
+            break;
+        }
+
+        if ( r->sec->sh_type == SHT_RELA )
+            rc = arch_xsplice_perform_rela(elf, base, r);
+        else /* SHT_REL */
+            rc = arch_xsplice_perform_rel(elf, base, r);
+
+        if ( rc )
+            break;
+    }
+
+    return rc;
+}
+
 static int xsplice_header_check(const struct xsplice_elf *elf)
 {
     const Elf_Ehdr *hdr = elf->hdr;
+    int rc;
 
     if ( sizeof(*elf->hdr) > elf->len )
     {
@@ -279,6 +393,10 @@ static int xsplice_header_check(const struct xsplice_elf *elf)
         return -EOPNOTSUPP;
     }
 
+    rc = arch_xsplice_verify_elf(elf);
+    if ( rc )
+        return rc;
+
     if ( elf->hdr->e_shstrndx == SHN_UNDEF )
     {
         dprintk(XENLOG_ERR, XSPLICE "%s: Section name idx is undefined!?\n",
diff --git a/xen/include/xen/elfstructs.h b/xen/include/xen/elfstructs.h
index 85f35ed..2b9bd3f 100644
--- a/xen/include/xen/elfstructs.h
+++ b/xen/include/xen/elfstructs.h
@@ -472,6 +472,8 @@ typedef struct {
 #endif
 
 #if defined(ELFSIZE) && (ELFSIZE == 32)
+#define PRIxElfAddr	"08x"
+
 #define Elf_Ehdr	Elf32_Ehdr
 #define Elf_Phdr	Elf32_Phdr
 #define Elf_Shdr	Elf32_Shdr
@@ -497,6 +499,8 @@ typedef struct {
 
 #define AuxInfo		Aux32Info
 #elif defined(ELFSIZE) && (ELFSIZE == 64)
+#define PRIxElfAddr	PRIx64
+
 #define Elf_Ehdr	Elf64_Ehdr
 #define Elf_Phdr	Elf64_Phdr
 #define Elf_Shdr	Elf64_Shdr
diff --git a/xen/include/xen/xsplice.h b/xen/include/xen/xsplice.h
index 7559877..857c264 100644
--- a/xen/include/xen/xsplice.h
+++ b/xen/include/xen/xsplice.h
@@ -6,6 +6,9 @@
 #ifndef __XEN_XSPLICE_H__
 #define __XEN_XSPLICE_H__
 
+struct xsplice_elf;
+struct xsplice_elf_sec;
+struct xsplice_elf_sym;
 struct xen_sysctl_xsplice_op;
 
 #ifdef CONFIG_XSPLICE
@@ -15,6 +18,27 @@ struct xen_sysctl_xsplice_op;
 
 int xsplice_op(struct xen_sysctl_xsplice_op *);
 
+/* Arch hooks. */
+int arch_xsplice_verify_elf(const struct xsplice_elf *elf);
+int arch_xsplice_perform_rel(struct xsplice_elf *elf,
+                             const struct xsplice_elf_sec *base,
+                             const struct xsplice_elf_sec *rela);
+int arch_xsplice_perform_rela(struct xsplice_elf *elf,
+                              const struct xsplice_elf_sec *base,
+                              const struct xsplice_elf_sec *rela);
+enum va_type {
+    XSPLICE_VA_RX, /* .text */
+    XSPLICE_VA_RW, /* .data */
+    XSPLICE_VA_RO, /* .rodata */
+};
+
+/*
+ * Function to secure the allocate pages (from arch_xsplice_alloc_payload)
+ * with the right page permissions.
+ */
+int arch_xsplice_secure(const void *va, unsigned int pages, enum va_type types);
+
+void arch_xsplice_init(void);
 #else
 
 #include <xen/errno.h> /* For -ENOSYS */
diff --git a/xen/include/xen/xsplice_elf.h b/xen/include/xen/xsplice_elf.h
index 686aaf0..750dc94 100644
--- a/xen/include/xen/xsplice_elf.h
+++ b/xen/include/xen/xsplice_elf.h
@@ -15,6 +15,8 @@ struct xsplice_elf_sec {
                                             elf_resolve_section_names. */
     const void *data;                    /* Pointer to the section (done by
                                             elf_resolve_sections). */
+    void *load_addr;                     /* A pointer to the allocated destination.
+                                            Done by load_payload_data. */
 };
 
 struct xsplice_elf_sym {
@@ -29,8 +31,10 @@ struct xsplice_elf {
     struct xsplice_elf_sec *sec;         /* Array of sections, allocated by us. */
     struct xsplice_elf_sym *sym;         /* Array of symbols , allocated by us. */
     unsigned int nsym;
-    const struct xsplice_elf_sec *symtab;/* Pointer to .symtab section - aka to sec[x]. */
-    const struct xsplice_elf_sec *strtab;/* Pointer to .strtab section - aka to sec[y]. */
+    const struct xsplice_elf_sec *symtab;/* Pointer to .symtab section - aka to
+                                            sec[symtab_idx]. */
+    const struct xsplice_elf_sec *strtab;/* Pointer to .strtab section. */
+    unsigned int symtab_idx;
 };
 
 const struct xsplice_elf_sec *xsplice_elf_sec_by_name(const struct xsplice_elf *elf,
@@ -38,6 +42,9 @@ const struct xsplice_elf_sec *xsplice_elf_sec_by_name(const struct xsplice_elf *
 int xsplice_elf_load(struct xsplice_elf *elf, const void *data);
 void xsplice_elf_free(struct xsplice_elf *elf);
 
+int xsplice_elf_resolve_symbols(struct xsplice_elf *elf);
+int xsplice_elf_perform_relocs(struct xsplice_elf *elf);
+
 #endif /* __XEN_XSPLICE_ELF_H__ */
 
 /*
-- 
2.4.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel