* [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid
@ 2025-11-28 13:59 Petr Mladek
2025-11-28 13:59 ` [PATCH v3 1/7] kallsyms: Clean up @namebuf initialization in kallsyms_lookup_buildid() Petr Mladek
` (8 more replies)
0 siblings, 9 replies; 12+ messages in thread
From: Petr Mladek @ 2025-11-28 13:59 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
This patchset is cleaning up kallsyms code related to module buildid.
It is fixing an invalid access when printing backtraces, see [v1] for
more details:
+ 1st..4th patches are preparatory.
+ 5th and 6th patches are fixing bpf and ftrace related APIs.
+ 7th patch prevents a potential race.
Changes against [v2]:
+ Fixed typos in commit message [Alexei]
+ Added Acks [Alexei]
Changes against [v1]:
+ Added existing Reviewed-by tags.
+ Shuffled patches to update the kallsyms_lookup_buildid() initialization
code 1st.
+ Initialized also *modname and *modbuildid in kallsyms_lookup_buildid().
+ Renamed __bpf_address_lookup() to bpf_address_lookup() and used it
in kallsyms_lookup_buildid(). Did this instead of passing @modbuildid
parameter just to clear it.
[v1] https://lore.kernel.org/r/20251105142319.1139183-1-pmladek@suse.com
[v2] https://lore.kernel.org/r/20251112142003.182062-1-pmladek@suse.com
Petr Mladek (7):
kallsyms: Clean up @namebuf initialization in
kallsyms_lookup_buildid()
kallsyms: Clean up modname and modbuildid initialization in
kallsyms_lookup_buildid()
module: Add helper function for reading module_buildid()
kallsyms: Cleanup code for appending the module buildid
kallsyms/bpf: Rename __bpf_address_lookup() to bpf_address_lookup()
kallsyms/ftrace: Set module buildid in ftrace_mod_address_lookup()
kallsyms: Prevent module removal when printing module name and buildid
arch/arm64/net/bpf_jit_comp.c | 2 +-
arch/powerpc/net/bpf_jit_comp.c | 2 +-
include/linux/filter.h | 26 ++----------
include/linux/ftrace.h | 6 ++-
include/linux/module.h | 9 ++++
kernel/bpf/core.c | 4 +-
kernel/kallsyms.c | 73 ++++++++++++++++++++++++---------
kernel/module/kallsyms.c | 9 +---
kernel/trace/ftrace.c | 5 ++-
9 files changed, 81 insertions(+), 55 deletions(-)
--
2.52.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/7] kallsyms: Clean up @namebuf initialization in kallsyms_lookup_buildid()
2025-11-28 13:59 [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
@ 2025-11-28 13:59 ` Petr Mladek
2025-11-28 13:59 ` [PATCH v3 2/7] kallsyms: Clean up modname and modbuildid " Petr Mladek
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Petr Mladek @ 2025-11-28 13:59 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
The function kallsyms_lookup_buildid() initializes the given @namebuf
by clearing the first and the last byte. It is not clear why.
The 1st byte makes sense because some callers ignore the return code
and expect that the buffer contains a valid string, for example:
- function_stat_show()
- kallsyms_lookup()
- kallsyms_lookup_buildid()
The initialization of the last byte does not make much sense because it
can later be overwritten. Fortunately, it seems that all called
functions behave correctly:
- kallsyms_expand_symbol() explicitly adds the trailing '\0'
at the end of the function.
- All *__address_lookup() functions either use the safe strscpy()
or they do not touch the buffer at all.
Document the reason for clearing the first byte. And remove the useless
initialization of the last byte.
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kallsyms.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 1e7635864124..e08c1e57fc0d 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -352,7 +352,12 @@ static int kallsyms_lookup_buildid(unsigned long addr,
{
int ret;
- namebuf[KSYM_NAME_LEN - 1] = 0;
+ /*
+ * kallsyms_lookus() returns pointer to namebuf on success and
+ * NULL on error. But some callers ignore the return value.
+ * Instead they expect @namebuf filled either with valid
+ * or empty string.
+ */
namebuf[0] = 0;
if (is_ksym_addr(addr)) {
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 2/7] kallsyms: Clean up modname and modbuildid initialization in kallsyms_lookup_buildid()
2025-11-28 13:59 [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
2025-11-28 13:59 ` [PATCH v3 1/7] kallsyms: Clean up @namebuf initialization in kallsyms_lookup_buildid() Petr Mladek
@ 2025-11-28 13:59 ` Petr Mladek
2025-11-28 13:59 ` [PATCH v3 3/7] module: Add helper function for reading module_buildid() Petr Mladek
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Petr Mladek @ 2025-11-28 13:59 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
The @modname and @modbuildid optional return parameters are set only
when the symbol is in a module.
Always initialize them so that they do not need to be cleared when
the module is not in a module. It simplifies the logic and makes
the code even slightly more safe.
Note that bpf_address_lookup() function will get updated in a separate
patch.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kallsyms.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index e08c1e57fc0d..ffb64eaa0505 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -359,6 +359,14 @@ static int kallsyms_lookup_buildid(unsigned long addr,
* or empty string.
*/
namebuf[0] = 0;
+ /*
+ * Initialize the module-related return values. They are not set
+ * when the symbol is in vmlinux or it is a bpf address.
+ */
+ if (modname)
+ *modname = NULL;
+ if (modbuildid)
+ *modbuildid = NULL;
if (is_ksym_addr(addr)) {
unsigned long pos;
@@ -367,10 +375,6 @@ static int kallsyms_lookup_buildid(unsigned long addr,
/* Grab name */
kallsyms_expand_symbol(get_symbol_offset(pos),
namebuf, KSYM_NAME_LEN);
- if (modname)
- *modname = NULL;
- if (modbuildid)
- *modbuildid = NULL;
return strlen(namebuf);
}
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 3/7] module: Add helper function for reading module_buildid()
2025-11-28 13:59 [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
2025-11-28 13:59 ` [PATCH v3 1/7] kallsyms: Clean up @namebuf initialization in kallsyms_lookup_buildid() Petr Mladek
2025-11-28 13:59 ` [PATCH v3 2/7] kallsyms: Clean up modname and modbuildid " Petr Mladek
@ 2025-11-28 13:59 ` Petr Mladek
2025-11-28 13:59 ` [PATCH v3 4/7] kallsyms: Cleanup code for appending the module buildid Petr Mladek
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Petr Mladek @ 2025-11-28 13:59 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek,
Daniel Gomez
Add a helper function for reading the optional "build_id" member
of struct module. It is going to be used also in
ftrace_mod_address_lookup().
Use "#ifdef" instead of "#if IS_ENABLED()" to match the declaration
of the optional field in struct module.
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/module.h | 9 +++++++++
kernel/module/kallsyms.c | 9 ++-------
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index e135cc79acee..4decae2b1675 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -747,6 +747,15 @@ static inline void __module_get(struct module *module)
__mod ? __mod->name : "kernel"; \
})
+static inline const unsigned char *module_buildid(struct module *mod)
+{
+#ifdef CONFIG_STACKTRACE_BUILD_ID
+ return mod->build_id;
+#else
+ return NULL;
+#endif
+}
+
/* Dereference module function descriptor */
void *dereference_module_function_descriptor(struct module *mod, void *ptr);
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 00a60796327c..0fc11e45df9b 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -334,13 +334,8 @@ int module_address_lookup(unsigned long addr,
if (mod) {
if (modname)
*modname = mod->name;
- if (modbuildid) {
-#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
- *modbuildid = mod->build_id;
-#else
- *modbuildid = NULL;
-#endif
- }
+ if (modbuildid)
+ *modbuildid = module_buildid(mod);
sym = find_kallsyms_symbol(mod, addr, size, offset);
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 4/7] kallsyms: Cleanup code for appending the module buildid
2025-11-28 13:59 [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
` (2 preceding siblings ...)
2025-11-28 13:59 ` [PATCH v3 3/7] module: Add helper function for reading module_buildid() Petr Mladek
@ 2025-11-28 13:59 ` Petr Mladek
2025-11-28 13:59 ` [PATCH v3 5/7] kallsyms/bpf: Rename __bpf_address_lookup() to bpf_address_lookup() Petr Mladek
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Petr Mladek @ 2025-11-28 13:59 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
Put the code for appending the optional "buildid" into a helper
function, It makes __sprint_symbol() better readable.
Also print a warning when the "modname" is set and the "buildid" isn't.
It might catch a situation when some lookup function in
kallsyms_lookup_buildid() does not handle the "buildid".
Use pr_*_once() to avoid an infinite recursion when the function
is called from printk(). The recursion is rather theoretical but
better be on the safe side.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kallsyms.c | 42 +++++++++++++++++++++++++++++++++---------
1 file changed, 33 insertions(+), 9 deletions(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index ffb64eaa0505..f25b122397ce 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -432,6 +432,37 @@ int lookup_symbol_name(unsigned long addr, char *symname)
return lookup_module_symbol_name(addr, symname);
}
+#ifdef CONFIG_STACKTRACE_BUILD_ID
+
+static int append_buildid(char *buffer, const char *modname,
+ const unsigned char *buildid)
+{
+ if (!modname)
+ return 0;
+
+ if (!buildid) {
+ pr_warn_once("Undefined buildid for the module %s\n", modname);
+ return 0;
+ }
+
+ /* build ID should match length of sprintf */
+#ifdef CONFIG_MODULES
+ static_assert(sizeof(typeof_member(struct module, build_id)) == 20);
+#endif
+
+ return sprintf(buffer, " %20phN", buildid);
+}
+
+#else /* CONFIG_STACKTRACE_BUILD_ID */
+
+static int append_buildid(char *buffer, const char *modname,
+ const unsigned char *buildid)
+{
+ return 0;
+}
+
+#endif /* CONFIG_STACKTRACE_BUILD_ID */
+
/* Look up a kernel symbol and return it in a text buffer. */
static int __sprint_symbol(char *buffer, unsigned long address,
int symbol_offset, int add_offset, int add_buildid)
@@ -454,15 +485,8 @@ static int __sprint_symbol(char *buffer, unsigned long address,
if (modname) {
len += sprintf(buffer + len, " [%s", modname);
-#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
- if (add_buildid && buildid) {
- /* build ID should match length of sprintf */
-#if IS_ENABLED(CONFIG_MODULES)
- static_assert(sizeof(typeof_member(struct module, build_id)) == 20);
-#endif
- len += sprintf(buffer + len, " %20phN", buildid);
- }
-#endif
+ if (add_buildid)
+ len += append_buildid(buffer + len, modname, buildid);
len += sprintf(buffer + len, "]");
}
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 5/7] kallsyms/bpf: Rename __bpf_address_lookup() to bpf_address_lookup()
2025-11-28 13:59 [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
` (3 preceding siblings ...)
2025-11-28 13:59 ` [PATCH v3 4/7] kallsyms: Cleanup code for appending the module buildid Petr Mladek
@ 2025-11-28 13:59 ` Petr Mladek
2025-11-28 13:59 ` [PATCH v3 6/7] kallsyms/ftrace: Set module buildid in ftrace_mod_address_lookup() Petr Mladek
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Petr Mladek @ 2025-11-28 13:59 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
bpf_address_lookup() has been used only in kallsyms_lookup_buildid().
It was supposed to set @modname and @modbuildid when the symbol was
in a module.
But it always just cleared @modname because BPF symbols were never in
a module. And it did not clear @modbuildid because the pointer was
not passed.
The wrapper is no longer needed. Both @modname and @modbuildid
are now always initialized to NULL in kallsyms_lookup_buildid().
Remove the wrapper and rename __bpf_address_lookup() to
bpf_address_lookup() because this variant is used everywhere.
Fixes: 9294523e3768 ("module: add printk formats to add module build ID to stacktraces")
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
arch/arm64/net/bpf_jit_comp.c | 2 +-
arch/powerpc/net/bpf_jit_comp.c | 2 +-
include/linux/filter.h | 26 ++++----------------------
kernel/bpf/core.c | 4 ++--
kernel/kallsyms.c | 5 ++---
5 files changed, 10 insertions(+), 29 deletions(-)
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 0c9a50a1e73e..17e6a041ea4d 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -2939,7 +2939,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
u64 plt_target = 0ULL;
bool poking_bpf_entry;
- if (!__bpf_address_lookup((unsigned long)ip, &size, &offset, namebuf))
+ if (!bpf_address_lookup((unsigned long)ip, &size, &offset, namebuf))
/* Only poking bpf text is supported. Since kernel function
* entry is set up by ftrace, we reply on ftrace to poke kernel
* functions.
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 88ad5ba7b87f..21f7f26a5e2f 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -1122,7 +1122,7 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
branch_flags = poke_type == BPF_MOD_CALL ? BRANCH_SET_LINK : 0;
/* We currently only support poking bpf programs */
- if (!__bpf_address_lookup(bpf_func, &size, &offset, name)) {
+ if (!bpf_address_lookup(bpf_func, &size, &offset, name)) {
pr_err("%s (0x%lx): kernel/modules are not supported\n", __func__, bpf_func);
return -EOPNOTSUPP;
}
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 973233b82dc1..0189f7488044 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1373,24 +1373,13 @@ static inline bool bpf_jit_kallsyms_enabled(void)
return false;
}
-int __bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char *sym);
+int bpf_address_lookup(unsigned long addr, unsigned long *size,
+ unsigned long *off, char *sym);
bool is_bpf_text_address(unsigned long addr);
int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
char *sym);
struct bpf_prog *bpf_prog_ksym_find(unsigned long addr);
-static inline int
-bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
-{
- int ret = __bpf_address_lookup(addr, size, off, sym);
-
- if (ret && modname)
- *modname = NULL;
- return ret;
-}
-
void bpf_prog_kallsyms_add(struct bpf_prog *fp);
void bpf_prog_kallsyms_del(struct bpf_prog *fp);
@@ -1429,8 +1418,8 @@ static inline bool bpf_jit_kallsyms_enabled(void)
}
static inline int
-__bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char *sym)
+bpf_address_lookup(unsigned long addr, unsigned long *size,
+ unsigned long *off, char *sym)
{
return 0;
}
@@ -1451,13 +1440,6 @@ static inline struct bpf_prog *bpf_prog_ksym_find(unsigned long addr)
return NULL;
}
-static inline int
-bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
-{
- return 0;
-}
-
static inline void bpf_prog_kallsyms_add(struct bpf_prog *fp)
{
}
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index d595fe512498..c2278f392e93 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -713,8 +713,8 @@ static struct bpf_ksym *bpf_ksym_find(unsigned long addr)
return n ? container_of(n, struct bpf_ksym, tnode) : NULL;
}
-int __bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char *sym)
+int bpf_address_lookup(unsigned long addr, unsigned long *size,
+ unsigned long *off, char *sym)
{
struct bpf_ksym *ksym;
int ret = 0;
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index f25b122397ce..97b92fc8871d 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -342,7 +342,7 @@ int kallsyms_lookup_size_offset(unsigned long addr, unsigned long *symbolsize,
return 1;
}
return !!module_address_lookup(addr, symbolsize, offset, NULL, NULL, namebuf) ||
- !!__bpf_address_lookup(addr, symbolsize, offset, namebuf);
+ !!bpf_address_lookup(addr, symbolsize, offset, namebuf);
}
static int kallsyms_lookup_buildid(unsigned long addr,
@@ -383,8 +383,7 @@ static int kallsyms_lookup_buildid(unsigned long addr,
ret = module_address_lookup(addr, symbolsize, offset,
modname, modbuildid, namebuf);
if (!ret)
- ret = bpf_address_lookup(addr, symbolsize,
- offset, modname, namebuf);
+ ret = bpf_address_lookup(addr, symbolsize, offset, namebuf);
if (!ret)
ret = ftrace_mod_address_lookup(addr, symbolsize,
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 6/7] kallsyms/ftrace: Set module buildid in ftrace_mod_address_lookup()
2025-11-28 13:59 [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
` (4 preceding siblings ...)
2025-11-28 13:59 ` [PATCH v3 5/7] kallsyms/bpf: Rename __bpf_address_lookup() to bpf_address_lookup() Petr Mladek
@ 2025-11-28 13:59 ` Petr Mladek
2025-11-28 13:59 ` [PATCH v3 7/7] kallsyms: Prevent module removal when printing module name and buildid Petr Mladek
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Petr Mladek @ 2025-11-28 13:59 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
__sprint_symbol() might access an invalid pointer when
kallsyms_lookup_buildid() returns a symbol found by
ftrace_mod_address_lookup().
The ftrace lookup function must set both @modname and @modbuildid
the same way as module_address_lookup().
Fixes: 9294523e3768 ("module: add printk formats to add module build ID to stacktraces")
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/ftrace.h | 6 ++++--
kernel/kallsyms.c | 4 ++--
kernel/trace/ftrace.c | 5 ++++-
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 07f8c309e432..9cc60e2506af 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -87,11 +87,13 @@ struct ftrace_hash;
defined(CONFIG_DYNAMIC_FTRACE)
int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym);
+ unsigned long *off, char **modname,
+ const unsigned char **modbuildid, char *sym);
#else
static inline int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname,
+ const unsigned char **modbuildid, char *sym)
{
return 0;
}
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 97b92fc8871d..5bc1646f8639 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -386,8 +386,8 @@ static int kallsyms_lookup_buildid(unsigned long addr,
ret = bpf_address_lookup(addr, symbolsize, offset, namebuf);
if (!ret)
- ret = ftrace_mod_address_lookup(addr, symbolsize,
- offset, modname, namebuf);
+ ret = ftrace_mod_address_lookup(addr, symbolsize, offset,
+ modname, modbuildid, namebuf);
return ret;
}
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 59cfacb8a5bb..d0001dffd98a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7708,7 +7708,8 @@ ftrace_func_address_lookup(struct ftrace_mod_map *mod_map,
int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname,
+ const unsigned char **modbuildid, char *sym)
{
struct ftrace_mod_map *mod_map;
int ret = 0;
@@ -7720,6 +7721,8 @@ ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
if (ret) {
if (modname)
*modname = mod_map->mod->name;
+ if (modbuildid)
+ *modbuildid = module_buildid(mod_map->mod);
break;
}
}
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 7/7] kallsyms: Prevent module removal when printing module name and buildid
2025-11-28 13:59 [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
` (5 preceding siblings ...)
2025-11-28 13:59 ` [PATCH v3 6/7] kallsyms/ftrace: Set module buildid in ftrace_mod_address_lookup() Petr Mladek
@ 2025-11-28 13:59 ` Petr Mladek
2025-12-16 14:00 ` [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
2025-12-17 21:09 ` Andrew Morton
8 siblings, 0 replies; 12+ messages in thread
From: Petr Mladek @ 2025-11-28 13:59 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel, Petr Mladek
kallsyms_lookup_buildid() copies the symbol name into the given buffer
so that it can be safely read anytime later. But it just copies pointers
to mod->name and mod->build_id which might get reused after the related
struct module gets removed.
The lifetime of struct module is synchronized using RCU. Take the rcu
read lock for the entire __sprint_symbol().
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kallsyms.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 5bc1646f8639..202d39f5493a 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -471,6 +471,9 @@ static int __sprint_symbol(char *buffer, unsigned long address,
unsigned long offset, size;
int len;
+ /* Prevent module removal until modname and modbuildid are printed */
+ guard(rcu)();
+
address += symbol_offset;
len = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid,
buffer);
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid
2025-11-28 13:59 [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
` (6 preceding siblings ...)
2025-11-28 13:59 ` [PATCH v3 7/7] kallsyms: Prevent module removal when printing module name and buildid Petr Mladek
@ 2025-12-16 14:00 ` Petr Mladek
2025-12-17 21:10 ` Andrew Morton
2025-12-17 21:09 ` Andrew Morton
8 siblings, 1 reply; 12+ messages in thread
From: Petr Mladek @ 2025-12-16 14:00 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel
Hi,
I wonder who could take this patchset.
IMHO, the failed test report is bogus. The system went out of memory.
Anyway, the info provided by the mail is not enough for debugging.
IMHO. this patchset is ready for linux-next. Unfortunately, kallsyms
do not have any dedicated maintainer. I though about Kees (hardening)
or Andrew (core stuff). Or I could take it via printk tree.
Best Regards,
Petr
On Fri 2025-11-28 14:59:13, Petr Mladek wrote:
> This patchset is cleaning up kallsyms code related to module buildid.
> It is fixing an invalid access when printing backtraces, see [v1] for
> more details:
>
> + 1st..4th patches are preparatory.
>
> + 5th and 6th patches are fixing bpf and ftrace related APIs.
>
> + 7th patch prevents a potential race.
>
>
> Changes against [v2]:
>
> + Fixed typos in commit message [Alexei]
>
> + Added Acks [Alexei]
>
>
> Changes against [v1]:
>
> + Added existing Reviewed-by tags.
>
> + Shuffled patches to update the kallsyms_lookup_buildid() initialization
> code 1st.
>
> + Initialized also *modname and *modbuildid in kallsyms_lookup_buildid().
>
> + Renamed __bpf_address_lookup() to bpf_address_lookup() and used it
> in kallsyms_lookup_buildid(). Did this instead of passing @modbuildid
> parameter just to clear it.
>
>
> [v1] https://lore.kernel.org/r/20251105142319.1139183-1-pmladek@suse.com
> [v2] https://lore.kernel.org/r/20251112142003.182062-1-pmladek@suse.com
>
>
> Petr Mladek (7):
> kallsyms: Clean up @namebuf initialization in
> kallsyms_lookup_buildid()
> kallsyms: Clean up modname and modbuildid initialization in
> kallsyms_lookup_buildid()
> module: Add helper function for reading module_buildid()
> kallsyms: Cleanup code for appending the module buildid
> kallsyms/bpf: Rename __bpf_address_lookup() to bpf_address_lookup()
> kallsyms/ftrace: Set module buildid in ftrace_mod_address_lookup()
> kallsyms: Prevent module removal when printing module name and buildid
>
> arch/arm64/net/bpf_jit_comp.c | 2 +-
> arch/powerpc/net/bpf_jit_comp.c | 2 +-
> include/linux/filter.h | 26 ++----------
> include/linux/ftrace.h | 6 ++-
> include/linux/module.h | 9 ++++
> kernel/bpf/core.c | 4 +-
> kernel/kallsyms.c | 73 ++++++++++++++++++++++++---------
> kernel/module/kallsyms.c | 9 +---
> kernel/trace/ftrace.c | 5 ++-
> 9 files changed, 81 insertions(+), 55 deletions(-)
>
> --
> 2.52.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid
2025-11-28 13:59 [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
` (7 preceding siblings ...)
2025-12-16 14:00 ` [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
@ 2025-12-17 21:09 ` Andrew Morton
2025-12-18 9:09 ` Petr Mladek
8 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2025-12-17 21:09 UTC (permalink / raw)
To: Petr Mladek
Cc: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Kees Cook,
Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel
On Fri, 28 Nov 2025 14:59:13 +0100 Petr Mladek <pmladek@suse.com> wrote:
> This patchset is cleaning up kallsyms code related to module buildid.
> It is fixing an invalid access when printing backtraces, see [v1] for
> more details:
>
> ...
>
> [v1] https://lore.kernel.org/r/20251105142319.1139183-1-pmladek@suse.com
> [v2] https://lore.kernel.org/r/20251112142003.182062-1-pmladek@suse.com
>
It's best to avoid sending people off to the WWW to understand a
patchset - better that the git history be self-contained. So when
staging this for mm.git I scooped the relevant material from [1] and
added it to your cover letter, as below. Looks OK?
From: Petr Mladek <pmladek@suse.com>
Subject: kallsyms: clean up @namebuf initialization in kallsyms_lookup_buildid()
Date: Fri, 28 Nov 2025 14:59:14 +0100
Patch series "kallsyms: Prevent invalid access when showing module
buildid", v3.
We have seen nested crashes in __sprint_symbol(), see below. They seem to
be caused by an invalid pointer to "buildid". This patchset cleans up
kallsyms code related to module buildid and fixes this invalid access when
printing backtraces.
I made an audit of __sprint_symbol() and found several situations
when the buildid might be wrong:
+ bpf_address_lookup() does not set @modbuildid
+ ftrace_mod_address_lookup() does not set @modbuildid
+ __sprint_symbol() does not take rcu_read_lock and
the related struct module might get removed before
mod->build_id is printed.
This patchset solves these problems:
+ 1st, 2nd patches are preparatory
+ 3rd, 4th, 6th patches fix the above problems
+ 5th patch cleans up a suspicious initialization code.
This is the backtrace, we have seen. But it is not really important.
The problems fixed by the patchset are obvious:
crash64> bt [62/2029]
PID: 136151 TASK: ffff9f6c981d4000 CPU: 367 COMMAND: "btrfs"
#0 [ffffbdb687635c28] machine_kexec at ffffffffb4c845b3
#1 [ffffbdb687635c80] __crash_kexec at ffffffffb4d86a6a
#2 [ffffbdb687635d08] hex_string at ffffffffb51b3b61
#3 [ffffbdb687635d40] crash_kexec at ffffffffb4d87964
#4 [ffffbdb687635d50] oops_end at ffffffffb4c41fc8
#5 [ffffbdb687635d70] do_trap at ffffffffb4c3e49a
#6 [ffffbdb687635db8] do_error_trap at ffffffffb4c3e6a4
#7 [ffffbdb687635df8] exc_stack_segment at ffffffffb5666b33
#8 [ffffbdb687635e20] asm_exc_stack_segment at ffffffffb5800cf9
...
This patch (of 7)
The function kallsyms_lookup_buildid() initializes the given @namebuf by
clearing the first and the last byte. It is not clear why.
The 1st byte makes sense because some callers ignore the return code and
expect that the buffer contains a valid string, for example:
- function_stat_show()
- kallsyms_lookup()
- kallsyms_lookup_buildid()
The initialization of the last byte does not make much sense because it
can later be overwritten. Fortunately, it seems that all called functions
behave correctly:
- kallsyms_expand_symbol() explicitly adds the trailing '\0'
at the end of the function.
- All *__address_lookup() functions either use the safe strscpy()
or they do not touch the buffer at all.
Document the reason for clearing the first byte. And remove the useless
initialization of the last byte.
Link: https://lkml.kernel.org/r/20251128135920.217303-2-pmladek@suse.com
Signed-off-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkman <daniel@iogearbox.net>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Kees Cook <kees@kernel.org>
Cc: Luis Chamberalin <mcgrof@kernel.org>
Cc: Marc Rutland <mark.rutland@arm.com>
Cc: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Cc: Petr Pavlu <petr.pavlu@suse.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
kernel/kallsyms.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
--- a/kernel/kallsyms.c~kallsyms-clean-up-namebuf-initialization-in-kallsyms_lookup_buildid
+++ a/kernel/kallsyms.c
@@ -355,7 +355,12 @@ static int kallsyms_lookup_buildid(unsig
{
int ret;
- namebuf[KSYM_NAME_LEN - 1] = 0;
+ /*
+ * kallsyms_lookus() returns pointer to namebuf on success and
+ * NULL on error. But some callers ignore the return value.
+ * Instead they expect @namebuf filled either with valid
+ * or empty string.
+ */
namebuf[0] = 0;
if (is_ksym_addr(addr)) {
_
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid
2025-12-16 14:00 ` [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
@ 2025-12-17 21:10 ` Andrew Morton
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Morton @ 2025-12-17 21:10 UTC (permalink / raw)
To: Petr Mladek
Cc: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Kees Cook,
Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel
On Tue, 16 Dec 2025 15:00:22 +0100 Petr Mladek <pmladek@suse.com> wrote:
> I wonder who could take this patchset.
>
> IMHO, the failed test report is bogus. The system went out of memory.
> Anyway, the info provided by the mail is not enough for debugging.
>
> IMHO. this patchset is ready for linux-next. Unfortunately, kallsyms
> do not have any dedicated maintainer. I though about Kees (hardening)
> or Andrew (core stuff). Or I could take it via printk tree.
I seem to be a usual kallsyms patch monkey so I scooped it up, thanks.
If you or Kees prefer to take it then I'll drop the mm.git copy when I
get notified of the duplication by the linux-next maintainer.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid
2025-12-17 21:09 ` Andrew Morton
@ 2025-12-18 9:09 ` Petr Mladek
0 siblings, 0 replies; 12+ messages in thread
From: Petr Mladek @ 2025-12-18 9:09 UTC (permalink / raw)
To: Andrew Morton
Cc: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Kees Cook,
Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel
On Wed 2025-12-17 13:09:04, Andrew Morton wrote:
> On Fri, 28 Nov 2025 14:59:13 +0100 Petr Mladek <pmladek@suse.com> wrote:
>
> > This patchset is cleaning up kallsyms code related to module buildid.
> > It is fixing an invalid access when printing backtraces, see [v1] for
> > more details:
> >
> > ...
> >
> > [v1] https://lore.kernel.org/r/20251105142319.1139183-1-pmladek@suse.com
> > [v2] https://lore.kernel.org/r/20251112142003.182062-1-pmladek@suse.com
> >
>
> It's best to avoid sending people off to the WWW to understand a
> patchset - better that the git history be self-contained.
I see. I'll do better next time.
> So when
> staging this for mm.git I scooped the relevant material from [1] and
> added it to your cover letter, as below. Looks OK?
It looks OK to me. Thanks for taking the patchset.
Best Regards,
Petr
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-12-18 9:09 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-28 13:59 [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
2025-11-28 13:59 ` [PATCH v3 1/7] kallsyms: Clean up @namebuf initialization in kallsyms_lookup_buildid() Petr Mladek
2025-11-28 13:59 ` [PATCH v3 2/7] kallsyms: Clean up modname and modbuildid " Petr Mladek
2025-11-28 13:59 ` [PATCH v3 3/7] module: Add helper function for reading module_buildid() Petr Mladek
2025-11-28 13:59 ` [PATCH v3 4/7] kallsyms: Cleanup code for appending the module buildid Petr Mladek
2025-11-28 13:59 ` [PATCH v3 5/7] kallsyms/bpf: Rename __bpf_address_lookup() to bpf_address_lookup() Petr Mladek
2025-11-28 13:59 ` [PATCH v3 6/7] kallsyms/ftrace: Set module buildid in ftrace_mod_address_lookup() Petr Mladek
2025-11-28 13:59 ` [PATCH v3 7/7] kallsyms: Prevent module removal when printing module name and buildid Petr Mladek
2025-12-16 14:00 ` [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid Petr Mladek
2025-12-17 21:10 ` Andrew Morton
2025-12-17 21:09 ` Andrew Morton
2025-12-18 9:09 ` Petr Mladek
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).