Re: [PATCH v1 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 2 potential issue(s) to consider:
- [High] Error recovery via `io__drain_line()` incorrectly consumes and discards the next valid map entry if a parsing failure occurs due to a premature newline character.
- [Medium] The PERF_RECORD_MISC_PROC_MAP_PARSE_TIMEOUT flag is silently dropped if the truncation timeout triggers on a map entry that fails the PROT_EXEC/mmap_data filters.
--

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

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

This commit addresses logic and boundary bugs in read_proc_maps_line() and its
caller. It ensures that mid-line parsing failures invoke io__drain_line() to
preserve synchronization and replaces truncation bugs with a standard fallback
literal.

> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 5307d707711d8..723b7323c06d9 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -291,6 +291,15 @@ static int perf_event__synthesize_fork(const struct perf_tool *tool,
>  	return 0;
>  }
>  
> +static void io__drain_line(struct io *io)
> +{
> +	int ch;
> +
> +	do {
> +		ch = io__get_char(io);
> +	} while (ch >= 0 && ch != '\n');
> +}
> +
>  static bool read_proc_maps_line(struct io *io, __u64 *start, __u64 *end,
>  				u32 *prot, u32 *flags, __u64 *offset,
>  				u32 *maj, u32 *min,
> @@ -299,69 +308,121 @@ static bool read_proc_maps_line(struct io *io, __u64 *start, __u64 *end,
>  {
>  	__u64 temp;
>  	int ch;
> -	char *start_pathname = pathname;
> +	size_t written = 0;
> +	bool overflowed = false;
>  
> -	if (io__get_hex(io, start) != '-')
> +	if (io__get_hex(io, start) != '-') {
> +		if (!io->eof)
> +			io__drain_line(io);

[Severity: High]
If a parser like io__get_hex() or io__get_char() fails because it encounters a
premature newline character, does calling io__drain_line() here unintentionally
consume and discard the next valid map entry?

Since the newline character for the malformed line would have already been
consumed by the parser, it appears io__drain_line() might read until the end
of the subsequent line, causing it to be skipped.

> +		return false;
> +	}

[ ... ]

> @@ -487,50 +544,64 @@ int perf_event__synthesize_mmap_events(const struct perf_tool *tool,

[ ... ]

> -		if ((event->mmap2.prot & PROT_EXEC) == 0) {
> -			if (!mmap_data || (event->mmap2.prot & PROT_READ) == 0)
> +		if ((prot & PROT_EXEC) == 0) {
> +			if (!mmap_data || (prot & PROT_READ) == 0) {
> +				if (truncation)
> +					break;

[Severity: Medium]
When the truncation timeout triggers on a map entry that fails the PROT_EXEC
or mmap_data filters, does this break prevent the timeout flag
(PERF_RECORD_MISC_PROC_MAP_PARSE_TIMEOUT) from being set and emitted?

By breaking out of the loop early here, it seems the code bypasses both the
PERF_RECORD_MISC_PROC_MAP_PARSE_TIMEOUT assignment and the call to
perf_tool__process_synth_event(), which might leave analysis tools unaware
that the data stream was truncated.

>  				continue;
> +			}
>  
>  			event->header.misc |= PERF_RECORD_MISC_MMAP_DATA;
>  		}
>  
> -out:
>  		if (truncation)
>  			event->header.misc |= PERF_RECORD_MISC_PROC_MAP_PARSE_TIMEOUT;

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