Re: [Crash-utility] [PATCH v2 3/3] Automatically detect kernel aslr offset.

----- Original Message ----- > On Fri, Feb 21, 2014 at 9:06 AM, Dave Anderson <anderson(a)redhat.com&gt; wrote: > > > > Hi Andy, > > > > This patch fails immediately on non-kaslr kernels if _stext is aligned on > > a page boundary. Here's why: > > > > ----- Original Message ----- > > ... [ cut ] ... > >> +static void > >> +derive_kaslr_offset(bfd *abfd, int dynamic, bfd_byte *start, bfd_byte > >> *end, > >> + unsigned int size, asymbol *store) > >> +{ > >> + symbol_info syminfo; > >> + asymbol *sym; > >> + char *name; > >> + unsigned long relocate; > >> + char buf[BUFSIZE]; > >> + > >> + for (; start < end; start += size) { > >> + sym = bfd_minisymbol_to_symbol(abfd, dynamic, start, store); > >> + if (sym == NULL) > >> + error(FATAL, "bfd_minisymbol_to_symbol() failed\n"); > >> + > >> + bfd_get_symbol_info(abfd, sym, &syminfo); > >> + name = strip_symbol_end(syminfo.name, buf); > >> + if (strcmp("_stext", name) == 0) { > >> + relocate = syminfo.value - kt->vmcoreinfo._stext_SYMBOL; > >> + /* > >> + *To avoid mistaking an mismatched kernel version with > >> + * a kaslr offset, we make sure that the offset is > >> + * aligned by 0x1000, as it always will be for > >> + * kaslr. > >> + */ > >> + if ((relocate & 0xFFF) == 0) { > >> + kt->relocate = relocate; > >> + kt->flags |= RELOC_SET; > >> + } > >> + } > >> + } > >> +} > > > > This function is a waste of time if kt->vmcoreinfo._stext_SYMBOL was never set, say > > when running on a live system, or on a dumpfile that has no vmcoreinfo. And what's > > worse in those cases, if _stext is aligned on a page boundary, then kt->relocate is > > set to the _stext symbol value, and all hell breaks loose. > > > > Yes, I see your point. I can easily fix this issue by making sure > _stext_SYMBOL is set before trying to derive the kaslr offset. If > you're happy with that change I'll send a new patch. I am a bit > worried that crash handles so many different cases/situations that I'm > still missing something. I see 3 options > 1) Fix this patch by checking _stext_SYMBOL is set before trying to > derive kaslr offset. > 2) Make this auto-derive functionality opt-in so it doesn't break > other things (something like --kaslr=auto) > 3) Wait until the kaslr offset is properly output in the vmcore and > avoid this somewhat hacky calculation entirely. > > I'm fine with any of these, but I figured I'd enumerate them and see > which you'd prefer. If you're comfortable with 1 then I'm happy with > it. Right, it would be nice if the KERNELOFFSET vmcoreinfo item had gone into the kernel at the same time as kALSR. I see that it's not in Linus' tree yet -- has it been accepted into any other tree waiting to be pulled? Anyway, for that reason, I like the idea of the --kaslr=auto option, and maybe setting a flag somewhere, say in st->flags. And when the KERNELOFFSET item does eventually show up, the same flag could be set during the initial scan of the dumpfile header, obviating the need for --kaslr. Which reminds me -- the "SYMBOL(_stext)" check that you make in is_netdump() also has to be done in is_diskdump() as well. Currently makedumpfile does not work with kaslr dumpfiles, but they will eventually get it working.

> > > And also, if kt->relocate/RELOC_SET does get set legitimately, the function should > > return immediately rather than cycling through the remaining symbols. > > > Will do. > > > BTW, even though the kernel code seems to indicate that this feature would be > > applicable to 32-bit x86, should I restrict the man page and help data to indicate > > it only applies to x86_64? > > I tried to make it only apply to x86_64, which is why the > drive_kaslr_offset function call is within a machine_type("X86_64") if > clause. > thanks, > Andy But the manual setting of --kalsr=<offset> would still be passed through for 32-bit x86, correct? Maybe just --kaslr=auto could be restricted?

Dave