[Crash-utility] [PATCH v3] Add support for kASLR for offline vmcore files
Kees Cook
keescook at google.com
Tue Jan 21 21:07:22 UTC 2014
On Tue, Jan 21, 2014 at 12:03 PM, Dave Anderson <anderson at redhat.com> wrote:
>
>
> ----- Original Message -----
>> On Tue, Jan 21, 2014 at 11:32 AM, Dave Anderson <anderson at redhat.com> wrote:
>> >
>> > Hi Andy,
>> >
>> > Since the kASLR code is being pulled into 3.14, I was considering
>> > queueing this v3 patch for crash-7.0.5. But given this LKML post
>> > today:
>> >
>> > https://lkml.org/lkml/2014/1/21/339
>> >
>> > Date: Tue, 21 Jan 2014 10:37:00 -0800
>> > Subject: Re: [GIT PULL] x86/kaslr for v3.14
>> > From: Kees Cook <>
>> >
>> > On Tue, Jan 21, 2014 at 6:20 AM, H. Peter Anvin <hpa at zytor.com> wrote:
>> > > On 01/21/2014 01:00 AM, Peter Zijlstra wrote:
>> > >>>
>> > >>> So this is presumably something that needs to be fixed in perf?
>> > >>
>> > >> Where do we learn about the offset from userspace?
>> > >
>> > > Now this is tricky... if this offset is too easy to get it completely
>> > > defeats kASLR. On the other hand, I presume that if we are exporting
>> > > /proc/kcore we're not secure anyway. Kees, I assume that in "secure"
>> > > mode perf annotations simply wouldn't work anyway?
>> >
>> > The goal scope of the kernel base address randomization is to keep it
>> > secret from non-root users, confined processes, and/or remote systems.
>> > For local secrecy, if you're running with kaslr and you haven't set
>> > kptr_restrict, dmesg_restrict, and perf_event_paranoid, that's a
>> > problem since you're likely leaking things trivially through
>> > /proc/kallsyms, dmesg, and/or perf.
>> >
>> > -Kees
>> >
>> > --
>> > Kees Cook
>> > Chrome OS Security
>> >
>> > Then, my questions are:
>> >
>> > (1) on a live system, how would a root user determine the offset from
>> > userspace?
>>
>> AFAICT, it can be calculated from /proc/kallsyms.
>
> Will /proc/kallsyms contain the relocated addresses? Andy had mentioned that
> the offset would be in the dmesg buffer but that can be overwritten.
Yeah, kallsyms should show the current actual locations. It should
only show up in dmesg on a crash.
>> > (2) given a random vmlinux/vmcore pair, how would any user determine the
>> > offset?
>>
>> It'd be nice for the vmcore to contain offset details.
>
> Right -- Andy mentioned that it would be put in a VMCOREINFO item:
>
> https://www.redhat.com/archives/crash-utility/2013-October/msg00043.html
>
> But I'm presuming that wasn't part of your patchset.
It was not, no. What's needed to get that added?
> Anyway, looking back at that post, I'll defer adding this patch until
> Andy updates it, or at least confirms that it might be useful as-is
> for now.
Okay, cool. I'm happy to help however is needed. :)
Thanks!
-Kees
>
> Dave
>
>>
>> -Kees
>
>
>
>> >
>> > Dave
>> >
>> >
>> > ----- 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 for a slightly incorrect syntax.
>> >>
>> >> 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 at google.com>
>> >> ---
>> >> defs.h | 2 ++
>> >> main.c | 8 ++++++--
>> >> symbols.c | 69
>> >> +++++++++++++++++++++++++++++++++++++++++++++------------------
>> >> x86_64.c | 20 ++++++++++++------
>> >> 4 files changed, 72 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..5a22d1a 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,20 @@ relocate(ulong symval, char *symname, int
>> >> first_symbol)
>> >> break;
>> >> }
>> >>
>> >> - return (symval - kt->relocate);
>> >> + if (machine_type("X86_64")) {
>> >> + /*
>> >> + * 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;
>> >> + }
>> >> + } else
>> >> + return symval - kt->relocate;
>> >> }
>> >>
>> >> /*
>> >> @@ -9506,6 +9521,7 @@ section_header_info(bfd *bfd, asection *section,
>> >> void *reqptr)
>> >> struct load_module *lm;
>> >> ulong request;
>> >> asection **sec;
>> >> + ulong section_end_address;
>> >>
>> >> request = ((ulong)reqptr);
>> >>
>> >> @@ -9524,6 +9540,12 @@ section_header_info(bfd *bfd, asection *section,
>> >> void *reqptr)
>> >> kt->etext_init = kt->stext_init +
>> >> (ulong)bfd_section_size(bfd, section);
>> >> }
>> >> +
>> >> + if (STREQ(bfd_get_section_name(bfd, section), ".text")) {
>> >> + st->first_section_start = (ulong)
>> >> + bfd_get_section_vma(bfd, section);
>> >> + }
>> >> +
>> >> if (STREQ(bfd_get_section_name(bfd, section), ".text") ||
>> >> STREQ(bfd_get_section_name(bfd, section), ".data")) {
>> >> if (!(bfd_get_section_flags(bfd, section) &
>> >> SEC_LOAD))
>> >> @@ -9540,6 +9562,15 @@ section_header_info(bfd *bfd, asection *section,
>> >> void *reqptr)
>> >> st->dwarf_debug_frame_file_offset =
>> >> (off_t)section->filepos;
>> >> st->dwarf_debug_frame_size =
>> >> (ulong)bfd_section_size(bfd, section);
>> >> }
>> >> +
>> >> + if (st->first_section_start != 0) {
>> >> + section_end_address =
>> >> + (ulong) bfd_get_section_vma(bfd, section) +
>> >> + (ulong) bfd_section_size(bfd, section);
>> >> + if (section_end_address > st->last_section_end)
>> >> + st->last_section_end = section_end_address;
>> >> + }
>> >> +
>> >> break;
>> >>
>> >> case (ulong)MODULE_SECTIONS:
>> >> diff --git a/x86_64.c b/x86_64.c
>> >> index 1d915b1..0c22ee1 100755
>> >> --- a/x86_64.c
>> >> +++ b/x86_64.c
>> >> @@ -5382,16 +5382,22 @@ search_for_switch_to(ulong start, ulong end)
>> >> {
>> >> ulong max_instructions, address;
>> >> char buf1[BUFSIZE];
>> >> - char buf2[BUFSIZE];
>> >> + char search_string1[BUFSIZE];
>> >> + char search_string2[BUFSIZE];
>> >> int found;
>> >>
>> >> max_instructions = end - start;
>> >> found = FALSE;
>> >> sprintf(buf1, "x/%ldi 0x%lx", max_instructions, start);
>> >> - if (symbol_exists("__switch_to"))
>> >> - sprintf(buf2, "callq 0x%lx", symbol_value("__switch_to"));
>> >> - else
>> >> - buf2[0] = NULLCHAR;
>> >> + if (symbol_exists("__switch_to")) {
>> >> + sprintf(search_string1,
>> >> + "call 0x%lx", symbol_value("__switch_to"));
>> >> + sprintf(search_string2,
>> >> + "callq 0x%lx", symbol_value("__switch_to"));
>> >> + } else {
>> >> + search_string1[0] = NULLCHAR;
>> >> + search_string2[0] = NULLCHAR;
>> >> + }
>> >>
>> >> open_tmpfile();
>> >>
>> >> @@ -5404,7 +5410,9 @@ search_for_switch_to(ulong start, ulong end)
>> >> break;
>> >> if (strstr(buf1, "<__switch_to>"))
>> >> found = TRUE;
>> >> - if (strlen(buf2) && strstr(buf1, buf2))
>> >> + if (strlen(search_string1) && strstr(buf1, search_string1))
>> >> + found = TRUE;
>> >> + if (strlen(search_string2) && strstr(buf1, search_string2))
>> >> found = TRUE;
>> >> }
>> >> close_tmpfile();
>> >> --
>> >
>>
>>
>>
>> --
>> Kees Cook
>> Chrome OS Security
>>
--
Kees Cook
Chrome OS Security
More information about the Crash-utility
mailing list