[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v1 07/11] xsplice: Implement payload loading
On Tue, Nov 03, 2015 at 06:16:04PM +0000, Ross Lagerwall wrote: > 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. > - Process xsplice specific sections. > > Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx> > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/xsplice.c | 23 ++++ > xen/arch/x86/Makefile | 1 + > xen/arch/x86/setup.c | 7 + > xen/arch/x86/xsplice.c | 90 ++++++++++++ > xen/common/xsplice.c | 282 > ++++++++++++++++++++++++++++++++++++++ > xen/include/asm-x86/x86_64/page.h | 2 + > xen/include/xen/xsplice.h | 22 +++ > 8 files changed, 428 insertions(+) > 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 1ef39f7..f785c07 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -39,6 +39,7 @@ obj-y += device.o > obj-y += decode.o > obj-y += processor.o > obj-y += smc.o > +obj-y += 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..8d85fa9 > --- /dev/null > +++ b/xen/arch/arm/xsplice.c > @@ -0,0 +1,23 @@ > +#include <xen/lib.h> > +#include <xen/errno.h> > +#include <xen/xsplice_elf.h> > +#include <xen/xsplice.h> > + > +int xsplice_verify_elf(uint8_t *data, ssize_t len) > +{ > + return -ENOSYS; > +} > + > +int xsplice_perform_rel(struct xsplice_elf *elf, > + struct xsplice_elf_sec *base, > + struct xsplice_elf_sec *rela) > +{ > + return -ENOSYS; > +} > + > +int xsplice_perform_rela(struct xsplice_elf *elf, > + struct xsplice_elf_sec *base, > + struct xsplice_elf_sec *rela) > +{ > + return -ENOSYS; > +} > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile > index 39a8059..6e05532 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-y += xsplice.o > obj-y += xstate.o > > obj-$(crash_debug) += gdbstub.o > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index 4ed0110..a79c5e3 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -99,6 +99,9 @@ unsigned long __read_mostly xen_phys_start; > > unsigned long __read_mostly xen_virt_end; > > +unsigned long __read_mostly module_virt_start; > +unsigned long __read_mostly module_virt_end; > + > DEFINE_PER_CPU(struct tss_struct, init_tss); > > char __section(".bss.stack_aligned") cpu0_stack[STACK_SIZE]; > @@ -1145,6 +1148,10 @@ void __init noreturn __start_xen(unsigned long mbi_p) > ~((1UL << L2_PAGETABLE_SHIFT) - 1); > destroy_xen_mappings(xen_virt_end, XEN_VIRT_START + BOOTSTRAP_MAP_BASE); > > + module_virt_start = xen_virt_end; > + module_virt_end = XEN_VIRT_END - NR_CPUS * PAGE_SIZE; > + BUG_ON(module_virt_end <= module_virt_start); > + > memguard_init(); > > nr_pages = 0; > diff --git a/xen/arch/x86/xsplice.c b/xen/arch/x86/xsplice.c > new file mode 100644 > index 0000000..dbff0d5 > --- /dev/null > +++ b/xen/arch/x86/xsplice.c > @@ -0,0 +1,90 @@ You would want to put Citrix regular Copyright header here. > +#include <xen/lib.h> > +#include <xen/errno.h> > +#include <xen/xsplice_elf.h> > +#include <xen/xsplice.h> > + > +int xsplice_verify_elf(uint8_t *data, ssize_t len) > +{ > + > + Elf64_Ehdr *hdr = (Elf64_Ehdr *)data; > + > + if ( len < (sizeof *hdr) || > + !IS_ELF(*hdr) || > + hdr->e_ident[EI_CLASS] != ELFCLASS64 || > + hdr->e_ident[EI_DATA] != ELFDATA2LSB || > + hdr->e_machine != EM_X86_64 ) > + { > + printk(XENLOG_ERR "Invalid ELF file\n"); For audit reasons I think we should have at least the name (id) that the payload was. Could we include that as argument and print it here? > + return -EINVAL; > + } > + > + return 0; > +} > + > +int xsplice_perform_rel(struct xsplice_elf *elf, > + struct xsplice_elf_sec *base, > + struct xsplice_elf_sec *rela) > +{ > + printk(XENLOG_ERR "SHT_REL relocation unsupported\n"); %s: SHR_REL relocation ..\n", elf->name); > + return -ENOSYS; > +} > + > +int xsplice_perform_rela(struct xsplice_elf *elf, > + struct xsplice_elf_sec *base, > + struct xsplice_elf_sec *rela) > +{ > + Elf64_Rela *r; > + int symndx, i; unsigned int > + uint64_t val; > + uint8_t *dest; > + Can you double check that rela->sec-sh_entsize is not zero first? > + for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ ) > + { > + r = (Elf64_Rela *)(rela->data + i * rela->sec->sh_entsize); Can you check that that 'r' is not past the memory allocated for 'data'? > + symndx = ELF64_R_SYM(r->r_info); > + dest = base->load_addr + r->r_offset; > + val = r->r_addend + elf->sym[symndx].sym->st_value; Can you check that 'symndx' is not past the what we allocated for elf->sym? > + > + switch ( ELF64_R_TYPE(r->r_info) ) > + { > + case R_X86_64_NONE: > + break; > + case R_X86_64_64: > + *(uint64_t *)dest = val; > + break; > + case R_X86_64_32: > + *(uint32_t *)dest = val; > + if (val != *(uint32_t *)dest) > + goto overflow; > + break; > + case R_X86_64_32S: > + *(int32_t *)dest = val; > + if ((int64_t)val != *(int32_t *)dest) > + goto overflow; > + 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. > + */ > + /* Fall through */ > + case R_X86_64_PC32: > + *(uint32_t *)dest = val - (uint64_t)dest; > + break; > + default: > + printk(XENLOG_ERR "Unhandled relocation %lu\n", > + ELF64_R_TYPE(r->r_info)); > + return -EINVAL; > + } > + } > + > + return 0; > + > + overflow: > + printk(XENLOG_ERR "Overflow in relocation %d in %s\n", i, rela->name); > + return -EOVERFLOW; > +} > diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c > index d984c8a..5e88c55 100644 > --- a/xen/common/xsplice.c > +++ b/xen/common/xsplice.c > @@ -12,6 +12,7 @@ > #include <xen/stdbool.h> > #include <xen/sched.h> > #include <xen/lib.h> > +#include <xen/xsplice_elf.h> > #include <xen/xsplice.h> > #include <public/sysctl.h> > > @@ -29,9 +30,15 @@ struct payload { > > struct list_head list; /* Linked to 'payload_list'. */ > > + void *module_address; > + size_t module_pages; > + > char id[XEN_XSPLICE_NAME_SIZE + 1]; /* Name of it. */ > }; > > +static int load_module(struct payload *payload, uint8_t *raw, ssize_t len); > +static void free_module(struct payload *payload); > + > static const char *state2str(int32_t state) > { > #define STATE(x) [XSPLICE_STATE_##x] = #x > @@ -140,6 +147,7 @@ static void __free_payload(struct payload *data) > list_del(&data->list); > payload_cnt --; > payload_version ++; > + free_module(data); > xfree(data); > } > > @@ -178,6 +186,10 @@ static int xsplice_upload(xen_sysctl_xsplice_upload_t > *upload) > if ( copy_from_guest(raw_data, upload->payload, upload->size) ) > goto err_raw; > > + rc = load_module(data, raw_data, upload->size); > + if ( rc ) > + goto err_raw; > + > data->state = XSPLICE_STATE_LOADED; > data->rc = 0; > INIT_LIST_HEAD(&data->list); > @@ -390,6 +402,276 @@ int xsplice_control(xen_sysctl_xsplice_op_t *xsplice) > return rc; > } > > + > +/* > + * The following functions prepare an xSplice module to be executed by > + * allocating space, loading the allocated sections, resolving symbols, > + * performing relocations, etc. > + */ > +#ifdef CONFIG_X86 > +static void *alloc_module(size_t size) s/module/payload/ > +{ > + mfn_t *mfn, *mfn_ptr; > + size_t pages, i; > + struct page_info *pg; > + unsigned long hole_start, hole_end, cur; > + struct payload *data, *data2; > + > + ASSERT(size); > + > + 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)); > + } This looks like 'vmalloc'. Why not use that? (That explanation should be part of the commit description probably) > + > + hole_start = (unsigned long)module_virt_start; > + hole_end = hole_start + pages * PAGE_SIZE; > + 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->module_address; > + end = start + data2->module_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 could be made in a nice function. 'find_hole' perhaps? > + > + if ( hole_end >= module_virt_end ) > + goto error; > + > + for ( cur = hole_start, mfn_ptr = mfn; pages--; ++mfn_ptr, cur += > PAGE_SIZE ) > + { > + if ( map_pages_to_xen(cur, mfn_x(*mfn_ptr), 1, PAGE_HYPERVISOR_RWX) ) > + { > + if ( cur != hole_start ) > + destroy_xen_mappings(hole_start, cur); I think 'destroy_xen_mappings' is OK handling hole_start == cur. > + goto error; > + } > + } > + xfree(mfn); > + return (void *)hole_start; > + > + error: > + while ( i-- ) > + free_domheap_page(mfn_to_page(mfn_x(mfn[i]))); > + xfree(mfn); > + return NULL; > +} > +#else > +static void *alloc_module(size_t size) s/module/payload/ > +{ > + return NULL; > +} > +#endif > + > +static void free_module(struct payload *payload) > +{ > + int i; unsigned int; > + struct page_info *pg; > + PAGE_LIST_HEAD(pg_list); > + void *va = payload->module_address; > + unsigned long addr = (unsigned long)va; > + > + if ( !payload->module_address ) > + return; How about 'if ( !addr ) return; ? > + > + payload->module_address = NULL; > + > + for ( i = 0; i < payload->module_pages; i++ ) > + page_list_add(vmap_to_page(va + i * PAGE_SIZE), &pg_list); > + > + destroy_xen_mappings(addr, addr + payload->module_pages * PAGE_SIZE); > + > + while ( (pg = page_list_remove_head(&pg_list)) != NULL ) > + free_domheap_page(pg); > + > + payload->module_pages = 0; > +} > + > +static void alloc_section(struct xsplice_elf_sec *sec, size_t *core_size) s/alloc/compute/? > +{ > + size_t align_size = ROUNDUP(*core_size, sec->sec->sh_addralign); > + sec->sec->sh_entsize = align_size; > + *core_size = sec->sec->sh_size + align_size; > +} > + > +static int move_module(struct payload *payload, struct xsplice_elf *elf) > +{ > + uint8_t *buf; > + int i; unsigned int i; > + size_t core_size = 0; > + > + /* Allocate text regions */ s/Allocate/Compute/ > + for ( i = 0; i < elf->hdr->e_shnum; i++ ) > + { > + if ( (elf->sec[i].sec->sh_flags & (SHF_ALLOC|SHF_EXECINSTR)) == > + (SHF_ALLOC|SHF_EXECINSTR) ) > + alloc_section(&elf->sec[i], &core_size); > + } > + > + /* Allocate rw data */ > + for ( i = 0; 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) ) > + alloc_section(&elf->sec[i], &core_size); > + } > + > + /* Allocate ro data */ > + for ( i = 0; 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) ) > + alloc_section(&elf->sec[i], &core_size); > + } > + > + buf = alloc_module(core_size); > + if ( !buf ) { > + printk(XENLOG_ERR "Could not allocate memory for module\n"); (%s: Could not allocate %u memory for payload!\n", elf->name, core_size); > + return -ENOMEM; > + } > + memset(buf, 0, core_size); Perhaps for fun it ought to be 'ud2' ? > + > + for ( i = 0; i < elf->hdr->e_shnum; i++ ) > + { > + if ( elf->sec[i].sec->sh_flags & SHF_ALLOC ) > + { > + elf->sec[i].load_addr = buf + elf->sec[i].sec->sh_entsize; > + memcpy(elf->sec[i].load_addr, elf->sec[i].data, > + elf->sec[i].sec->sh_size); > + printk(XENLOG_DEBUG "Loaded %s at 0x%p\n", Add %s: at the start .. > + elf->sec[i].name, elf->sec[i].load_addr); which would be elf->name. > + } > + } > + > + payload->module_address = buf; > + payload->module_pages = PFN_UP(core_size); Instead of module could we name it payload? > + > + return 0; > +} > + > +static int resolve_symbols(struct xsplice_elf *elf) s/resolve/check/ > +{ > + int i; unsigned int; > + > + for ( i = 1; i < elf->nsym; i++ ) Why 1? Please explain as comment. > + { > + switch ( elf->sym[i].sym->st_shndx ) > + { > + case SHN_COMMON: > + printk(XENLOG_ERR "Unexpected common symbol: %s\n", > + elf->sym[i].name); Please also include elf->name in the error. > + return -EINVAL; > + break; > + case SHN_UNDEF: > + printk(XENLOG_ERR "Unknown symbol: %s\n", elf->sym[i].name); Ditto. > + return -ENOENT; > + break; > + case SHN_ABS: > + printk(XENLOG_DEBUG "Absolute symbol: %s => 0x%p\n", > + 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 "Symbol resolved: %s => 0x%p\n", Ditto; > + elf->sym[i].name, (void > *)elf->sym[i].sym->st_value); > + } > + } > + } > + > + return 0; > +} > + > +static int perform_relocs(struct xsplice_elf *elf) > +{ > + struct xsplice_elf_sec *rela, *base; > + int i, rc; > + unsigned int i; > + for ( i = 0; i < elf->hdr->e_shnum; i++ ) > + { > + rela = &elf->sec[i]; > + > + /* Is it a valid relocation section? */ > + if ( rela->sec->sh_info >= elf->hdr->e_shnum ) > + continue; Um, don't we want to mark it as invalid or such? Or overwrite it so we won't use it? > + > + 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 if ( elf->sec[i].sec->sh_type == SHT_REL ) > + rc = xsplice_perform_rel(elf, base, rela); > + > + if ( rc ) > + return rc; > + } > + > + return 0; > +} > + > +static int load_module(struct payload *payload, uint8_t *raw, ssize_t len) > +{ > + struct xsplice_elf elf; Wait a minute? We ditch it after this? > + int rc = 0; > + > + rc = xsplice_verify_elf(raw, len); > + if ( rc ) > + return rc; > + > + rc = xsplice_elf_load(&elf, raw, len); > + if ( rc ) > + return rc; > + > + rc = move_module(payload, &elf); > + if ( rc ) > + goto err_elf; > + > + rc = resolve_symbols(&elf); > + if ( rc ) > + goto err_module; > + > + rc = perform_relocs(&elf); > + if ( rc ) > + goto err_module; > + Shouldn't you call xsplice_elf_free(&elf) here? Or hook up the elf to the 'struct payload'? If not, who is going to clean up elf->sec and elf->sym when the payload is unloaded? > + return 0; > + > + err_module: > + free_module(payload); > + err_elf: > + xsplice_elf_free(&elf); > + > + return rc; > +} > + > static int __init xsplice_init(void) > { > register_keyhandler('x', xsplice_printall, "print xsplicing info", 1); > diff --git a/xen/include/asm-x86/x86_64/page.h > b/xen/include/asm-x86/x86_64/page.h > index 19ab4d0..e6f08e9 100644 > --- a/xen/include/asm-x86/x86_64/page.h > +++ b/xen/include/asm-x86/x86_64/page.h > @@ -38,6 +38,8 @@ > #include <xen/pdx.h> > > extern unsigned long xen_virt_end; > +extern unsigned long module_virt_start; > +extern unsigned long module_virt_end; > > #define spage_to_pdx(spg) (((spg) - > spage_table)<<(SUPERPAGE_SHIFT-PAGE_SHIFT)) > #define pdx_to_spage(pdx) (spage_table + > ((pdx)>>(SUPERPAGE_SHIFT-PAGE_SHIFT))) > diff --git a/xen/include/xen/xsplice.h b/xen/include/xen/xsplice.h > index 41e28da..a3946a3 100644 > --- a/xen/include/xen/xsplice.h > +++ b/xen/include/xen/xsplice.h > @@ -1,9 +1,31 @@ > #ifndef __XEN_XSPLICE_H__ > #define __XEN_XSPLICE_H__ > > +struct xsplice_elf; > +struct xsplice_elf_sec; > +struct xsplice_elf_sym; > + > +struct xsplice_patch_func { > + unsigned long new_addr; > + unsigned long new_size; > + unsigned long old_addr; > + unsigned long old_size; > + char *name; > + unsigned char undo[8]; > +}; We don't use them in this patch. They could be moved to another patch. > + > struct xen_sysctl_xsplice_op; > int xsplice_control(struct xen_sysctl_xsplice_op *); > > extern void xsplice_printall(unsigned char key); > > +/* Arch hooks */ > +int xsplice_verify_elf(uint8_t *data, ssize_t len); > +int xsplice_perform_rel(struct xsplice_elf *elf, > + struct xsplice_elf_sec *base, > + struct xsplice_elf_sec *rela); > +int xsplice_perform_rela(struct xsplice_elf *elf, > + struct xsplice_elf_sec *base, > + struct xsplice_elf_sec *rela); > + > #endif /* __XEN_XSPLICE_H__ */ > -- > 2.4.3 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxx > http://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |