Re: [PATCH v7 5/6] perf tools: Show memory region in perf-script subcommand

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Integer underflow leading to stack buffer overflow if the accumulated string length reaches the buffer size in `perf_mem__region_scnprintf`.
- [Low] Unnecessary full header inclusion of "session.h" in mem-events.h instead of using a forward declaration.
--

commit f7491b8d4beca72ae600f38adac9d0669b182411
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

perf tools: Show memory region in perf-script subcommand

Show the memory region in perf-script subcommand. Memory region is found
in the mem_region field of the memory information data source.

Link: https://lore.kernel.org/all/20260114011750.350569-1-dapeng1.mi@linux.intel.com/

> diff --git a/tools/perf/util/mem-events.h b/tools/perf/util/mem-events.h
> index daa22748f9fea..4ebb8109fc3c4 100644
> --- a/tools/perf/util/mem-events.h
> +++ b/tools/perf/util/mem-events.h
> @@ -4,6 +4,7 @@
>  
>  #include <stdbool.h>
>  #include <linux/types.h>
> +#include "session.h"

[Severity: Low]
Does this code introduce unnecessary namespace pollution and risk musl 
compilation failures? The subsystem guidelines recommend using a forward 
declaration for struct perf_session instead of a full header inclusion 
when only structure pointer handles are referenced.

[ ... ]

> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
> index 4fd48fd200552..8ce4996cad8d3 100644
> --- a/tools/perf/util/mem-events.c
> +++ b/tools/perf/util/mem-events.c
[ ... ]
> +static int perf_mem__region_scnprintf(char *out, size_t sz, const struct mem_info *mem_info)
>  {
> +	size_t l = 0;
> +	u64 mem = PERF_MEM_REGION_NA;
> +
> +	sz -= 1; /* -1 for null termination */
> +	out[0] = '\0';

[Severity: Medium]
Can this lead to an integer underflow and a stack buffer overflow?

In perf_script__meminfo_scnprintf, the characters written are accumulated 
in i. If i grows larger than or equal to the initial buffer size (e.g. 200) 
due to previous print operations, sz - i passed to 
perf_mem__region_scnprintf evaluates to 0. 

When sz is 0 in perf_mem__region_scnprintf, sz -= 1 underflows to the 
maximum unsigned size. The subsequent out[0] assignment writes 
out-of-bounds, and scnprintf receives the maximum size as its limit, which 
can cause a massive buffer overflow when parsing a complex perf.data file.

[ ... ]

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