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

[Andrew Honig]

On Fri, Feb 21, 2014 at 11:52 AM, Dave Anderson <anderson at redhat.com> wrote:
>
>
> ----- Original Message -----
>> On Fri, Feb 21, 2014 at 9:06 AM, Dave Anderson <anderson at redhat.com> 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.
>

Sounds good to me, I'll do that.  I'll check with kees on the
KERNELOFFSET patch status.

>>
>> > 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?
>
Yes, the manual setting of --kaslr would work with 32-bit x86, but it
probably shouldn't be used that way.  I like the idea of restricting
the man page.  I don't think it's worth checking to make sure it's not
set on a 32-bit kernel, but I can add that if you prefer.

> Dave
>