Message ID | 1458849640-22588-18-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
>>> On 24.03.16 at 21:00, <konrad.wilk@oracle.com> wrote: > --- a/xen/common/xsplice.c > +++ b/xen/common/xsplice.c > @@ -573,6 +573,25 @@ static int prepare_payload(struct payload *payload, > region->frame[i].n_bugs = sec->sec->sh_size / sizeof(struct bug_frame); > } > > +#ifdef CONFIG_X86 > + sec = xsplice_elf_sec_by_name(elf, ".ex_table"); > + if ( sec ) > + { > + if ( !sec->sec->sh_size || > + (sec->sec->sh_size % sizeof (struct exception_table_entry)) ) > + { > + dprintk(XENLOG_DEBUG, "%s%s: Wrong size of .ex_table (exp:%lu vs %lu)!\n", > + XSPLICE, elf->name, sizeof (struct exception_table_entry), > + sec->sec->sh_size); > + return -EINVAL; > + } > + > + region->ex = (struct exception_table_entry *)sec->load_addr; > + region->ex_end = (struct exception_table_entry *)(sec->load_addr + sec->sec->sh_size); > + > + sort_exception_table(region->ex, region->ex_end); > + } > +#endif Nothing here is really x86-specific, so the earlier comment on the conditionals better going away applies here too. > --- a/xen/include/asm-x86/uaccess.h > +++ b/xen/include/asm-x86/uaccess.h > @@ -276,6 +276,11 @@ extern struct exception_table_entry > __start___pre_ex_table[]; > extern struct exception_table_entry __stop___pre_ex_table[]; > > extern unsigned long search_exception_table(unsigned long); > +extern unsigned long search_one_extable(const struct exception_table_entry *first, > + const struct exception_table_entry *last, > + unsigned long value); I can't seem to find a use of the outside its defining file. Why is this being made global? Jan
On Fri, Apr 01, 2016 at 10:06:54AM -0600, Jan Beulich wrote: > >>> On 24.03.16 at 21:00, <konrad.wilk@oracle.com> wrote: > > --- a/xen/common/xsplice.c > > +++ b/xen/common/xsplice.c > > @@ -573,6 +573,25 @@ static int prepare_payload(struct payload *payload, > > region->frame[i].n_bugs = sec->sec->sh_size / sizeof(struct bug_frame); > > } > > > > +#ifdef CONFIG_X86 > > + sec = xsplice_elf_sec_by_name(elf, ".ex_table"); > > + if ( sec ) > > + { > > + if ( !sec->sec->sh_size || > > + (sec->sec->sh_size % sizeof (struct exception_table_entry)) ) > > + { > > + dprintk(XENLOG_DEBUG, "%s%s: Wrong size of .ex_table (exp:%lu vs %lu)!\n", > > + XSPLICE, elf->name, sizeof (struct exception_table_entry), > > + sec->sec->sh_size); > > + return -EINVAL; > > + } > > + > > + region->ex = (struct exception_table_entry *)sec->load_addr; > > + region->ex_end = (struct exception_table_entry *)(sec->load_addr + sec->sec->sh_size); > > + > > + sort_exception_table(region->ex, region->ex_end); > > + } > > +#endif > > Nothing here is really x86-specific, so the earlier comment on the > conditionals better going away applies here too. But there is no sort_exception_table on ARM, nor would the sizeof work on ARM? Or are you saying I should add an .. empty function and structure for this? > > > --- a/xen/include/asm-x86/uaccess.h > > +++ b/xen/include/asm-x86/uaccess.h > > @@ -276,6 +276,11 @@ extern struct exception_table_entry > > __start___pre_ex_table[]; > > extern struct exception_table_entry __stop___pre_ex_table[]; > > > > extern unsigned long search_exception_table(unsigned long); > > +extern unsigned long search_one_extable(const struct exception_table_entry *first, > > + const struct exception_table_entry *last, > > + unsigned long value); > > I can't seem to find a use of the outside its defining file. Why is > this being made global? > > Jan >
On 06/04/16 15:41, Konrad Rzeszutek Wilk wrote: > On Fri, Apr 01, 2016 at 10:06:54AM -0600, Jan Beulich wrote: >>>>> On 24.03.16 at 21:00, <konrad.wilk@oracle.com> wrote: >>> --- a/xen/common/xsplice.c >>> +++ b/xen/common/xsplice.c >>> @@ -573,6 +573,25 @@ static int prepare_payload(struct payload *payload, >>> region->frame[i].n_bugs = sec->sec->sh_size / sizeof(struct bug_frame); >>> } >>> >>> +#ifdef CONFIG_X86 >>> + sec = xsplice_elf_sec_by_name(elf, ".ex_table"); >>> + if ( sec ) >>> + { >>> + if ( !sec->sec->sh_size || >>> + (sec->sec->sh_size % sizeof (struct exception_table_entry)) ) >>> + { >>> + dprintk(XENLOG_DEBUG, "%s%s: Wrong size of .ex_table (exp:%lu vs %lu)!\n", >>> + XSPLICE, elf->name, sizeof (struct exception_table_entry), >>> + sec->sec->sh_size); >>> + return -EINVAL; >>> + } >>> + >>> + region->ex = (struct exception_table_entry *)sec->load_addr; >>> + region->ex_end = (struct exception_table_entry *)(sec->load_addr + sec->sec->sh_size); >>> + >>> + sort_exception_table(region->ex, region->ex_end); >>> + } >>> +#endif >> Nothing here is really x86-specific, so the earlier comment on the >> conditionals better going away applies here too. > But there is no sort_exception_table on ARM, nor would the sizeof work > on ARM? Or are you saying I should add an .. empty function and > structure for this? Given the lack of "struct exception_table_entry" entirely on ARM, would recommend keeping the #ifdefs, as being the far cleaner option. Longterm, ARM should gain exception table handing. ~Andrew
>>> Konrad Rzeszutek Wilk <konrad@kernel.org> 04/06/16 4:42 PM >>> >On Fri, Apr 01, 2016 at 10:06:54AM -0600, Jan Beulich wrote: >> >>> On 24.03.16 at 21:00, <konrad.wilk@oracle.com> wrote: >> > --- a/xen/common/xsplice.c >> > +++ b/xen/common/xsplice.c >> > @@ -573,6 +573,25 @@ static int prepare_payload(struct payload *payload, >> > region->frame[i].n_bugs = sec->sec->sh_size / sizeof(struct bug_frame); >> > } >> > >> > +#ifdef CONFIG_X86 >> > + sec = xsplice_elf_sec_by_name(elf, ".ex_table"); >> > + if ( sec ) >> > + { >> > + if ( !sec->sec->sh_size || >> > + (sec->sec->sh_size % sizeof (struct exception_table_entry)) ) >> > + { >> > + dprintk(XENLOG_DEBUG, "%s%s: Wrong size of .ex_table (exp:%lu vs %lu)!\n", >> > + XSPLICE, elf->name, sizeof (struct exception_table_entry), >> > + sec->sec->sh_size); >> > + return -EINVAL; >> > + } >> > + >> > + region->ex = (struct exception_table_entry *)sec->load_addr; >> > + region->ex_end = (struct exception_table_entry *)(sec->load_addr + sec->sec->sh_size); >> > + >> > + sort_exception_table(region->ex, region->ex_end); >> > + } >> > +#endif >> >> Nothing here is really x86-specific, so the earlier comment on the >> conditionals better going away applies here too. > >But there is no sort_exception_table on ARM, nor would the sizeof work >on ARM? Or are you saying I should add an .. empty function and >structure for this? The lack of sort_exception_table() would be easy to deal with, but I indeed overlooked the sizeof(). So I agree with you and Andrew that the conditionals should stay, but I'd like them to be inverted (i.e. test for !ARM). Jan
diff --git a/xen/arch/x86/extable.c b/xen/arch/x86/extable.c index c6c367a..a7f0144 100644 --- a/xen/arch/x86/extable.c +++ b/xen/arch/x86/extable.c @@ -20,7 +20,7 @@ static inline unsigned long ex_cont(const struct exception_table_entry *x) return EX_FIELD(x, cont); } -static int __init cmp_ex(const void *a, const void *b) +static int cmp_ex(const void *a, const void *b) { const struct exception_table_entry *l = a, *r = b; unsigned long lip = ex_addr(l); @@ -35,7 +35,7 @@ static int __init cmp_ex(const void *a, const void *b) } #ifndef swap_ex -static void __init swap_ex(void *a, void *b, int size) +static void swap_ex(void *a, void *b, int size) { struct exception_table_entry *l = a, *r = b, tmp; long delta = b - a; @@ -48,19 +48,23 @@ static void __init swap_ex(void *a, void *b, int size) } #endif -void __init sort_exception_tables(void) +void sort_exception_table(struct exception_table_entry *start, + struct exception_table_entry *stop) { - sort(__start___ex_table, __stop___ex_table - __start___ex_table, - sizeof(struct exception_table_entry), cmp_ex, swap_ex); - sort(__start___pre_ex_table, - __stop___pre_ex_table - __start___pre_ex_table, + sort(start, stop - start, sizeof(struct exception_table_entry), cmp_ex, swap_ex); } -static inline unsigned long -search_one_table(const struct exception_table_entry *first, - const struct exception_table_entry *last, - unsigned long value) +void __init sort_exception_tables(void) +{ + sort_exception_table(__start___ex_table, __stop___ex_table); + sort_exception_table(__start___pre_ex_table, __stop___pre_ex_table); +} + +unsigned long +search_one_extable(const struct exception_table_entry *first, + const struct exception_table_entry *last, + unsigned long value) { const struct exception_table_entry *mid; long diff; @@ -85,7 +89,7 @@ search_exception_table(unsigned long addr) struct virtual_region *region = search_for_text(addr); if ( region && region->ex ) - return search_one_table(region->ex, region->ex_end-1, addr); + return search_one_extable(region->ex, region->ex_end-1, addr); return 0; } @@ -94,7 +98,7 @@ unsigned long search_pre_exception_table(struct cpu_user_regs *regs) { unsigned long addr = (unsigned long)regs->eip; - unsigned long fixup = search_one_table( + unsigned long fixup = search_one_extable( __start___pre_ex_table, __stop___pre_ex_table-1, addr); if ( fixup ) { diff --git a/xen/arch/x86/test/xen_hello_world.c b/xen/arch/x86/test/xen_hello_world.c index 5364114..0f26b06 100644 --- a/xen/arch/x86/test/xen_hello_world.c +++ b/xen/arch/x86/test/xen_hello_world.c @@ -12,6 +12,7 @@ static char xen_hello_world_name[] = "xen_hello_world"; extern const char *xen_hello_world(void); +static unsigned long *non_canonical_addr = (unsigned long *)(1UL<<48); /* External symbol. */ extern const char *xen_extra_version(void); @@ -26,10 +27,20 @@ void apply_hook(void) void revert_hook(void) { + unsigned long tmp = 0xdeadbeef; + int rc; + printk(KERN_DEBUG "Hook unloaded.\n"); /* The hook is called _after_ the unpatching. */ if ( !strcmp(xen_extra_version(), "Hello World") ) BUG(); + /* + * But before unregistering the virtual region. Which means any + * BUG, or WARN_ON will contain symbol name. And also exceptions + * will be caught and processed properly. + */ + rc = __get_user(tmp, non_canonical_addr); + BUG_ON(rc != -EFAULT); } XSPLICE_LOAD_HOOK(apply_hook); diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c index 7b92602..4548b8b 100644 --- a/xen/common/xsplice.c +++ b/xen/common/xsplice.c @@ -573,6 +573,25 @@ static int prepare_payload(struct payload *payload, region->frame[i].n_bugs = sec->sec->sh_size / sizeof(struct bug_frame); } +#ifdef CONFIG_X86 + sec = xsplice_elf_sec_by_name(elf, ".ex_table"); + if ( sec ) + { + if ( !sec->sec->sh_size || + (sec->sec->sh_size % sizeof (struct exception_table_entry)) ) + { + dprintk(XENLOG_DEBUG, "%s%s: Wrong size of .ex_table (exp:%lu vs %lu)!\n", + XSPLICE, elf->name, sizeof (struct exception_table_entry), + sec->sec->sh_size); + return -EINVAL; + } + + region->ex = (struct exception_table_entry *)sec->load_addr; + region->ex_end = (struct exception_table_entry *)(sec->load_addr + sec->sec->sh_size); + + sort_exception_table(region->ex, region->ex_end); + } +#endif return 0; } diff --git a/xen/include/asm-x86/uaccess.h b/xen/include/asm-x86/uaccess.h index 947470d..9e67bf0 100644 --- a/xen/include/asm-x86/uaccess.h +++ b/xen/include/asm-x86/uaccess.h @@ -276,6 +276,11 @@ extern struct exception_table_entry __start___pre_ex_table[]; extern struct exception_table_entry __stop___pre_ex_table[]; extern unsigned long search_exception_table(unsigned long); +extern unsigned long search_one_extable(const struct exception_table_entry *first, + const struct exception_table_entry *last, + unsigned long value); extern void sort_exception_tables(void); +extern void sort_exception_table(struct exception_table_entry *start, + struct exception_table_entry *stop); #endif /* __X86_UACCESS_H__ */