Re: [PATCH 2/2] kallsyms: Optimise symbol name search

[David Laight <david.laight.linux@gmail.com>]

On Wed, 30 Sep 2026 13:35:25 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] kallsyms: 32-bit offset truncation in scripts/kallsyms.c causes panic
> - [Medium] kallsyms: off-by-one in get_name_address binary search bounds doubles scan time
> --
> 
> commit aa83959ffd157437ca56938c0da66225c5fc9aaf
> Author: David Laight <david.laight.linux@gmail.com>
> 
> kallsyms: Optimise symbol name search
> 
> Change the alphabetically ordered lookup table (kallsyms_seqs_of_names)
> with one that indexes the table of compressed names rather than the
> array of symbol values. This removes linear scans during the binary search
> for the symbol name.
> 
> > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> > index d18d78e626db2..2f39fa850deae 100644
> > --- a/kernel/kallsyms.c
> > +++ b/kernel/kallsyms.c  
> [ ... ]
> > @@ -177,6 +177,36 @@ static unsigned int get_symbol_offset(unsigned long pos)
> >  	return name - kallsyms_names;
> >  }
> >  
> > +/*
> > + * Find the value of a symbol givem the offset in the compressed stream.
> > + */
> > +static unsigned long get_name_address(unsigned int name_offset)
> > +{
> > +	unsigned int low, pos, high;
> > +
> > +	low = 0;
> > +	high = kallsyms_num_syms >> 8;
> > +
> > +	while (high - low > 1) {  
> 
> [Severity: Medium]
> Since high acts as an exclusive upper bound here, doesn't this omit the final
> lookup marker from the search space if kallsyms_num_syms is not an exact
> multiple of 256?
> 
> Any kallsyms lookup for symbols in the last partial block (up to 255 symbols)
> might fall back to the second-to-last marker, unnecessarily doubling the
> linear scan fallback duration.

I'll double check.
This matched one of the other scans.
I did check the first and last symbols (by name and number could be found). 

> 
> > +		pos = low + (high - low) / 2;
> > +		if (name_offset >= kallsyms_markers[pos])
> > +			low = pos;
> > +		else
> > +			high = pos;
> > +	}  
> [ ... ]
> > diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> > index 494852ade6d87..74264ead1efe1 100644
> > --- a/scripts/kallsyms.c
> > +++ b/scripts/kallsyms.c  
> [ ... ]
> > @@ -448,13 +439,28 @@ static void write_src(void)
> >  	printf("\n");
> >  
> >  	sort_symbols_by_name();
> > -	output_label("kallsyms_seqs_of_names");
> > -	for (i = 0; i < table_cnt; i++)
> > -		printf("\t.byte 0x%02x, 0x%02x, 0x%02x\t/* %s */\n",
> > -			(unsigned char)(table[i]->seq >> 16),
> > -			(unsigned char)(table[i]->seq >> 8),
> > -			(unsigned char)(table[i]->seq >> 0),
> > -		       table[i]->sym);
> > +	if (table_size < (1u << 24)) {
> > +		output_label("kallsyms_off24_of_names");
> > +		for (i = 0; i < table_cnt; i++) {
> > +			printf("\t.byte 0x%02x, 0x%02x, 0x%02x\t/* %s */\n",
> > +#ifdef CONFIG_CPU_BIG_ENDIAN
> > +				(unsigned char)(table[i]->seq >> 16),
> > +				(unsigned char)(table[i]->seq >> 8),
> > +				(unsigned char)(table[i]->seq >> 0),
> > +#else
> > +				(unsigned char)(table[i]->seq >> 0),
> > +				(unsigned char)(table[i]->seq >> 8),
> > +				(unsigned char)(table[i]->seq >> 16),
> > +#endif
> > +			       table[i]->sym);
> > +		}
> > +	} else {
> > +		output_label("kallsyms_off32_of_names");
> > +		for (i = 0; i < table_cnt; i++) {
> > +			printf("\t.long %#04x\t/* %s */\n",
> > +				table[i]->seq >> 16, table[i]->sym);  
> 
> [Severity: High]
> Does this code inadvertently truncate the 32-bit offset?

That is a C&P error :-(

David

> 
> When the compressed symbol table exceeds 16MB (such as in allyesconfig builds),
> this code shifts table[i]->seq right by 16 bits. This unconditionally discards
> the lower 16 bits of the sequence offset.
> 
> Could this corrupt the lookup table kallsyms_off32_of_names and lead to a
> panic during boot when the kernel attempts to look up wild memory addresses?
> 
> > +		}
> > +	}
> >  	printf("\n");
> >  }  
>