Re: [PATCH 08/11] perf bpf: Use scnprintf() in snprintf_hex() and synthesize_bpf_prog_name()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Accumulation of `snprintf` return values combined with fixed increments causes buffer size integer underflows, leading to a stack buffer overflow in `perf_pmu__for_each_event`.
--

commit 9c31711b5b63e0f7c6e422153e389b761e114baf
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf bpf: Use scnprintf() in snprintf_hex() and synthesize_bpf_prog_name()

This patch fixes a potential buffer overflow by switching from snprintf() to
scnprintf() during formatted output accumulation. This prevents the remaining
buffer size from underflowing if the output is truncated.

> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index a27945c279efb..2c09842469f1f 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c

[ ... ]

> @@ -153,9 +153,10 @@ static int synthesize_bpf_prog_name(char *buf, int size,
>  			short_name = info->name;
>  	} else
>  		short_name = "F";
> -	if (short_name)
> -		name_len += snprintf(buf + name_len, size - name_len,
> -				     "_%s", short_name);
> +	if (short_name) {
> +		name_len += scnprintf(buf + name_len, size - name_len,
> +				      "_%s", short_name);
> +	}
>  	return name_len;
>  }

While we are fixing snprintf() accumulations here, there is a similar
pre-existing issue in tools/perf/util/pmu.c:

tools/perf/util/pmu.c:perf_pmu__for_each_event() {
...
		info.scale_unit = NULL;
		if (strlen(event->unit) || event->scale != 1.0) {
			info.scale_unit = buf + buf_used;
			buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
					"%G%s", event->scale, event->unit) + 1;
		}
		info.desc = event->desc;
		info.long_desc = event->long_desc;
		info.encoding_desc = buf + buf_used;
		buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
				"%.*s/%s/", (int)pmu_name_len, info.pmu_name, event->terms) + 1;
...
}

[Severity: High]
This is a pre-existing issue, but the same pattern is present in
perf_pmu__for_each_event().

Because snprintf() returns the length it would have written rather than the
amount actually written, and we unconditionally add 1 to it, buf_used can exceed
sizeof(buf).

When buf_used exceeds sizeof(buf), sizeof(buf) - buf_used will underflow to a
large positive size.

Could the subsequent snprintf() call use this massive size limit and write to
the out-of-bounds pointer buf + buf_used, overflowing buf[]?

Other similar bugs in hwmon_pmu.c and hist.c appear to be fixed in subsequent
patches of this series, but pmu.c seems to remain vulnerable at the end of the
series.

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