Re: [PATCH v3 next 04/13] scripts/kallsyms: make use of binary data output optional

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

On Wed, 11 Nov 2026 12:32:07 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential
> issues to consider:
> - [Medium] scripts/kallsyms: wrong variable negated for negative offsets
>   in write_src()

Will fix in v4 - all the symbols offsets in my build are positive.

> - [Medium] scripts/kallsyms: 9-digit buffer in write_src() clobbers sign
>   on large offsets

I can't count! Only affects kernels that are larger that 1G.

> - [Low] scripts/kallsyms: missing endian.h include for htobe32() and
>   htole32()

Will change since v4 is needed,

David

> --
> 
> commit b5cbf4ceba792715bcee0dd6d58ce67f9542a5ad
> Author: David Laight <david.laight.linux@gmail.com>
> 
> scripts/kallsyms: make use of binary data output optional
> 
> This commit makes the binary output file and input ELF file optional in
> scripts/kallsyms, writes the markers table in target endianness when using
> binary output, and formats pc-relative offsets into a buffer for fwrite().
> 
> > diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> > index aa3a8ec83fbb8..74db751600b4f 100644
> > --- a/scripts/kallsyms.c
> > +++ b/scripts/kallsyms.c  
> [ ... ]
> > @@ -406,51 +417,105 @@ static void write_src(FILE *out_bin_file, const char *out_bin_name)  
> [ ... ]
> > +		if (out_bin_file) {
> > +			if ((i & 0xFF) == 0)
> > +				markers[i >> 8] = big_endian ? htobe32(off) : htole32(off);  
> 
> [Severity: Low]
> Does scripts/kallsyms.c need to include <endian.h> for htobe32() and
> htole32()?  Glibc pulls it in transitively through <stdlib.h>, but on
> musl libc hosts neither scripts/kallsyms.c nor its included headers
> declare these functions, breaking the host tool build.
> 
> [ ... ]
> 
> > @@ -458,31 +523,15 @@ static void write_src(FILE *out_bin_file, const char *out_bin_name)  
> [ ... ]
> >  	output_label("kallsyms_offsets");
> >  
> >  	for (i = 0; i < table_cnt; i++) {
> >  		if (pc_relative) {
> > +			/* All 64 bit builds and (at least) relocatable 32 bit ones */
> > +			static char line[] = "\t.long\t_text - . +123456789";
> > +			char *end = line + sizeof(line) - 1;
> >  			long long offset = table[i]->addr - _text;
> > +			int off32;
> >  
> >  			if (offset < INT_MIN || offset > INT_MAX) {
> >  				errx(EXIT_FAILURE, "kallsyms failure: "
> > @@ -490,12 +539,28 @@ static void write_src(FILE *out_bin_file, const char *out_bin_name)
> >  					table[i]->addr);
> >  				exit(EXIT_FAILURE);
> >  			}
> > -			printf("\t.long\t_text - . + (%d)\t/* %s */\n",
> > -			       (int)offset, table[i]->sym);
> > +
> > +			/* Build the line so the the faster fwrite() can be used */
> > +			*end-- = out_bin_file ? '\n' : '\t';
> > +			memset(end - 8, ' ', 8);
> > +			off32 = offset;
> > +			if (off32 < 0) {
> > +				end[-9] = '-';
> > +				offset = -off32;
> > +			} else {
> > +				end[-9] = '+';
> > +			}  
> 
> [Severity: Medium]
> Should this negate off32 (or an unsigned magnitude) instead of assigning
> -off32 back to offset?  Because offset is not used after this point,
> off32 stays negative when a symbol lies below _text, so off32 % 10 in the
> digit loop below yields negative remainders and writes non-digit ASCII
> characters into the assembly output (and -off32 overflows if off32 is
> INT_MIN).
> 
> > +			do
> > +				*end-- = '0' + off32 % 10;
> > +			while (off32 /= 10);  
> 
> [Severity: Medium]
> Can a 10-digit offset in [INT_MIN, INT_MAX] overwrite the '+' or '-' sign
> in line[]?  The template only reserves 9 digit slots after end[-9], so any
> symbol at least 1,000,000,000 bytes from _text runs 10 iterations of the
> loop and clobbers end[-9], emitting invalid assembly syntax.
>