Re: [PATCH 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] Cascading integer underflows and stack buffer overflows in `perf_script__meminfo_scnprintf` and its helper functions due to unbounded `strcat` and unchecked size arithmetic.
--

--- Patch [1]: [PATCH 1/6] perf mem: Fix size tracking for mem_lvl's in perf_script__meminfo_scnprintf() ---
commit e57d4daab871fa57aef1ac0569d81f8d93753398
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
> --- 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);
>  	i += scnprintf(out + i, sz - i, "|SNP ");

[Severity: Critical]
This is a pre-existing issue, but does sz - i risk underflowing here if i
exceeds sz?

Looking at the helper functions called here, they use strcat() without bounds
checking and advance the length manually:

tools/perf/util/mem-events.c:perf_mem__lvl_scnprintf() {
    ...
    if (printed++) {
        strcat(out, " or ");
        l += 4;
    }
    l += scnprintf(out + l, sz - l, mem_lvl[i]);
    ...
}

If multiple bits are set and l exceeds sz due to the manual l += 4 increments,
could sz - l underflow to SIZE_MAX? This would cause subsequent scnprintf()
calls to write past the buffer boundaries. The same pattern appears to exist
in perf_mem__snp_scnprintf() and perf_mem__tlb_scnprintf().

Since these helpers can return a length greater than sz, the accumulated size i
in perf_script__meminfo_scnprintf() could also grow larger than sz. This would
cause cascading underflows when subtracting i from sz in subsequent calls.
Should we ensure the accumulated length is checked against sz before
subtraction, and consider replacing strcat() with a bounded string operation
in the helper functions?

>  	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/20260709221736.33446-1-thomas.falcon@intel.com?part=1