Re: [PATCH v3 next 09/13] kallsyms: Add a kernel parameter to change the kallsyms_markers[] density

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] kallsyms: wrong markers_cnt rounding in write_src() overflows
  or leaks heap
--

commit 03c7dad0d75ad1647bb5f932cd21410fa5140c26
Author: David Laight <david.laight.linux@gmail.com>

kallsyms: Add a kernel parameter to change the kallsyms_markers[] density

This commit adds CONFIG_KALLSYMS_MARKER_SHIFT and a --marker-shift option
to scripts/kallsyms so the interval between kallsyms_markers[] entries is
configurable rather than fixed at 256 symbols.

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

[ ... ]

> @@ -441,8 +443,8 @@ static void write_src(void)
>  	stamp("write token table");
>  
>  	/* table of offset markers, that give the offset in the compressed stream
> -	 * every 256 symbols */
> -	markers_cnt = (table_cnt + 255) / 256;
> +	 * every (1 << marker_shift) (typically 64 or 256) symbols */
> +	markers_cnt = (table_cnt + 255) >> marker_shift;
>  	markers = xmalloc(sizeof(*markers) * markers_cnt);

[Severity: High]
Should markers_cnt round up using (1u << marker_shift) - 1 instead of
255?

The loop below populates markers[i >> marker_shift] up to index
(table_cnt - 1) >> marker_shift.  With 255 still hardcoded,
marker_shift > 8 can under-allocate markers[] by one entry, overflowing
the xmalloc() buffer in write_src() and omitting the final entry from
kallsyms_markers[] so that get_symbol_offset() and get_name_address()
read past the end of kallsyms_markers[] at runtime.  Conversely,
marker_shift < 8 (including the new default of 6) over-allocates
markers_cnt and writes uninitialized heap memory from xmalloc() into the
generated kallsyms_markers[] table.

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