Re: [PATCH v1 01/58] perf inject: Fix itrace branch stack synthesis

sashiko-bot
2026-04-20T00:49:38Z

Sashiko AI review found 4 potential issue(s):
- [Critical] Heap Buffer Overflow in `perf_event__repipe_sample` during branch stack synthesis.
- [High] Out-of-bounds Read and Data Corruption when synthesizing branch stacks.
- [High] File Format Corruption due to late `evsel` header modifications missing in repiped events.
- [High] Deterministic NULL pointer dereference in `intel_pt_do_synth_pebs_sample`.
--

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

perf inject: Fix itrace branch stack synthesis

When using "perf inject --itrace=L" to synthesize branch stacks from
AUX data, several issues caused failures.

> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index f174bc69cec45..9da334740017b 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -375,7 +375,52 @@ static int perf_event__repipe_sample(const struct perf_tool *tool,

[ ... ]

> + if (inject->itrace_synth_opts.add_last_branch) {
> + /* Temporarily add in type bits for synthesis. */
> + evsel->core.attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> + evsel->core.attr.branch_sample_type |= PERF_SAMPLE_BRANCH_HW_INDEX;
> + evsel->core.attr.sample_type &= ~PERF_SAMPLE_AUX;
> + }
> +
> + sz = perf_event__sample_event_size(sample, evsel->core.attr.sample_type,
> + evsel->core.attr.read_format);

Does this code overflow event_copy?

If a large event near PERF_SAMPLE_MAX_SIZE is processed and add_last_branch
is set, adding PERF_SAMPLE_BRANCH_STACK causes the recalculated sz to exceed
65536 bytes.

perf_event__synthesize_sample() would then write out of bounds into the heap.

[ ... ]

> @@ -2434,12 +2479,26 @@ static int __cmd_inject(struct perf_inject *inject)

[ ... ]

> if (inject->itrace_synth_opts.set) {
> + struct evsel *evsel;
> +
> perf_header__clear_feat(&session->header,
> HEADER_AUXTRACE);
> +
> + evlist__for_each_entry(session->evlist, evsel) {
> + evsel->core.attr.sample_type &= ~PERF_SAMPLE_AUX;
> + }
> +
> if (inject->itrace_synth_opts.last_branch ||
> - inject->itrace_synth_opts.add_last_branch)
> + inject->itrace_synth_opts.add_last_branch) {
> perf_header__set_feat(&session->header,
> HEADER_BRANCH_STACK);
> +
> + evlist__for_each_entry(session->evlist, evsel) {
> + evsel->core.attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> + evsel->core.attr.branch_sample_type |=
> + PERF_SAMPLE_BRANCH_HW_INDEX;
> + }
> + }
> }

Can moving this attribute modification to just before writing the file header
corrupt the output?

During perf_event__repipe_sample(), if itrace_synth_opts.last_branch is true
but add_last_branch is false, the event is synthesized and written using its
original sample_type. Additionally, this skips the fallback call to
perf_inject__cut_auxtrace_sample(), leaving PERF_SAMPLE_AUX data intact.

When the file header is subsequently written with the modified attributes
here, it advertises that all events contain branch stacks and lack AUX data,
causing a structural mismatch between the global header and the payload.

> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> index fc9eec8b54b82..b9fcf3b457b0d 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -2557,7 +2557,8 @@ static int intel_pt_do_synth_pebs_sample(struct intel_pt_queue *ptq, struct evse
> sample.transaction = txn;
> }
>
> - ret = intel_pt_deliver_synth_event(pt, event, &sample, sample_type);
> + ret = intel_pt_deliver_synth_event(pt, event, &sample,
> + sample_type | evsel->synth_sample_type);

Could this cause a NULL pointer dereference in perf_event__sample_event_size()
which is called during event delivery?

If PERF_SAMPLE_BRANCH_STACK is present in evsel->synth_sample_type but
missing from evsel->core.attr.sample_type, sample.branch_stack will be NULL.

However, because the bit is set in the type passed to
intel_pt_deliver_synth_event(), the branch_stack pointer is dereferenced
unconditionally.

> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 85bee747f4cd2..33b530b73796b 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -1719,9 +1719,9 @@ int perf_event__synthesize_sample(union perf_event *event, u64 type, u64 read_fo
>
> if (type & PERF_SAMPLE_BRANCH_STACK) {
> sz = sample->branch_stack->nr * sizeof(struct branch_entry);
> - /* nr, hw_idx */
> - sz += 2 * sizeof(u64);
> - memcpy(array, sample->branch_stack, sz);
> + *array++ = sample->branch_stack->nr;
> + *array++ = sample->branch_stack->hw_idx;
> + memcpy(array, sample->branch_stack->entries, sz);
> array = (void *)array + sz;
> }

Are these reads safe for events that lack PERF_SAMPLE_BRANCH_HW_INDEX?

When an event lacks the hardware index, sample->branch_stack points directly
to the raw payload where hw_idx is omitted, meaning the entries array starts
8 bytes earlier (at offset 8 instead of 16).

Reading sample->branch_stack->hw_idx here will erroneously access the first
8 bytes of the first entry. Furthermore, sample->branch_stack->entries
evaluates to a pointer 8 bytes past the actual start of the entries, causing
the memcpy to read out of bounds at the end of the buffer.

Should this use the perf_sample__branch_entries() macro and check
sample->no_hw_idx instead?

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