Re: [PATCH v3 1/2] perf event: Fix size of synthesized sample with branch stacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Heap buffer overflow and event truncation in `perf_event__convert_sample_callchain` due to incorrectly using unmasked `sample_type`.
- [High] Hardware tracer event corruption due to hardcoded `branch_sample_type=0` omitting the `hw_idx` field.
--

commit bc54c42a1fd16f8d3cc2f06f7ef9e4f81b9c3d09
Author: Ian Rogers <irogers@google.com>

perf event: Fix size of synthesized sample with branch stacks

This commit fixes the writing of the nr and hw_idx values during sample event
synthesis by passing the branch_sample_type into the sample size and
synthesis functions.

> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
[ ... ]
> @@ -463,8 +463,13 @@ static int perf_event__convert_sample_callchain(const struct perf_tool *tool,
>  	/* remove sample_type {STACK,REGS}_USER for synthesize */
>  	sample_type &= ~(PERF_SAMPLE_STACK_USER | PERF_SAMPLE_REGS_USER);
>  
> -	perf_event__synthesize_sample(event_copy, sample_type,
> -				      evsel->core.attr.read_format, sample);
> +	ret = perf_event__synthesize_sample(event_copy, evsel->core.attr.sample_type,
> +					    evsel->core.attr.read_format,
> +					    evsel->core.attr.branch_sample_type, sample);

Is it possible this causes a heap buffer overflow or event truncation?

By using evsel->core.attr.sample_type instead of the local masked sample_type
variable, the synthesizer is instructed to write the full user stack and
registers into event_copy. If the original stack is large, appending the newly
resolved callchain might overflow the allocation.

Additionally, since event_copy->header.size was manually adjusted to the
smaller size earlier in this function, could this cause the output event
stream to become truncated or corrupted?

I noticed this is explicitly fixed later in the patch series in commit
5881c330c2ca4300a88cc8c8e4d578c17a6ebef5 ("perf inject: Fix itrace branch
stack synthesis").

> +	if (ret) {
> +		pr_err("Failed to synthesize sample\n");
> +		return ret;
> +	}
>  	return perf_event__repipe_synth(tool, event_copy);
>  }
[ ... ]
> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
> --- a/tools/perf/util/arm-spe.c
> +++ b/tools/perf/util/arm-spe.c
[ ... ]
> @@ -484,8 +484,11 @@ static void arm_spe__prep_branch_stack(struct arm_spe_queue *speq)
>  
>  static int arm_spe__inject_event(union perf_event *event, struct perf_sample *sample, u64 type)
>  {
> -	event->header.size = perf_event__sample_event_size(sample, type, 0);
> -	return perf_event__synthesize_sample(event, type, 0, sample);
> +	event->header.type = PERF_RECORD_SAMPLE;
> +	event->header.size = perf_event__sample_event_size(sample, type, /*read_format=*/0,
> +							   /*branch_sample_type=*/0);
> +	return perf_event__synthesize_sample(event, type, /*read_format=*/0,
> +					     /*branch_sample_type=*/0, sample);
>  }

Could hardcoding branch_sample_type to 0 here cause hardware tracer event
corruption?

Hardware tracers like ARM SPE, Intel PT, and CS-ETM explicitly set
PERF_SAMPLE_BRANCH_HW_INDEX in their event attributes during setup. By
passing 0 to the synthesizer, it skips writing the 8-byte hw_idx field in
the payload.

When parsing the synthesized event later, wouldn't tools like
evsel__parse_sample() see the flag and incorrectly consume the first 8 bytes
of the branch stack entries as hw_idx, misaligning all subsequent fields?

I noticed this same hardcoded 0 is used in the cs-etm.c and intel-pt.c updates
in this commit, and that this is explicitly fixed for all hardware tracers
later in the patch series in commit 5881c330c2ca4300a88cc8c8e4d578c17a6ebef5
("perf inject: Fix itrace branch stack synthesis").

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