On Tue, Jan 21, 2014 at 12:03 PM, Dave Anderson <anderson(a)redhat.com> wrote:
----- Original Message -----
> On Tue, Jan 21, 2014 at 11:32 AM, Dave Anderson <anderson(a)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(a)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(a)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