On Fri, Feb 21, 2014 at 9:06 AM, Dave Anderson <anderson(a)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.
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
Thanks,
Dave