Re: [PATCH v3 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 1 potential issue(s) to consider:
- [Medium] io__drain_line() fails to drain the line when a parsing error occurs, breaking map parsing synchronization and causing inefficient cascading failures.
--

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

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

This commit fixes logic and boundary bugs in read_proc_maps_line() and its
callers. It adds io__drain_line() to safely ignore already-consumed
newlines and preserve synchronization for subsequent map entries.

> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 5307d707711d8..059dd558a564c 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -291,6 +291,17 @@ static int perf_event__synthesize_fork(const struct perf_tool *tool,
>  	return 0;
>  }
>  
> +static void io__drain_line(struct io *io, int ch)
> +{
> +	if (ch == '\n')
> +		return;
> +	if (ch == -2 && io->data > io->buf && io->data[-1] == '\n')
> +		return;
> +
> +	while (ch >= 0 && ch != '\n')
> +		ch = io__get_char(io);

[Severity: Medium]
Will this loop correctly drain the line when a parsing error occurs?

When io__get_hex() or io__get_dec() encounters an invalid first character,
they consume it and return -2. read_proc_maps_line() then calls
io__drain_line(io, -2).

Because the loop condition strictly requires ch >= 0, it will skip entirely
when passed the -2 error code, returning without consuming any characters.

Instead of draining the rest of the line, the parser will re-evaluate every
remaining character as the potential start of a new map entry, leading to
parsing desynchronization and a tight loop of failures that could trigger
the timeout watchdog.

> +}
> +
>  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 +310,127 @@ 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) != '-')
> +	ch = io__get_hex(io, start);
> +	if (ch != '-') {
> +		if (!io->eof)
> +			io__drain_line(io, ch);
>  		return false;
> -	if (io__get_hex(io, end) != ' ')
> +	}

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