Re: [PATCH v7 1/6] perf mem: Fix size tracking for mem_lvl's in perf_script__meminfo_scnprintf()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Unsafe string formatting and size tracking in `perf_script__meminfo_scnprintf` and its helper functions lead to severe integer underflows and stack buffer overflows.
--

commit c7ba4f7e986e052ba237f51f607c40edec278ce2
Author: Thomas Falcon <thomas.falcon@intel.com>

perf mem: Fix size tracking for mem_lvl's in perf_script__meminfo_scnprintf()

When printing memory info in perf script, the entire size of the
buffer is passed to perf_mem__lvl_scnprintf() instead of the remaining
size. Pass the remaining buffer size instead.

> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
> index 0b49fce251fcc..4e490f9cd3488 100644
> --- a/tools/perf/util/mem-events.c
> +++ b/tools/perf/util/mem-events.c
> @@ -610,7 +610,7 @@ int perf_script__meminfo_scnprintf(char *out, size_t sz, const struct mem_info *
>  	i += scnprintf(out, sz, "|OP ");
>  	i += perf_mem__op_scnprintf(out + i, sz - i, mem_info);
>  	i += scnprintf(out + i, sz - i, "|LVL ");
> -	i += perf_mem__lvl_scnprintf(out + i, sz, mem_info);
> +	i += perf_mem__lvl_scnprintf(out + i, sz - i, mem_info);

[Severity: Critical]
This isn't a bug introduced by this patch, but since the patch attempts to fix
size tracking, are there still missing bounds checks and integer underflows
here?

If i >= sz because previous helper functions return a length greater than the
remaining size, the calculation sz - i underflows to a massive unsigned value,
defeating the bounds tracking.

Furthermore, inside perf_mem__lvl_scnprintf() and similar helpers, sz is
subtracted and unchecked strcat calls are used, which can result in stack
buffer overflows:

tools/perf/util/mem-events.c:perf_mem__lvl_scnprintf() {
	...
	sz -= 1; /* -1 for null termination */
	out[0] = '\0';
	...
		if (data_src.mem_remote) {
			strcat(out, "Remote ");
			l += 7;
		}
	...
		if (printed++) {
			strcat(out, " or ");
			l += 4;
		}
		l += scnprintf(out + l, sz - l, mem_lvl[i]);
	...
}

If sz is 0, sz -= 1 underflows to SIZE_MAX, and a null byte is written out of
bounds.

The unchecked strcat writes past bounds, and the unconditional increment of l
causes the inner sz - l calculation to underflow, disabling bounds checks for
subsequent scnprintf calls.

Can a maliciously crafted perf.data file trigger this overflow on the stack
buffer in data_src__fprintf()?

>  	i += scnprintf(out + i, sz - i, "|SNP ");
>  	i += perf_mem__snp_scnprintf(out + i, sz - i, mem_info);
>  	i += scnprintf(out + i, sz - i, "|TLB ");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909160218.174928-1-thomas.falcon@intel.com?part=1