Re: [PATCH v2 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:
- [High] Severe pre-existing stack buffer overflows and integer underflows in `perf_mem__*_scnprintf` functions due to unsafe `strcat` usage and missing bounds checks.
--

--- Patch [1]: [PATCH v2 1/6] perf mem: Fix size tracking for mem_lvl's in perf_script__meminfo_scnprintf() ---
commit cf8f02c307060f141740bb3567a69f77761e0f5f
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: High]
This is a pre-existing issue, but does passing sz - i here risk an integer
underflow and stack buffer overflow in the callee if the buffer is already
full?

If previous fields exhaust the buffer, sz - i can be 0 (or wrap around if
i > sz). If 0 is passed to perf_mem__lvl_scnprintf(), the size decrement
at the start of the function will underflow:

tools/perf/util/mem-events.c:perf_mem__lvl_scnprintf() {
        sz -= 1; /* -1 for null termination */
        out[0] = '\0';
        ...
        strcat(out, "Remote ");
}

When sz underflows to SIZE_MAX, out[0] = '\0' writes out of bounds, and
the unbounded strcat operations can corrupt the stack if a malformed or
untrusted perf.data file sets multiple bits in data_src.

Does this same unsafe pattern also exist in perf_mem__snp_scnprintf,
perf_mem__tlb_scnprintf, and perf_mem__blk_scnprintf, which all start
with the same sz -= 1 decrement?

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