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

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 eve=
nts 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 bo=
unds 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 <[email protected]>

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 =3D SHOW_FEAT_HEADER_FULL_INFO;
> =20
> +	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() prematur=
ely
here causes it to evaluate to false and permanently hides the memory region
field.

>  	if (symbol__init(env) < 0)
>  		goto out_delete;
> =20
> 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 =3D 0;
> +	u64 mem =3D PERF_MEM_REGION_NA;
> +
> +	sz -=3D 1; /* -1 for null termination */
> +	out[0] =3D '\0';

[Severity: High]
This is a pre-existing issue, but does this code safely handle the case whe=
re
sz is already 0?

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

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

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 +=3D 4;
		}
    ...
}

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

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803204540.1003=
[email protected]?part=3D5