Re: [PATCH v3 5/6] perf script: Add --lazy-load-symbols for lazy symbol loading

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] perf: heap buffer overflow in dso__build_ondemand_index via concurrent ELF modification
--

commit d40c3742b741bb2166cbf2426b72bf61c6192a01
Author: Alireza Haghdoost <haghdoost@uber.com>

perf script: Add --lazy-load-symbols for lazy symbol loading

This commit adds the --lazy-load-symbols option to perf script to lazily load
eligible symbols from userspace ELF DSOs. It builds a compact sorted index per
DSO, resolving sampled addresses via binary search and reading symbol names on
demand to reduce memory and time overhead.

> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
[ ... ]
> +static int dso__build_ondemand_index(struct dso *dso, struct symsrc *syms_ss,
> +				     struct symsrc *runtime_ss,
> +				     int dynsym)
> +{
[ ... ]
> +	for (i = 0; i < nr_entries; i++) {
> +		if (!gelf_getsym(syms, i, &sym))
> +			continue;
> +		if (ondemand_sym_ok(elf, secstrs, &sym, shdr.sh_link,
> +				    ehdr.e_machine))
> +			count++;
> +	}
> +
> +	if (!count)
> +		return 0;
> +	if (check_mul_overflow((size_t)count, sizeof(*od->sorted),
> +			       &index_bytes))
> +		return -EOVERFLOW;
[ ... ]
> +	od->sorted = zalloc(index_bytes);
> +	if (!od->sorted) {
> +		symbol__unaccount_bytes(index_bytes);
> +		free(od);
> +		return -1;
> +	}
> +	od->nr_alloc = count;
> +	name_offsets = malloc(count * sizeof(*name_offsets));
> +	if (!name_offsets) {
> +		symbol__unaccount_bytes(index_bytes);
> +		free(od->sorted);
> +		free(od);
> +		return -1;
> +	}
> +
> +	j = 0;
> +	for (i = 0; i < nr_entries; i++) {
> +		u64 adjusted;
> +		GElf_Phdr phdr;
> +
> +		if (!gelf_getsym(syms, i, &sym))
> +			continue;
> +		if (!ondemand_sym_ok(elf, secstrs, &sym, shdr.sh_link,
> +				     ehdr.e_machine))
> +			continue;
[ ... ]
> +		od->sorted[j].start = adjusted;
> +		od->sorted[j].end = sym.st_size;
> +		name_offsets[j] = sym.st_name;
> +		od->sorted[j].name_off = j;
> +		od->sorted[j].binding = GELF_ST_BIND(sym.st_info);
> +		od->sorted[j].type = GELF_ST_TYPE(sym.st_info);
> +		j++;
> +	}

[Severity: High]
Does this code have a bounds checking issue if the ELF file is modified
concurrently?

The algorithm in dso__build_ondemand_index() performs a two-pass scan on the
ELF file mapped into memory by libelf. The first pass computes the count of
valid symbols and allocates the od->sorted and name_offsets arrays based on
that exact count.

If the underlying ELF file (e.g. symbol table or string table) is modified
while this process is running, the memory-mapped data can change between the
two passes. This could cause the second pass to encounter more valid symbols
than originally counted, resulting in j exceeding count (or od->nr_alloc)
without any bounds checking.

Could this result in a heap buffer overflow when writing to od->sorted[j] and
name_offsets[j] if an administrator analyzes a dynamically modified,
user-writable ELF binary?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925-perf-symbol-memory-send-v3-0-3e4e234c363b@uber.com?part=5