[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v1 09/11] xsplice: Add support for bug frames
On Tue, Nov 03, 2015 at 06:16:06PM +0000, Ross Lagerwall wrote: > Add support for handling bug frames contained with xsplice modules. If a > trap occurs search either the kernel bug table or an applied module's payload. > bug table depending on the instruction pointer. > > Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx> > --- > xen/arch/x86/traps.c | 30 ++++++++----- > xen/common/symbols.c | 7 +++ > xen/common/xsplice.c | 107 > +++++++++++++++++++++++++++++++++++++++++----- > xen/include/xen/kernel.h | 1 + > xen/include/xen/xsplice.h | 4 ++ > 5 files changed, 129 insertions(+), 20 deletions(-) > > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > index b32f696..cd51cfd 100644 > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -48,6 +48,7 @@ > #include <xen/kexec.h> > #include <xen/trace.h> > #include <xen/paging.h> > +#include <xen/xsplice.h> > #include <xen/watchdog.h> > #include <asm/system.h> > #include <asm/io.h> > @@ -1076,20 +1077,29 @@ void do_invalid_op(struct cpu_user_regs *regs) > return; > } > > - if ( !is_active_kernel_text(regs->eip) || > + if ( !is_active_text(regs->eip) || > __copy_from_user(bug_insn, eip, sizeof(bug_insn)) || > memcmp(bug_insn, "\xf\xb", sizeof(bug_insn)) ) > goto die; > > - for ( bug = __start_bug_frames, id = 0; stop_frames[id]; ++bug ) > + if ( likely(is_active_kernel_text(regs->eip)) ) > { > - while ( unlikely(bug == stop_frames[id]) ) > - ++id; > - if ( bug_loc(bug) == eip ) > - break; > + for ( bug = __start_bug_frames, id = 0; stop_frames[id]; ++bug ) > + { > + while ( unlikely(bug == stop_frames[id]) ) > + ++id; > + if ( bug_loc(bug) == eip ) > + break; > + } > + if ( !stop_frames[id] ) > + goto die; > + } > + else > + { > + bug = xsplice_find_bug(eip, &id); > + if ( !bug ) > + goto die; > } > - if ( !stop_frames[id] ) > - goto die; > > eip += sizeof(bug_insn); > if ( id == BUGFRAME_run_fn ) > @@ -1103,7 +1113,7 @@ void do_invalid_op(struct cpu_user_regs *regs) > > /* WARN, BUG or ASSERT: decode the filename pointer and line number. */ > filename = bug_ptr(bug); > - if ( !is_kernel(filename) ) > + if ( !is_kernel(filename) && !is_module(filename) ) > goto die; > fixup = strlen(filename); > if ( fixup > 50 ) > @@ -1130,7 +1140,7 @@ void do_invalid_op(struct cpu_user_regs *regs) > case BUGFRAME_assert: > /* ASSERT: decode the predicate string pointer. */ > predicate = bug_msg(bug); > - if ( !is_kernel(predicate) ) > + if ( !is_kernel(predicate) && !is_module(predicate) ) is_payload ? > predicate = "<unknown>"; > > printk("Assertion '%s' failed at %s%s:%d\n", > diff --git a/xen/common/symbols.c b/xen/common/symbols.c > index a59c59d..bf5623f 100644 > --- a/xen/common/symbols.c > +++ b/xen/common/symbols.c > @@ -17,6 +17,7 @@ > #include <xen/lib.h> > #include <xen/string.h> > #include <xen/spinlock.h> > +#include <xen/xsplice.h> > #include <public/platform.h> > #include <xen/guest_access.h> > > @@ -101,6 +102,12 @@ bool_t is_active_kernel_text(unsigned long addr) > (system_state < SYS_STATE_active && is_kernel_inittext(addr))); > } > > +bool_t is_active_text(unsigned long addr) > +{ > + return is_active_kernel_text(addr) || > + is_active_module_text(addr); Ditto? > +} > + > const char *symbols_lookup(unsigned long addr, > unsigned long *symbolsize, > unsigned long *offset, > diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c > index 4476be5..982954b 100644 > --- a/xen/common/xsplice.c > +++ b/xen/common/xsplice.c > @@ -40,6 +40,11 @@ struct payload { > int nfuncs; > void *module_address; > size_t module_pages; > + size_t core_size; > + size_t core_text_size; > + > + struct bug_frame *start_bug_frames[4]; > + struct bug_frame *stop_bug_frames[4]; You need a #define for the 4 value. > > char id[XEN_XSPLICE_NAME_SIZE + 1]; /* Name of it. */ > }; > @@ -525,26 +530,27 @@ static void free_module(struct payload *payload) > payload->module_pages = 0; > } > > -static void alloc_section(struct xsplice_elf_sec *sec, size_t *core_size) > +static void alloc_section(struct xsplice_elf_sec *sec, size_t *size) > { > - size_t align_size = ROUNDUP(*core_size, sec->sec->sh_addralign); > + size_t align_size = ROUNDUP(*size, sec->sec->sh_addralign); > sec->sec->sh_entsize = align_size; > - *core_size = sec->sec->sh_size + align_size; > + *size = sec->sec->sh_size + align_size; > } That looks to be unrelated to this patch. Perhaps squash it in earlier? > > static int move_module(struct payload *payload, struct xsplice_elf *elf) > { > uint8_t *buf; > int i; > - size_t core_size = 0; > + size_t size = 0; > > /* Allocate text regions */ > 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); > + alloc_section(&elf->sec[i], &size); > } > + payload->core_text_size = size; > > /* Allocate rw data */ > for ( i = 0; i < elf->hdr->e_shnum; i++ ) > @@ -552,7 +558,7 @@ static int move_module(struct payload *payload, struct > xsplice_elf *elf) > 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); > + alloc_section(&elf->sec[i], &size); > } > > /* Allocate ro data */ > @@ -561,15 +567,16 @@ static int move_module(struct payload *payload, struct > xsplice_elf *elf) > 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); > + alloc_section(&elf->sec[i], &size); > } > + payload->core_size = size; > > - buf = alloc_module(core_size); > + buf = alloc_module(size); > if ( !buf ) { > printk(XENLOG_ERR "Could not allocate memory for module\n"); > return -ENOMEM; > } > - memset(buf, 0, core_size); > + memset(buf, 0, size); > > for ( i = 0; i < elf->hdr->e_shnum; i++ ) > { > @@ -584,7 +591,7 @@ static int move_module(struct payload *payload, struct > xsplice_elf *elf) > } > > payload->module_address = buf; > - payload->module_pages = PFN_UP(core_size); > + payload->module_pages = PFN_UP(size); > > return 0; > } > @@ -659,6 +666,7 @@ static int find_special_sections(struct payload *payload, > struct xsplice_elf *elf) > { > struct xsplice_elf_sec *sec; > + int i; unsigned int > > sec = xsplice_elf_sec_by_name(elf, ".xsplice.funcs"); > if ( !sec ) > @@ -670,6 +678,19 @@ static int find_special_sections(struct payload *payload, > payload->funcs = (struct xsplice_patch_func *)sec->load_addr; > payload->nfuncs = sec->sec->sh_size / (sizeof *payload->funcs); > > + for ( i = 0; i < 4; i++ ) use the #define. > + { > + char str[14]; And this needs a #define as well. > + > + snprintf(str, sizeof str, ".bug_frames.%d", i); > + sec = xsplice_elf_sec_by_name(elf, str); > + if ( !sec ) > + continue; > + > + payload->start_bug_frames[i] = (struct bug_frame *)sec->load_addr; > + payload->stop_bug_frames[i] = (struct bug_frame *)(sec->load_addr + > sec->sec->sh_size); You probably should double check that the 'sh_size' is not greater than 4. > + } > + > return 0; > } > > @@ -912,6 +933,72 @@ void do_xsplice(void) > } > } > > + > +/* > + * Functions for handling special sections. > + */ > +struct bug_frame *xsplice_find_bug(const char *eip, int *id) > +{ > + struct payload *data; > + struct bug_frame *bug; > + int i; unsigned int; > + > + /* No locking since this list is only ever changed during apply or revert > + * context. */ Please use hte different style of comments: /* * blah blah blah */ > + list_for_each_entry ( data, &applied_list, applied_list ) > + { > + for (i = 0; i < 4; i++) { > + if (!data->start_bug_frames[i]) > + continue; > + if ( !((void *)eip >= data->module_address && > + (void *)eip < (data->module_address + > data->core_text_size))) > + continue; > + > + for ( bug = data->start_bug_frames[i]; bug != > data->stop_bug_frames[i]; ++bug ) { Could we ever have the situation of where there is a NULL structure within start_bug_frames and stop_bug_frames? [Say the file is corrupted] Perhaps we should double-check that in find_special_sections? > + if ( bug_loc(bug) == eip ) > + { > + *id = i; > + return bug; > + } > + } > + } > + } > + > + return NULL; > +} > + > +bool_t is_module(const void *ptr) > +{ > + struct payload *data; > + > + /* No locking since this list is only ever changed during apply or revert > + * context. */ > + list_for_each_entry ( data, &applied_list, applied_list ) > + { > + if ( ptr >= data->module_address && > + ptr < (data->module_address + data->core_size)) > + return true; > + } > + > + return false; > +} > + > +bool_t is_active_module_text(unsigned long addr) > +{ > + struct payload *data; > + > + /* No locking since this list is only ever changed during apply or revert > + * context. */ > + list_for_each_entry ( data, &applied_list, applied_list ) > + { > + if ( (void *)addr >= data->module_address && > + (void *)addr < (data->module_address + data->core_text_size)) > + return true; > + } > + > + return false; > +} > + Those two look very very much the same. Could one of them call the other? Actually why not just have one? And do some casting? > static int __init xsplice_init(void) > { > register_keyhandler('x', xsplice_printall, "print xsplicing info", 1); > diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h > index 548b64d..df57754 100644 > --- a/xen/include/xen/kernel.h > +++ b/xen/include/xen/kernel.h > @@ -99,6 +99,7 @@ extern enum system_state { > } system_state; > > bool_t is_active_kernel_text(unsigned long addr); > +bool_t is_active_text(unsigned long addr); > > #endif /* _LINUX_KERNEL_H */ > > diff --git a/xen/include/xen/xsplice.h b/xen/include/xen/xsplice.h > index 507829c..772fa3a 100644 > --- a/xen/include/xen/xsplice.h > +++ b/xen/include/xen/xsplice.h > @@ -22,6 +22,10 @@ extern void xsplice_printall(unsigned char key); > > 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); > + > /* Arch hooks */ > int xsplice_verify_elf(uint8_t *data, ssize_t len); > int xsplice_perform_rel(struct xsplice_elf *elf, > -- > 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 |