Re: [PATCH v7 2/5] 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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Stack buffer overflow in cgroup event synthesis due to unhandled alignment padding in `perf_event__synthesize_cgroup`.
--

--- Patch [2]: [PATCH v7 2/5] perf synthetic-events: Fix line synchronization, bounds, and truncation bugs in proc maps reader ---
commit 5a6494ecb67ef07c1f503dd9bb0692e1d9860b8d
Author: Ian Rogers <irogers@google.com>
Date:   Tue Jul 21 23:52:51 2026 +0000

    perf synthetic-events: Fix line synchronization, bounds, and truncation bugs in proc maps reader
    
    Fix critical logic and boundary bugs in read_proc_maps_line() and caller.
    Ensure any mid-line hex/dec/char parsing failure invokes io__drain_line()
    safely, using a do-while loop to read and discard remaining characters
    until a newline or EOF is reached.

> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 5307d707711d8..e9952c27b3bc0 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
[ ... ]
> @@ -579,7 +658,8 @@ static int perf_event__synthesize_cgroup(const struct perf_tool *tool,
>  
>  	event->cgroup.id = handle.cgroup_id;
>  	strncpy(event->cgroup.path, path + mount_len, path_len);
> -	memset(event->cgroup.path + path_len, 0, machine->id_hdr_size);
> +	memset((char *)event + offsetof(struct perf_record_cgroup, path) + path_len,
> +	       0, machine->id_hdr_size);

[Severity: High]
This is a pre-existing issue, but can the padding loop earlier in
perf_event__synthesize_cgroup() overflow the cgrp_root stack buffer?

The function perf_event__synthesize_cgroups() allocates a fixed-size stack
array:

    char cgrp_root[PATH_MAX];

This buffer is passed to perf_event__walk_cgroup_tree(), which recursively
appends directory names. The recursion bounds check allows the length of the
path to reach up to PATH_MAX - 2:

    if (strlen(path) + strlen(dent->d_name) + 1 >= PATH_MAX)
        continue;

When this near-maximum path is passed to perf_event__synthesize_cgroup(),
the function calculates path_len and attempts to align it to an 8-byte
boundary:

    while (path_len % sizeof(u64))
        path[mount_len + path_len++] = '\0';

Because mount_len + path_len can start around 4095, the loop will write null
bytes up to index 4101, which exceeds the PATH_MAX limit of cgrp_root. Does
this out-of-bounds write need to be bounded to prevent stack corruption?

>  
>  	if (perf_tool__process_synth_event(tool, event, machine, process) < 0) {
>  		pr_debug("process synth event failed\n");

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