Re: [PATCH v3 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:

New issues:
- [Medium] The `perf_mem__show_region` flag is evaluated before feature events are processed in pipe mode, permanently hiding the memory region field for piped inputs.

Pre-existing issues:
- [High] Stack buffer overflow in memory event formatting due to missing bounds checks, unbounded `strcat`, and unsigned integer underflow.
--

--- Patch [5]: [PATCH v3 5/6] perf tools: Show memory region in perf-script subcommand ---
commit 453eaf4e1cef12c5307d93d17b31ce0722004b14
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

perf tools: Show memory region in perf-script subcommand

This commit adds support for showing the memory region in the perf-script
subcommand. It maps the mem_region field of the memory information data
source into a new Region field in the output.

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

> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -4468,6 +4468,9 @@ int cmd_script(int argc, const char **argv)
>  	if (show_full_info)
>  		script.tool.show_feat_hdr = SHOW_FEAT_HEADER_FULL_INFO;
>  
> +	perf_mem__set_show_region(perf_header__has_feat(&session->header,
> +							HEADER_MEMORY_RANGES));
> +

[Severity: Medium]
Will this correctly identify the memory region feature when reading from a
pipe?

In pipe mode, the session header features are not fully populated until
PERF_RECORD_HEADER_FEATURE events are processed dynamically during
perf_session__process_events(). Evaluating perf_header__has_feat() prematurely
here causes it to evaluate to false and permanently hides the memory region
field.

>  	if (symbol__init(env) < 0)
>  		goto out_delete;
>  
> 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
[ ... ]
> +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: High]
This is a pre-existing issue, but does this code safely handle the case where
sz is already 0?

If this function is called with sz = 0 due to earlier buffer exhaustion,
sz -= 1 will underflow to SIZE_MAX and blindly write a null byte out of bounds.

[ ... ]
> @@ -620,6 +693,10 @@ int perf_script__meminfo_scnprintf(char *out, size_t sz, const struct mem_info *
>  	i += perf_mem__lck_scnprintf(out + i, sz - i, mem_info);
>  	i += scnprintf(out + i, sz - i, "|BLK ");
>  	i += perf_mem__blk_scnprintf(out + i, sz - i, mem_info);
> +	if (perf_mem__show_region) {
> +		i += scnprintf(out + i, sz - i, "|Region ");
> +		i += perf_mem__region_scnprintf(out + i, sz - i, mem_info);
> +	}
>  
>  	return i;
>  }

[Severity: High]
This is a pre-existing issue, but can this sequence of formatting operations
cause a stack buffer overflow?

If a crafted perf.data file has an event with all memory levels, snoops,
and TLB bits set, the combined string might exceed the fixed 200-byte stack
arrays in data_src__fprintf() and set_sample_datasrc_in_dict().

Because i is accumulated without checking if i < sz, the expression sz - i
passed to subsequent functions can underflow to SIZE_MAX, disabling all
remaining bounds checks. 

Furthermore, pre-existing helper functions like perf_mem__lvl_scnprintf()
unconditionally use strcat() regardless of remaining buffer capacity:

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

Will this cause memory corruption if the buffer limit is exceeded?

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