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 <[email protected]>
> ---
>  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
[email protected]
https://www.redhat.com/mailman/listinfo/crash-utility

Reply via email to