Re: [PATCH v5 2/2] perf inject: Fix itrace branch stack synthesis

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added event size validation in `perf_event__repipe_attr` is executed after a memory copy and is vulnerable to integer overflow on 32-bit architectures.
--

commit d1753b02b3fecbf4dd36f4b01ecfc58131e9bfb4
Author: Ian Rogers <irogers@google.com>
perf inject: Fix itrace branch stack synthesis

This commit fixes several issues when synthesizing branch stacks from AUX
data using perf inject --itrace=L. It corrects missing branch stack flags,
resolves memory leaks, and adds size validation to prevent buffer overflows
and parse failures.

> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 5f06ee1c14a08..862e178491053 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
[ ... ]
> @@ -231,7 +242,30 @@ static int perf_event__repipe_attr(const struct perf_tool *tool,
>  	if (!inject->output.is_pipe)
>  		return 0;
>  
> -	return perf_event__repipe_synth(tool, event);
> +	if (!inject->itrace_synth_opts.set)
> +		return perf_event__repipe_synth(tool, event);
> +
> +	memset(&attr, 0, sizeof(attr));
> +	memcpy(&attr, &event->attr.attr,
> +	       min_t(size_t, sizeof(attr), (size_t)event->attr.attr.size));

Could this result in an out-of-bounds read from the event buffer if a
malformed event provides a large attr.size but a very small header.size?

The size validation check happens just after this memcpy. Would it be safer
to move the event->header.size check before the memcpy?

> +
> +	if (event->header.size < sizeof(event->header) + event->attr.attr.size) {
> +		pr_err("Attribute event size %u is too small\n", event->header.size);
> +		return -EINVAL;
> +	}

On 32-bit architectures, can the addition of sizeof(event->header) and
event->attr.attr.size overflow if the provided attr.size is exceptionally
large (like 0xFFFFFFF8)?

If it wraps around to a small value, it seems it would bypass this check
entirely and cause an underflow in the n_ids calculation below, potentially
leading to out-of-bounds memory accesses in perf_event__synthesize_attr().

> +	n_ids = event->header.size - sizeof(event->header) - event->attr.attr.size;
> +	n_ids /= sizeof(u64);
> +	ids = perf_record_header_attr_id(event);

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