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().

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 ff7017337535..1fda06b6638c 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -468,6 +468,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.51.1

On Wed, Nov 05, 2025 at 03:23:18PM +0100, Petr Mladek wrote:
> 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().
> 
> 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 ff7017337535..1fda06b6638c 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -468,6 +468,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.51.1

Looks good to me.

Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>

-- 
Aaron Tomlin

On Wed, Nov 05, 2025 at 03:23:18PM +0100, Petr Mladek wrote:
> 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().
> 
> 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 ff7017337535..1fda06b6638c 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -468,6 +468,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.51.1
> 
> 

Hi Petr,

If I am not mistaken, this is handled safely within the context of
module_address_lookup() since f01369239293e ("module: Use RCU in
find_kallsyms_symbol()."), no?


Kind regards,
-- 
Aaron Tomlin

On Fri 2025-11-07 19:36:35, Aaron Tomlin wrote:
> On Wed, Nov 05, 2025 at 03:23:18PM +0100, Petr Mladek wrote:
> > 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().
> > 
> > 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 ff7017337535..1fda06b6638c 100644
> > --- a/kernel/kallsyms.c
> > +++ b/kernel/kallsyms.c
> > @@ -468,6 +468,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.51.1
> > 
> > 
> 
> Hi Petr,
> 
> If I am not mistaken, this is handled safely within the context of
> module_address_lookup() since f01369239293e ("module: Use RCU in
> find_kallsyms_symbol()."), no?

The above mention commit fixed an API which is looking only for
the symbol name. It seems to be used, for example, in kprobe
or ftrace code.

This patch is fixing another API which is used in vsprintf() for
printing backtraces. It looks for more information: symbol name,
module name, and buildid. It needs its own RCU read protection.

Best Regards,
Petr

On Mon, Nov 10, 2025 at 02:26:52PM +0100, Petr Mladek wrote:
> > Hi Petr,
> > 
> > If I am not mistaken, this is handled safely within the context of
> > module_address_lookup() since f01369239293e ("module: Use RCU in
> > find_kallsyms_symbol()."), no?
> 
> The above mention commit fixed an API which is looking only for
> the symbol name. It seems to be used, for example, in kprobe
> or ftrace code.
> 
> This patch is fixing another API which is used in vsprintf() for
> printing backtraces. It looks for more information: symbol name,
> module name, and buildid. It needs its own RCU read protection.

Hi Petr,

I see and agree.

-- 
Aaron Tomlin