Re: [PATCH 3/4] 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:
- [Medium] perf: memory exhaustion in ondemand_read_name() via truncated ELF
--

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

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

This commit introduces lazy symbol loading for userspace ELF DSOs in perf
script. It builds a sorted in-memory index of symbols and resolves them via
binary search, reading symbol names on-demand from the string table using
pread() to reduce memory footprint.

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 914e42d21f70f..e4d77e46e8830 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c

[ ... ]

> +static const char *ondemand_read_name(int fd, u64 strtab_offset,
> +				      u64 strtab_size, u64 name_off,
> +				      char *buf, size_t buflen,
> +				      char **to_free)
> +{

[ ... ]

> +	/*
> +	 * Slow path: the name is longer than buflen.  Grow a heap buffer
> +	 * geometrically, doubling until the terminator appears, so a long
> +	 * name costs O(name length), not O(remaining strtab size).
> +	 */
> +	cap = 4096;
> +	for (;;) {
> +		char *tmp;
> +		size_t want = cap;
> +
> +		if (want > remain)
> +			want = remain;
> +		if (want == 0)
> +			break;
> +
> +		tmp = *to_free ? realloc(*to_free, want + 1) : malloc(want + 1);
> +		if (!tmp) {
> +			free(*to_free);
> +			*to_free = NULL;
> +			return NULL;
> +		}
> +		*to_free = tmp;
> +
> +		n = pread(fd, *to_free, want, file_off);
> +		if (n <= 0) {
> +			free(*to_free);
> +			*to_free = NULL;
> +			return NULL;
> +		}
> +		(*to_free)[n] = '\0';
> +
> +		if (memchr(*to_free, '\0', n))
> +			return *to_free;
> +
> +		/*
> +		 * Read the whole remaining strtab (or hit EOF) with no
> +		 * terminator: corrupt file, bail instead of re-reading.
> +		 */
> +		if (want >= remain || (u64)n >= remain)
> +			break;

[Severity: Medium]
Does this loop properly handle a short read (EOF) on a truncated ELF file?

If pread() hits EOF before filling the requested want bytes, n will be smaller
than want but greater than 0. The loop condition currently checks if
want >= remain or (u64)n >= remain, but it appears to miss an explicit
short-read/EOF check like (size_t)n < want.

Because of this, if remain is very large (based on an unvalidated sh_size
in the ELF header), the loop could continuously geometrically reallocate up
to remain bytes while reading the exact same short bytes from the constant
file_off. Could this cause memory exhaustion / an OOM allocation failure?

> +
> +		/* Avoid size_t overflow on absurdly large strtabs. */
> +		if (cap > SIZE_MAX / 2)
> +			break;
> +		cap *= 2;
> +	}
> +
> +	free(*to_free);
> +	*to_free = NULL;
> +	return NULL;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-perf-symbol-memory-send-v1-0-1d3360e21f07@uber.com?part=3