Re: [PATCH 01/11] perf tools: Fix get_max_num() size_t underflow on empty sysfs file

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Buffer underflow and out-of-bounds write when parsing empty PMU sysfs files
- [High] Missing null termination in `filename__read_int` and `filename__read_ull_base` leads to uninitialized stack memory read
--

commit 8d602fc039d9415d5215be33dc13b838d360d23b
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf tools: Fix get_max_num() size_t underflow on empty sysfs file

get_max_num() reads a sysfs file (cpu/possible, cpu/present, or
node/possible) and scans backward from the end to find the last
number.  If the file is empty, filename__read_str() returns num == 0.
The loop `while (--num)` decrements the size_t from 0 to SIZE_MAX,
reading backward across the heap until a comma or hyphen is found
or unmapped memory is hit.

Add an early return for empty files before the backward scan.

> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 21fa781b03cc7..1fab00ec4a59a 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -448,6 +448,12 @@ static int get_max_num(char *path, int *max)
>  
>  	buf[num] = '\0';
>  
> +	/* empty file — nothing to parse */
> +	if (num == 0) {
> +		err = -1;
> +		goto out;
> +	}
> +
>  	/* start on the right, to find highest node num */
>  	while (--num) {
>  		if ((buf[num] == ',') || (buf[num] == '-')) {

[Severity: High]
This isn't a bug introduced by this patch, but do similar underflows occur in
other files when parsing empty PMU sysfs files?

In tools/perf/util/pmu.c:pmu_id():
	str[len - 1] = 0; /* remove line feed */

If len is 0, this writes a null byte at str[-1].

Also in tools/perf/util/pmu.c:perf_pmu__parse_scale():
	if (scale[sret - 1] == '\n')
		scale[sret - 1] = '\0';
	else
		scale[sret] = '\0';

When sret is 0, this reads and potentially writes to scale[-1].

And in tools/perf/util/pmu.c:perf_pmu__parse_unit():
	if (alias->unit[sret - 1] == '\n')
		alias->unit[sret - 1] = '\0';
	else
		alias->unit[sret] = '\0';

When sret is 0, this accesses index -1 of the struct array field.

Could this corrupt heap metadata or memory when perf runs against empty sysfs
files?


[Severity: High]
This is a pre-existing issue, but I noticed another potential read issue.
Does filename__read_int() read uninitialized memory?

tools/lib/api/fs/fs.c:filename__read_int() {
	...
	if (read(fd, line, sizeof(line)) > 0) {
		*value = atoi(line);
	...
}

Since read() doesn't append a null terminator, if the file doesn't end with
a non-digit character or if the contents reach 64 bytes, could atoi() read
past the valid data into uninitialized stack memory?

Similarly, in tools/lib/api/fs/fs.c:filename__read_ull_base():
	if (read(fd, line, sizeof(line)) > 0) {
		*value = strtoull(line, NULL, base);

Could strtoull() also read out of bounds here for the same reason?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608201753.1979464-1-acme@kernel.org?part=1