Re: [PATCH v4 2/4] perf synthetic-events: Fix line synchronization, bounds, and truncation bugs in proc maps reader

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] Bypassing _FORTIFY_SOURCE array bounds checks introduces an exploitable stack buffer overflow when zeroing event trailers.
- [High] Removal of the stale build ID clearing leaks state and corrupts subsequent kernel module events.

Pre-existing issues:
- [High] Incomplete fix: _FORTIFY_SOURCE array bounds aborts still exist in multiple other synthesis functions.
--

--- Patch [2]: [PATCH v4 2/4] perf synthetic-events: Fix line synchronization, bounds, and truncation bugs in proc maps reader ---
commit ba0b89af1798a9c26d45cb174748a43558ce4d9d
Author: Ian Rogers <irogers@google.com>

perf synthetic-events: Fix line synchronization, bounds, and truncation bugs in proc maps reader

This commit fixes bugs in read_proc_maps_line() and its callers by ensuring
proper line draining on parsing failures. It also adds standard fallbacks
for over-length paths, safely emits timeout flags, and introduces pointer
casts to prevent _FORTIFY_SOURCE array bounds aborts when zeroing trailers.

> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 5307d707711d8..e0680d770462c 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
[ ... ]
> @@ -463,45 +533,53 @@ int perf_event__synthesize_mmap_events(const struct perf_tool *tool,
[ ... ]
> -		memset(event->mmap2.filename + size, 0, machine->id_hdr_size +
> -			(aligned_size - size));
> -		event->mmap2.header.size += machine->id_hdr_size;
> +		/* Zero the padding and ID header trailer safely! */
> +		memset((char *)event + offsetof(struct perf_record_mmap2, filename) + size, 0,
> +		       (aligned_size - size) + machine->id_hdr_size);
> +
> +		event->mmap2.header.size += machine->id_hdr_size;

[Severity: Critical]
Can bypassing the _FORTIFY_SOURCE array bounds checks introduce a stack
buffer overflow regression here?

When this function is called via machine__init_live(), the event union is
allocated on the stack without the extra padding for id_hdr_size:

tools/perf/util/machine.c:machine__init_live() {
    union perf_event event;
    memset(&event, 0, sizeof(event));
    ...
    perf_event__synthesize_mmap_events(tool, &event, ...);
}

If an attacker provides a pathname approaching PATH_MAX in /proc/pid/maps,
size pushes the memset destination to the end of the union perf_event
structure. By bypassing the fortify check, this memset blindly writes
machine->id_hdr_size bytes of zeroes completely outside the bounds of the
stack variable, silently corrupting the stack frame instead of safely
aborting.

[Severity: High]
This is a pre-existing issue, but does the same _FORTIFY_SOURCE array
bounds abort regression still exist in other synthesis functions?

While this patch applies a cast bypass here in
perf_event__synthesize_mmap_events(), similar bounds-violating memset calls
remain in other paths. 

For example, in perf_event__prepare_comm():

tools/perf/util/synthetic-events.c:perf_event__prepare_comm() {
    ...
    memset(event->comm.comm + size, 0, machine->id_hdr_size);
    ...
}

Since event->comm.comm is a static 16-byte array, if a process name is 15
characters long, the memset begins exactly at the end of the array and
writes id_hdr_size bytes. Under modern compiler flags (_FORTIFY_SOURCE=2
or 3), this reliably triggers an abort because the compiler detects
the memset writing past the bounds of the array. The same pattern is
also present in perf_event__synthesize_cgroups().

[ ... ]
> @@ -703,11 +781,6 @@ static int perf_event__synthesize_modules_maps_cb(struct map *map, void *data)
>  
>  		memcpy(event->mmap2.filename, dso__long_name(dso), dso__long_name_len(dso) + 1);
>  
> -		/* Clear stale build ID from previous module iteration */
> -		event->mmap2.header.misc &= ~PERF_RECORD_MISC_MMAP_BUILD_ID;
> -		memset(event->mmap2.build_id, 0, sizeof(event->mmap2.build_id));
> -		event->mmap2.build_id_size = 0;
> -
>  		perf_record_mmap2__read_build_id(&event->mmap2, args->machine, false);
>  	} else {
>  		size = PERF_ALIGN(dso__long_name_len(dso) + 1, sizeof(u64));

[Severity: High]
Could removing this explicit clearing of the shared union's build ID leak
state between loop iterations?

The perf_event__synthesize_modules() function allocates a single
union perf_event structure outside the loop and reuses it for every kernel
module via maps__for_each_map().

By removing the explicit clearing of PERF_RECORD_MISC_MMAP_BUILD_ID, if one
module successfully reads a build ID, the flag remains set for all
subsequent modules.

When processing subsequent modules, perf_record_mmap2__read_build_id() sees
the flag is already set, assumes the event already contains a valid build
ID, and reads the stale build ID from the previous iteration. This could
attribute samples to incorrect binaries.

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