Re: [PATCH v3 next 08/13] kallsyms: Optimise symbol name search

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] kallsyms: broken 24-bit big-endian offset calculation in
  write_src()
--

commit 129df190c855fc8323de3f8129f606cadccd6466
Author: David Laight <david.laight.linux@gmail.com>

kallsyms: Optimise symbol name search

This commit changes the alphabetically sorted symbol table from indexing
kallsyms_offsets to indexing the compressed kallsyms_names stream, with the
low bit indicating duplicate symbol names.

> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 74db751600b4f..930ec27a53c06 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c

[ ... ]

> @@ -570,23 +571,41 @@ static void write_src(void)
>  	sort_symbols_by_name();
>  	stamp("sort by name");
>  
> -	output_label("kallsyms_seqs_of_names");
> -	write_incbin_header("seq_of_names", &bin_offset);
> +	off_size = table_size < (1u << 23) ? 24 : 32;
> +	output_label("kallsyms_offs_of_names");
> +	printf("\t.long\t%u\n", off_size);
> +	write_incbin_header("offs_of_names", &bin_offset);
>  	for (i = 0; i < table_cnt; i++) {
>  		se = table[i];
> -		seq = se->seq;
> +		off = se->seq << 1;
> +		if (i && se->len == table[i - 1]->len &&
> +		    !memcmp(se->sym + 1, table[i - 1]->sym + 1, se->len - 1))
> +			/* Mark that this is a duplicate of the previous symbol */
> +			off |= 1;
>  		if (out_bin_file) {
> -			unsigned int seq_be = htobe32(seq);
> -			fwrite((char *)&seq_be + 1, 1, 3, out_bin_file);
> +			unsigned int off_te = big_endian ? htobe32(off) >> (32 - off_size): htole32(off);
> +			fwrite(&off_te, 1, off_size / 8 , out_bin_file);

[Severity: High]
Does this corrupt the 24-bit entries written to out_bin_file when building
a big-endian kernel on a big-endian host?

On a big-endian host, htobe32(off) is a no-op, so shifting the result right
by 8 discards the low byte of off and leaves two leading zero bytes at the
start of off_te.  fwrite(&off_te, 1, 3, out_bin_file) then emits two zero
bytes followed by only the most significant byte of the 24-bit entry,
breaking kallsyms_lookup_name() and kallsyms_on_each_match_symbol() at
runtime.

Shifting off before byte-swapping, as in htobe32(off << (32 - off_size)),
places the three big-endian bytes at the start of off_te on both
little-endian and big-endian hosts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111115911.1142000-1-david.laight.linux@gmail.com?part=8