Re: [PATCH v2 04/12] module: Add printk format to add module build ID to stacktraces

[Stephen Boyd <swboyd@chromium.org>]

Quoting Petr Mladek (2021-03-30 03:29:24)
> On Tue 2021-03-23 19:04:35, Stephen Boyd wrote:
> > Let's make kernel stacktraces easier to identify by including the build
> > ID[1] of a module if the stacktrace is printing a symbol from a module.
> > 
> > Example:
> > 
> >  WARNING: CPU: 3 PID: 3373 at drivers/misc/lkdtm/bugs.c:83 lkdtm_WARNING+0x28/0x30 [lkdtm]
> >  Modules linked in: lkdtm rfcomm algif_hash algif_skcipher af_alg xt_cgroup uinput xt_MASQUERADE hci_uart <modules trimmed>
> >  CPU: 3 PID: 3373 Comm: bash Not tainted 5.11 #12 a8c0d47f7051f3e6670ceaea724af66a39c6cec8
> 
> I tested it with "echo l >/proc/sysrq-trigger" and I got the following
> output:
> 
> [   71.905593] CPU: 0 PID: 1762 Comm: bash Kdump: loaded Not tainted 5.12.0-rc5-default+ #167 00000080ffffffff0000000000000000
> 00000000
> 
> It does not looks like an unique build-id.
> Any idea what was going wrong?

Hmm not sure. What does 'hexdump /sys/kernel/notes' show or 'file
vmlinux' parse out of the kernel image?

> 
> > --- a/include/linux/kallsyms.h
> > +++ b/include/linux/kallsyms.h
> > @@ -91,6 +93,7 @@ const char *kallsyms_lookup(unsigned long addr,
> >  
> >  /* Look up a kernel symbol and return it in a text buffer. */
> >  extern int sprint_symbol(char *buffer, unsigned long address);
> > +extern int sprint_symbol_stacktrace(char *buffer, unsigned long address);
> >  extern int sprint_symbol_no_offset(char *buffer, unsigned long address);
> >  extern int sprint_backtrace(char *buffer, unsigned long address);
> 
> Could we use a more clear name? It is hard to guess what is
> the difference between sprint_*_stacktrace() and sprint_backtrace().
> 
> What about sprint_symbol_build_id() ?

Sure.

> 
> 
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 30479355ab85..9e9cb502fb33 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -2770,6 +2771,20 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
> >       }
> >       mod->core_kallsyms.num_symtab = ndst;
> >  }
> > +
> > +static void init_build_id(struct module *mod, const struct load_info *info)
> > +{
> > +     const Elf_Shdr *sechdr;
> > +     unsigned int i;
> > +
> > +     for (i = 0; i < info->hdr->e_shnum; i++) {
> > +             sechdr = &info->sechdrs[i];
> > +             if (!sect_empty(sechdr) && sechdr->sh_type == SHT_NOTE &&
> > +                 !build_id_parse_buf((void *)sechdr->sh_addr, mod->build_id,
> > +                                     sechdr->sh_size))
> > +                     break;
> > +     }
> 
> Just to be sure. Is this really reliable way how to find the build ID,
> please?
> 

The build ID is always part of a note section, so if we parse the notes
and find a build ID type of section (see nhdr->n_type == BUILD_ID) with
a GNU name then we know we have the right section and will be able to
parse the ID out of the buffer.