[PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid

Petr Mladek posted 7 patches 2 months, 1 week ago
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(-)
[PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid
Posted by Petr Mladek 2 months, 1 week ago
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
Re: [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid
Posted by Petr Mladek 1 month, 3 weeks ago
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
Re: [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid
Posted by Andrew Morton 1 month, 3 weeks ago
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.
Re: [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid
Posted by Andrew Morton 1 month, 3 weeks ago
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)) {
_
Re: [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid
Posted by Petr Mladek 1 month, 2 weeks ago
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