Yes, I did miss the second question, sorry about that. Yes, I agree it's best not to disturb the 32-bit behavior that has been working. I'd prefer not to restrict it to instances where --kaslr was specified on the command line because I'd like to submit a patch in the near future that auto-detects the aslr offset and makes the argument unnecessary. How about a check for MACHINE_TYPE("X86_64") for this behavior while MACHINE_TYPE("X86") gets existing behavior? On Wed, Oct 16, 2013 at 1:21 PM, Dave Anderson <anderson@xxxxxxxxxx> wrote: > > Andy, > > You must have missed my second question about the return value > of relocate() -- see below: > > ----- Original Message ----- >> >> This patch adds a --kaslr command line parameter for loading x86_64 >> crash dumps with kaslr enabled. This reuses the code from 32-bit >> x86 relocations with some small changes. The ASLR offset is postive >> instead of negative. Also had to move the code to traverse the >> kernel section before the symbol storing code to figure out which >> symbols were outside any sections and therefore were not relocated. >> >> Also made a very small change in search_for_switch_to it was >> searching through gdb command output which can be different under >> different settings. >> >> Tested: Tested by loading kdump files from kernels with aslr enabled >> and not enabled. Ran bt, files, and struct file 0xXXXXXX. >> >> Signed-off-by: Andy Honig <ahonig@xxxxxxxxxx> >> --- >> defs.h | 2 ++ >> main.c | 8 ++++++-- >> symbols.c | 66 >> +++++++++++++++++++++++++++++++++++++++++++++------------------ >> x86_64.c | 20 +++++++++++++------ >> 4 files changed, 69 insertions(+), 27 deletions(-) >> >> diff --git a/defs.h b/defs.h >> index 83a4402..8de1fa4 100755 >> --- a/defs.h >> +++ b/defs.h >> @@ -2394,6 +2394,8 @@ struct symbol_table_data { >> ulong __per_cpu_end; >> off_t dwarf_debug_frame_file_offset; >> ulong dwarf_debug_frame_size; >> + ulong first_section_start; >> + ulong last_section_end; >> }; >> >> /* flags for st */ >> diff --git a/main.c b/main.c >> index 3b469e3..5a41c1a 100755 >> --- a/main.c >> +++ b/main.c >> @@ -57,6 +57,7 @@ static struct option long_options[] = { >> {"CRASHPAGER", 0, 0, 0}, >> {"no_scroll", 0, 0, 0}, >> {"reloc", required_argument, 0, 0}, >> + {"kaslr", required_argument, 0, 0}, >> {"active", 0, 0, 0}, >> {"minimal", 0, 0, 0}, >> {"mod", required_argument, 0, 0}, >> @@ -216,12 +217,15 @@ main(int argc, char **argv) >> else if (STREQ(long_options[option_index].name, "mod")) >> kt->module_tree = optarg; >> >> - else if (STREQ(long_options[option_index].name, "reloc")) { >> + else if (STREQ(long_options[option_index].name, "reloc") || >> + STREQ(long_options[option_index].name, "kaslr")) { >> if (!calculate(optarg, &kt->relocate, NULL, 0)) { >> error(INFO, "invalid --reloc argument: %s\n", >> optarg); >> program_usage(SHORT_FORM); >> - } >> + } else if (STREQ(long_options[option_index].name, "kaslr")) { >> + kt->relocate *= -1; >> + } >> kt->flags |= RELOC_SET; >> } >> >> diff --git a/symbols.c b/symbols.c >> index 93d9c8c..345c0de 100755 >> --- a/symbols.c >> +++ b/symbols.c >> @@ -192,22 +192,6 @@ symtab_init(void) >> if (!check_gnu_debuglink(st->bfd)) >> no_debugging_data(FATAL); >> } >> - >> - symcount = bfd_read_minisymbols(st->bfd, FALSE, &minisyms, &size); >> - >> - if (symcount <= 0) >> - no_debugging_data(FATAL); >> - >> - sort_x = bfd_make_empty_symbol(st->bfd); >> - sort_y = bfd_make_empty_symbol(st->bfd); >> - if (sort_x == NULL || sort_y == NULL) >> - error(FATAL, "bfd_make_empty_symbol() failed\n"); >> - >> - gnu_qsort(st->bfd, minisyms, symcount, size, sort_x, sort_y); >> - >> - store_symbols(st->bfd, FALSE, minisyms, symcount, size); >> - >> - free(minisyms); >> >> /* >> * Gather references to the kernel sections. >> @@ -217,6 +201,7 @@ symtab_init(void) >> error(FATAL, "symbol table section array malloc: %s\n", >> strerror(errno)); >> BZERO(st->sections, st->bfd->section_count * sizeof(struct sec *)); >> + st->first_section_start = st->last_section_end = 0; >> >> bfd_map_over_sections(st->bfd, section_header_info, KERNEL_SECTIONS); >> if ((st->flags & (NO_SEC_LOAD|NO_SEC_CONTENTS)) == >> @@ -227,6 +212,23 @@ symtab_init(void) >> error(FATAL, DEBUGINFO_ERROR_MESSAGE2); >> } >> } >> + >> + symcount = bfd_read_minisymbols(st->bfd, FALSE, &minisyms, &size); >> + >> + if (symcount <= 0) >> + no_debugging_data(FATAL); >> + >> + sort_x = bfd_make_empty_symbol(st->bfd); >> + sort_y = bfd_make_empty_symbol(st->bfd); >> + if (sort_x == NULL || sort_y == NULL) >> + error(FATAL, "bfd_make_empty_symbol() failed\n"); >> + >> + gnu_qsort(st->bfd, minisyms, symcount, size, sort_x, sort_y); >> + >> + store_symbols(st->bfd, FALSE, minisyms, symcount, size); >> + >> + free(minisyms); >> + >> >> symname_hash_init(); >> symval_hash_init(); >> @@ -585,7 +587,7 @@ store_symbols(bfd *abfd, int dynamic, void *minisyms, >> long symcount, >> st->symcnt = 0; >> sp = st->symtable; >> >> - if (machine_type("X86")) { >> + if (machine_type("X86") || machine_type("X86_64")) { >> if (!(kt->flags & RELOC_SET)) >> kt->flags |= RELOC_FORCE; >> } else >> @@ -658,7 +660,7 @@ store_sysmap_symbols(void) >> error(FATAL, "symbol table namespace malloc: %s\n", >> strerror(errno)); >> >> - if (!machine_type("X86")) >> + if (!machine_type("X86") && !machine_type("X86_64")) >> kt->flags &= ~RELOC_SET; >> >> first = 0; >> @@ -730,7 +732,17 @@ relocate(ulong symval, char *symname, int first_symbol) >> break; >> } >> >> - return (symval - kt->relocate); >> + /* >> + * There are some symbols which are outside of any section >> + * either because they are offsets or because they are absolute >> + * addresses. These should not be relocated. >> + */ >> + if (symval >= st->first_section_start && >> + symval <= st->last_section_end) { >> + return (symval - kt->relocate); >> + } else { >> + return symval; >> + } >> } > > The change above modifies the behavior for relocated 32-bit x86 relocated kernels > (i.e., not -kalsr x86 kernels). > > I've never had any reports of problems with the old return value, so I'm a > little nervous about changing it for both arches. Should the change above > be restricted to --kalsr kernels? > > Dave > > > -- > Crash-utility mailing list > Crash-utility@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/crash-utility -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility