Re: [PATCH v3 3/4] perf synthetic-events: Fix bounds, stale state, and misc flags in kernel module synthesis

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The memset used to clear the filename padding and ID header trailer will trigger a _FORTIFY_SOURCE runtime abort when a DSO name length approaches PATH_MAX.
--

--- Patch [3]: [PATCH v3 3/4] perf synthetic-events: Fix bounds, stale state, and misc flags in kernel module synthesis ---
commit e862d233381463bc8c1e84b7d335152fe0a7c118
Author: Ian Rogers <irogers@google.com>

perf synthetic-events: Fix bounds, stale state, and misc flags in kernel module synthesis

This commit clamps long DSO names to mmap/mmap2 filename boundaries to prevent
buffer overruns in perf_event__synthesize_modules_maps_cb(). It also explicitly
clears misc flags and union padding to prevent stale Build-ID state from leaking
between module synthesis events.

> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c

[ ... ]

> @@ -761,49 +762,78 @@ static int perf_event__synthesize_modules_maps_cb(struct map *map, void *data)
>  	if (!symbol_conf.no_buildid_mmap2) {
>  		const char *long_name = dso__long_name(dso);
>  
> -		size = PERF_ALIGN(dso__long_name_len(dso) + 1, sizeof(u64));
> +		size = strlen(long_name);
> +		if (size >= sizeof(event->mmap2.filename))
> +			size = sizeof(event->mmap2.filename) - 1;
> +
> +		strlcpy(event->mmap2.filename, long_name,
> +			sizeof(event->mmap2.filename));
> +
> +		aligned_size = PERF_ALIGN(size + 1, sizeof(u64));
>  		event->mmap2.header.type = PERF_RECORD_MMAP2;
> -		event->mmap2.header.size = (sizeof(event->mmap2) -
> -					(sizeof(event->mmap2.filename) - size));
> -		memset(event->mmap2.filename + size, 0, args->machine->id_hdr_size);
> +		event->mmap2.header.misc = args->misc;
> +		event->mmap2.header.size =
> +			offsetof(struct perf_record_mmap2, filename) +
> +			aligned_size;
> +
> +		/* Zero the padding and ID header trailer safely! */
> +		memset(event->mmap2.filename + size, 0,
> +		       (aligned_size - size) + args->machine->id_hdr_size);

[Severity: High]
This is a pre-existing issue, but can this memset trigger a _FORTIFY_SOURCE
runtime abort when a DSO name length approaches PATH_MAX?

The event->mmap2.filename field is declared as a fixed-size array of
PATH_MAX (4096 bytes). If a kernel module path is very long, size is clamped
to 4095 and aligned_size evaluates to 4096. The memset then attempts to write
(1 + args->machine->id_hdr_size) bytes starting at event->mmap2.filename + 4095.

Because the memset is anchored to the filename member array, _FORTIFY_SOURCE
dynamically tracks that there is only 1 byte remaining in that specific member.
Since 1 + id_hdr_size > 1, this exceeds the compiler-enforced array bound,
triggering __chk_fail() and terminating perf, even though the underlying
allocation is large enough.

> +
>  		event->mmap2.header.size += args->machine->id_hdr_size;
>  		event->mmap2.start = map__start(map);

[ ... ]

>  		perf_record_mmap2__read_build_id(&event->mmap2, args->machine, false);
>  	} else {
> -		size = PERF_ALIGN(dso__long_name_len(dso) + 1, sizeof(u64));
> +		const char *long_name = dso__long_name(dso);
> +
> +		size = strlen(long_name);
> +		if (size >= sizeof(event->mmap.filename))
> +			size = sizeof(event->mmap.filename) - 1;
> +
> +		strlcpy(event->mmap.filename, long_name,
> +			sizeof(event->mmap.filename));
> +
> +		aligned_size = PERF_ALIGN(size + 1, sizeof(u64));
>  		event->mmap.header.type = PERF_RECORD_MMAP;
> -		event->mmap.header.size = (sizeof(event->mmap) -
> -					(sizeof(event->mmap.filename) - size));
> -		memset(event->mmap.filename + size, 0, args->machine->id_hdr_size);
> +		event->mmap.header.misc = args->misc;
> +		event->mmap.header.size =
> +			offsetof(struct perf_record_mmap, filename) +
> +			aligned_size;
> +
> +		/* Zero the padding and ID header trailer safely! */
> +		memset(event->mmap.filename + size, 0,
> +		       (aligned_size - size) + args->machine->id_hdr_size);

[Severity: High]
This is a pre-existing issue, but this memset is subject to the same
_FORTIFY_SOURCE abort as the mmap2 case above when the DSO name approaches
PATH_MAX and sample IDs are enabled.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721173347.9163-1-irogers@google.com?part=3