Messages in this thread | | | From | Ard Biesheuvel <> | Date | Mon, 24 Oct 2016 15:16:54 +0100 | Subject | Re: [PATCH] printk: Make %pS print offsets into modules |
| |
Hi Kevin,
On 23 October 2016 at 22:21, Kevin Cernekee <cernekee@chromium.org> wrote: > If kallsyms cannot find a symbol for an address, entries like this will > appear in backtraces: > > Call trace: > [<ffffffbffc1ecd7c>] 0xffffffbffc1ecd7c > [<ffffffbffc1ef7f0>] 0xffffffbffc1ef7f0 > [<ffffffbffc1f0094>] 0xffffffbffc1f0094 > > This isn't particularly useful for debugging because modules are not > loaded at fixed addresses. Instead, print the offset from the module's > base, so that the offending location can be easily located in a > disassembly of the .ko file: > > Call trace: > [<ffffffbffc1d57f4>] [mwifiex_pcie+0x37f4/0x9000] > [<ffffffbffc1d60ac>] [mwifiex_pcie+0x40ac/0x9000] > [<ffffffbffc188fe0>] mwifiex_main_process+0xdc/0x6fc [mwifiex] >
This looks like a useful feature, although this example is a bit misleading, since it suggests that only some symbols are unresolveable in some cases, whereas the primary use case for this facility imo is a kernel with no kallsyms to begin with.
> Signed-off-by: Kevin Cernekee <cernekee@chromium.org> > --- > include/linux/module.h | 16 ++++++++++++++++ > kernel/kallsyms.c | 19 ++++++++++++++++++- > kernel/module.c | 23 +++++++++++++++++++++++ > 3 files changed, 57 insertions(+), 1 deletion(-) > > > Tested on 4.4 only (so the core_layout / init_layout stuff is untested). >
Please don't send untested stuff. v4.4 is ancient history for many of us, so this needs to be tested and working against mainline or -next
> diff --git a/include/linux/module.h b/include/linux/module.h > index 0c3207d26ac0..611d1e71b7c8 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -616,6 +616,14 @@ const char *module_address_lookup(unsigned long addr, > unsigned long *offset, > char **modname, > char *namebuf); > + > +/* For kallsyms to ask which module, if any, contains addr. On success, > + * returns true and populates module_size, module_offset, and modname. */ > +bool module_base_lookup(unsigned long addr, > + unsigned long *module_size, > + unsigned long *module_offset, > + char **modname); > + > int lookup_module_symbol_name(unsigned long addr, char *symname); > int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name); > > @@ -698,6 +706,14 @@ static inline const char *module_address_lookup(unsigned long addr, > return NULL; > } > > +static inline bool module_base_lookup(unsigned long addr, > + unsigned long *module_size, > + unsigned long *module_offset, > + char **modname) > +{ > + return false; > +} > + > static inline int lookup_module_symbol_name(unsigned long addr, char *symname) > { > return -ERANGE; > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c > index fafd1a3ef0da..43b114e6861f 100644 > --- a/kernel/kallsyms.c > +++ b/kernel/kallsyms.c > @@ -387,8 +387,25 @@ static int __sprint_symbol(char *buffer, unsigned long address, > > address += symbol_offset; > name = kallsyms_lookup(address, &size, &offset, &modname, buffer); > - if (!name) > + if (!name) { > + /* > + * Fall back to [module_base+offset/size] if the actual > + * symbol name is unavailable. > + */ > + if (module_base_lookup(address, &size, &offset, &modname)) { > + if (add_offset) { > + return snprintf(buffer, KSYM_SYMBOL_LEN, > + "[%s+%#lx/%#lx]", modname, > + address - offset - > + symbol_offset, > + size); > + } else { > + return snprintf(buffer, KSYM_SYMBOL_LEN, > + "[%s]", modname); > + } > + } > return sprintf(buffer, "0x%lx", address - symbol_offset); > + } > > if (name != buffer) > strcpy(buffer, name); > diff --git a/kernel/module.c b/kernel/module.c > index f57dd63186e6..5e09f568c601 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -3899,6 +3899,29 @@ const char *module_address_lookup(unsigned long addr, > return ret; > } > > +bool module_base_lookup(unsigned long addr, > + unsigned long *module_size, > + unsigned long *module_offset, > + char **modname) > +{ > + struct module *mod; > + > + preempt_disable(); > + > + mod = __module_address(addr); > + if (!mod) { > + preempt_enable(); > + return false; > + } > + > + *modname = mod->name;
I'm aware that this pattern may exist elsewhere, but I don't see how it is safe to dereference modname after reenabling preemption. If mod gets freed inadvertently, so will mod->name.
> + *module_offset = (unsigned long)mod->core_layout.base; > + *module_size = mod->init_layout.size + mod->core_layout.size; > + > + preempt_enable(); > + return true; > +} > + > int lookup_module_symbol_name(unsigned long addr, char *symname) > { > struct module *mod; > -- > 2.8.0.rc3.226.g39d4020 >
| |