On Sat, 24 Aug 2019 14:12:31 +0100 Marc Zyngier <m...@kernel.org> 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 <mhira...@kernel.org> Thank you, > > Fixes: ffc5089196446 ("[PATCH] Create kallsyms_lookup_size_offset()") > Cc: Masami Hiramatsu <mhira...@kernel.org> > Cc: Arnaldo Carvalho de Melo <a...@redhat.com> > Cc: Peter Zijlstra <pet...@infradead.org> > Cc: Will Deacon <w...@kernel.org> > Cc: Catalin Marinas <catalin.mari...@arm.com> > Signed-off-by: Marc Zyngier <m...@kernel.org> > --- > kernel/kallsyms.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c > index 95a260f9214b..136ce049c4ad 100644 > --- a/kernel/kallsyms.c > +++ b/kernel/kallsyms.c > @@ -263,8 +263,10 @@ int kallsyms_lookup_size_offset(unsigned long addr, > unsigned long *symbolsize, > { > char namebuf[KSYM_NAME_LEN]; > > - if (is_ksym_addr(addr)) > - return !!get_symbol_pos(addr, symbolsize, offset); > + if (is_ksym_addr(addr)) { > + get_symbol_pos(addr, symbolsize, offset); > + return 1; > + } > return !!module_address_lookup(addr, symbolsize, offset, NULL, namebuf) > || > !!__bpf_address_lookup(addr, symbolsize, offset, namebuf); > } > -- > 2.20.1 > -- Masami Hiramatsu <mhira...@kernel.org>