Re: [PATCH v6 01/59] perf inject: Fix itrace branch stack synthesis

sashiko-bot
2026-04-25T18:31:17Z

Sashiko AI review found 3 potential issue(s):
- [High] Missing initialization of `event_copy->header.misc` in `perf_event__repipe_sample()` causes corruption of output events.
- [High] Hardcoded `branch_sample_type=0` in trace decoders omits `hw_idx` from branch events, creating corrupted `perf.data` files.
- [High] `perf_event__repipe_sample` fails to strip `PERF_SAMPLE_AUX` when `--itrace=l` is used, leading to `-EFAULT` abortion or file corruption.
--

commit 9516abbe162f4ebd2ee0085c49fc396bb54c1328
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 f174bc69cec4..88c0ef4f5ff1 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
[ ... ]
> @@ -375,7 +375,59 @@ static int perf_event__repipe_sample(const struct perf_tool *tool,
>
> build_id__mark_dso_hit(tool, event, sample, evsel, machine);
>
> - if (inject->itrace_synth_opts.set && sample->aux_sample.size) {
> + if (inject->itrace_synth_opts.set &&
> + (inject->itrace_synth_opts.last_branch ||
> + inject->itrace_synth_opts.add_last_branch)) {
> + union perf_event *event_copy = (void *)inject->event_copy;
> + struct branch_stack dummy_bs = { .nr = 0 };
> + int err;
> + size_t sz;
> + u64 orig_type = evsel->core.attr.sample_type;
> + u64 orig_branch_type = evsel->core.attr.branch_sample_type;
> +
> + if (event_copy == NULL) {
> + inject->event_copy = malloc(PERF_SAMPLE_MAX_SIZE);
> + if (!inject->event_copy)
> + return -ENOMEM;
> +
> + event_copy = (void *)inject->event_copy;
> + }
> +
> + if (!sample->branch_stack)
> + sample->branch_stack = &dummy_bs;
> +
> + 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;
> + }

If `--itrace=l` is used (so `last_branch` is true but `add_last_branch` is
false), does `evsel->core.attr.sample_type` retain the `PERF_SAMPLE_AUX`
flag here?

If the flag remains set, wouldn't `perf_event__sample_event_size()` include
the massive `sample->aux_sample.size` in the new event size, potentially
exceeding `PERF_SAMPLE_MAX_SIZE` (64KB) and causing `perf inject` to abort
with `-EFAULT` below?

> +
> + sz = perf_event__sample_event_size(sample, evsel->core.attr.sample_type,
> + evsel->core.attr.read_format,
> + evsel->core.attr.branch_sample_type);
> +
> + if (sz > PERF_SAMPLE_MAX_SIZE) {
> + pr_err("Sample size %zu exceeds max size %d\n", sz, PERF_SAMPLE_MAX_SIZE);
> + return -EFAULT;
> + }
> +
> + event_copy->header.type = PERF_RECORD_SAMPLE;
> + event_copy->header.size = sz;

Does this code leave `event_copy->header.misc` uninitialized?

Since `event_copy` is allocated via `malloc()` without clearing the memory,
and `type` and `size` are set explicitly but `misc` is skipped, could this
write uninitialized heap memory into the `misc` field? This might corrupt
privilege level flags like `PERF_RECORD_MISC_USER` and break symbol resolution.

[ ... ]
> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
> index e5835042acdf..c4ed9f10e731 100644
> --- 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);
> }

Can hardcoding `0` for `branch_sample_type` cause the `hw_idx` field to be
omitted from the synthesized binary event payload?

If the trace decoder setup configures synthetic event attributes with
`PERF_SAMPLE_BRANCH_HW_INDEX`, tools parsing this injected `perf.data` file
will expect the 8-byte `hw_idx` field. Its absence could cause out-of-bounds
reads and corrupt parsing for subsequent sample fields.

[ ... ]
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 8a639d2e51a4..1ebc1a6a5e75 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1425,8 +1425,10 @@ static void cs_etm__update_last_branch_rb(struct cs_etm_queue *etmq,
> static int cs_etm__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.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);
> }

This code appears to have the same issue with hardcoding `branch_sample_type`
as `0`, potentially omitting `hw_idx`.

[ ... ]
> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> index fc9eec8b54b8..2dce6106c038 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -1731,8 +1731,12 @@ static void intel_pt_prep_b_sample(struct intel_pt *pt,
> static int intel_pt_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);
> }

Similarly here, could hardcoding `0` omit the `hw_idx` field and lead to
corrupted `perf.data` files?

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