Re: [PATCH] kallsyms: Don't let kallsyms_lookup_size_offset() fail on retrieving the first symbol

[Will Deacon]

On Mon, Aug 26, 2019 at 10:46:38AM +0900, Masami Hiramatsu wrote:
> On Sat, 24 Aug 2019 14:12:31 +0100
> Marc Zyngier <maz@xxxxxxxxxx> wrote:
> 
> > An arm64 kernel configured with
> > 
> >   CONFIG_KPROBES=y
> >   CONFIG_KALLSYMS=y
> >   # CONFIG_KALLSYMS_ALL is not set
> >   CONFIG_KALLSYMS_BASE_RELATIVE=y
> > 
> > reports the following kprobe failure:
> > 
> >   [    0.032677] kprobes: failed to populate blacklist: -22
> >   [    0.033376] Please take care of using kprobes.
> > 
> > It appears that kprobe fails to retrieve the symbol at address
> > 0xffff000010081000, despite this symbol being in System.map:
> > 
> >   ffff000010081000 T __exception_text_start
> > 
> > This symbol is part of the first group of aliases in the
> > kallsyms_offsets array (symbol names generated using ugly hacks in
> > scripts/kallsyms.c):
> > 
> >   kallsyms_offsets:
> >           .long   0x1000 // do_undefinstr
> >           .long   0x1000 // efi_header_end
> >           .long   0x1000 // _stext
> >           .long   0x1000 // __exception_text_start
> >           .long   0x12b0 // do_cp15instr
> > 
> > Looking at the implementation of get_symbol_pos(), it returns the
> > lowest index for aliasing symbols. In this case, it return 0.
> > 
> > But kallsyms_lookup_size_offset() considers 0 as a failure, which
> > is obviously wrong (there is definitely a valid symbol living there).
> > In turn, the kprobe blacklisting stops abruptly, hence the original
> > error.
> > 
> > A CONFIG_KALLSYMS_ALL kernel wouldn't fail as there is always
> > some random symbols at the beginning of this array, which are never
> > looked up via kallsyms_lookup_size_offset.
> > 
> > Fix it by considering that get_symbol_pos() is always successful
> > (which is consistent with the other uses of this function).
> 
> Thank you for fixing this issue!
> This looks good to me :)
> 
> Reviewed-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>

There doesn't appear to be an official tree for this file so, unless anybody
objects, I'll pick this up via arm64 as it's looking like there are some
other fixes on the horizon too.

Will