----- Original Message -----
> On Fri, Feb 21, 2014 at 9:06 AM, Dave Anderson <anderson@xxxxxxxxxx> 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
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
