* [PATCH] printk: Make %pS print offsets into modules
@ 2016-10-23 21:21 Kevin Cernekee
2016-10-24 14:16 ` Ard Biesheuvel
0 siblings, 1 reply; 2+ messages in thread
From: Kevin Cernekee @ 2016-10-23 21:21 UTC (permalink / raw)
To: rusty
Cc: akpm, ard.biesheuvel, keescook, computersforpeace, dianders,
linux-kernel
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]
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).
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;
+ *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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] printk: Make %pS print offsets into modules
2016-10-23 21:21 [PATCH] printk: Make %pS print offsets into modules Kevin Cernekee
@ 2016-10-24 14:16 ` Ard Biesheuvel
0 siblings, 0 replies; 2+ messages in thread
From: Ard Biesheuvel @ 2016-10-24 14:16 UTC (permalink / raw)
To: Kevin Cernekee
Cc: Rusty Russell, Andrew Morton, Kees Cook, Brian Norris,
Doug Anderson, linux-kernel
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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-10-24 14:17 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-23 21:21 [PATCH] printk: Make %pS print offsets into modules Kevin Cernekee
2016-10-24 14:16 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).