Re: [PATCH 2/6] kallsyms: Cleanup code for appending the module buildid

[Petr Mladek <pmladek@suse.com>]

On Wed 2025-11-05 14:59:53, bot+bpf-ci@kernel.org wrote:
> > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> > index 1e7635864..9455e3bb0 100644
> > --- a/kernel/kallsyms.c
> > +++ b/kernel/kallsyms.c
> > @@ -423,6 +423,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;
> > +	}
> 
> When ftrace_mod_address_lookup() succeeds in kallsyms_lookup_buildid(),
> it sets *modname but doesn't initialize *modbuildid. This leaves the
> buildid variable uninitialized when __sprint_symbol() calls
> append_buildid().

Just for record. This is a great analyze. This patchset is fixing
this bug in a later patch. ;-)

> Can the check above read uninitialized memory?> 
> Looking at kallsyms_lookup_buildid():
> - module_address_lookup() properly initializes both modname and
>   modbuildid
> - bpf_address_lookup() sets modname=NULL (so append_buildid isn't
>   called)
> - ftrace_mod_address_lookup() sets modname=mod_map->mod->name but has
>   no modbuildid parameter
> 
> The commit message mentions wanting to catch when lookup functions don't
> handle buildid, but shouldn't kallsyms_lookup_buildid() initialize
> *modbuildid=NULL before calling the lookup functions to avoid undefined
> behavior?

It seems that we are going this way, see
https://lore.kernel.org/all/aQ3vWIqG31BgE4YD@pathway.suse.cz/

Best Regards,
Petr