Re: [PATCH v2 08/13] xsplice: Implement payload loading (v2)

[Ross Lagerwall <ross.lagerwall@citrix.com>]

On 01/14/2016 09:47 PM, Konrad Rzeszutek Wilk wrote:
> From: Ross Lagerwall <ross.lagerwall@citrix.com>
>
> 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.
> - Resolve section symbols. All other symbols must be absolute addresses.
> - Perform relocations.
>
> Note that the structure 'xsplice_patch_func' differs a bit from the design
> by usurping 8 bytes from the padding. We use that for our own uses.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.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
snip
>
> +static void find_hole(ssize_t pages, unsigned long *hole_start,
> +                      unsigned long *hole_end)
> +{
> +    struct payload *data, *data2;
> +
> +    spin_lock(&payload_list_lock);
> +    list_for_each_entry ( data, &payload_list, list )
> +    {
> +        list_for_each_entry ( data2, &payload_list, list )
> +        {
> +            unsigned long start, end;
> +
> +            start = (unsigned long)data2->payload_address;
> +            end = start + data2->payload_pages * PAGE_SIZE;
> +            if ( *hole_end > start && *hole_start < end )
> +            {
> +                *hole_start = end;
> +                *hole_end = *hole_start + pages * PAGE_SIZE;
> +                break;
> +            }
> +        }
> +        if ( &data2->list == &payload_list )
> +            break;
> +    }
> +    spin_unlock(&payload_list_lock);
> +}

This function above should go down into the CONFIG_X86 section below.

> +
> +/*
> + * The following functions prepare an xSplice payload to be executed by
> + * allocating space, loading the allocated sections, resolving symbols,
> + * performing relocations, etc.
> + */
> +#ifdef CONFIG_X86
> +static void *alloc_payload(size_t size)
> +{
> +    mfn_t *mfn, *mfn_ptr;
> +    size_t pages, i;
> +    struct page_info *pg;
> +    unsigned long hole_start, hole_end, cur;
> +
> +    ASSERT(size);
> +
> +    /*
> +     * Copied from vmalloc which allocates pages and then maps them to an
> +     * arbitrary virtual address with PAGE_HYPERVISOR. We need specific
> +     * virtual address with PAGE_HYPERVISOR_RWX.
> +     */
> +    pages = PFN_UP(size);
> +    mfn = xmalloc_array(mfn_t, pages);
> +    if ( mfn == NULL )
> +        return NULL;
> +
> +    for ( i = 0; i < pages; i++ )
> +    {
> +        pg = alloc_domheap_page(NULL, 0);
> +        if ( pg == NULL )
> +            goto error;
> +        mfn[i] = _mfn(page_to_mfn(pg));
snip
> diff --git a/xen/common/xsplice_elf.c b/xen/common/xsplice_elf.c
> index a5e9d63..ea7eb73 100644
> --- a/xen/common/xsplice_elf.c
> +++ b/xen/common/xsplice_elf.c
> @@ -199,3 +199,87 @@ void xsplice_elf_free(struct xsplice_elf *elf)
>       elf->name = NULL;
>       elf->len = 0;
>   }
> +
> +int xsplice_elf_resolve_symbols(struct xsplice_elf *elf)
> +{
> +    unsigned int i;
> +
> +    /*
> +     * The first entry of an ELF symbol table is the "undefined symbol index".
> +     * aka reserved so we skip it.
> +     */
> +    ASSERT( elf->sym );
> +    for ( i = 1; i < elf->nsym; i++ )
> +    {
> +        switch ( elf->sym[i].sym->st_shndx )
> +        {
> +            case SHN_COMMON:
> +                printk(XENLOG_ERR "%s: Unexpected common symbol: %s\n",
> +                       elf->name, elf->sym[i].name);
> +                return_(-EINVAL);
> +                break;
> +            case SHN_UNDEF:
> +                printk(XENLOG_ERR "%s: Unknown symbol: %s\n", elf->name,
> +                       elf->sym[i].name);
> +                return_(-ENOENT);
> +                break;
> +            case SHN_ABS:
> +                printk(XENLOG_DEBUG "%s: Absolute symbol: %s => 0x%p\n",
> +                      elf->name, elf->sym[i].name,
> +                      (void *)elf->sym[i].sym->st_value);
> +                break;
> +            default:
> +                if ( elf->sec[elf->sym[i].sym->st_shndx].sec->sh_flags & SHF_ALLOC )
> +                {
> +                    elf->sym[i].sym->st_value +=
> +                        (unsigned long)elf->sec[elf->sym[i].sym->st_shndx].load_addr;
> +                    printk(XENLOG_DEBUG "%s: Symbol resolved: %s => 0x%p\n",
> +                           elf->name, elf->sym[i].name,
> +                           (void *)elf->sym[i].sym->st_value);
> +                }
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +int xsplice_elf_perform_relocs(struct xsplice_elf *elf)
> +{
> +    struct xsplice_elf_sec *rela, *base;
> +    unsigned int i;
> +    int rc;
> +
> +    /*
> +     * The first entry of an ELF symbol table is the "undefined symbol index".
> +     * aka reserved so we skip it.
> +     */
> +    ASSERT( elf->sym );
> +    for ( i = 1; i < elf->hdr->e_shnum; i++ )
> +    {
> +        rela = &elf->sec[i];
> +
> +        if ( (rela->sec->sh_type != SHT_RELA ) &&
> +             (rela->sec->sh_type != SHT_REL ) )
> +            continue;
> +
> +         /* Is it a valid relocation section? */
> +         if ( rela->sec->sh_info >= elf->hdr->e_shnum )
> +            continue;
> +
> +         base = &elf->sec[rela->sec->sh_info];
> +
> +         /* Don't relocate non-allocated sections. */
> +         if ( !(base->sec->sh_flags & SHF_ALLOC) )
> +            continue;
> +
> +        if ( elf->sec[i].sec->sh_type == SHT_RELA )
> +            rc = xsplice_perform_rela(elf, base, rela);
> +        else /* SHT_REL */
> +            rc = xsplice_perform_rel(elf, base, rela);
> +
> +        if ( rc )
> +            return rc;
> +    }
> +
> +    return 0;
> +}

Is there a reason the above two functions weren't put in the previous patch?

> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index bd832df..4ea66bf 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -15,8 +15,10 @@
>
>   #if defined(CONFIG_ARM_64)
>   # define LONG_BYTEORDER 3
> +# define ELFSIZE 64
>   #else
>   # define LONG_BYTEORDER 2
> +# define ELFSIZE 32
>   #endif

What does this do?

(And perhaps it should also be in the previous patch since it's 
mentioned in the previous patch's changelog?)

-- 
Ross Lagerwall