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

[sashiko-bot@kernel.org]

Sashiko AI review found 4 potential issue(s):
- [High] Uninitialized `header.misc` field in synthesized branch stack samples corrupts CPU mode metadata.
- [High] Pipeline stream corruption in pipe mode due to missing ATTR header updates.
- [High] Hardcoded `branch_sample_type=0` in auxtrace decoders omits `hw_idx` for PEBS samples, breaking alignment.
- [High] `last_branch` injection bypasses AUX data stripping, causing a mismatch between the payload and the file header.
--

commit f5581930b39cbe639866b1a06c082d518b3d6c69
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,
failures occurred due to incorrect sample_type and unhandled hardware indices.
This commit fixes sample_type flags during synthesis and explicitly passes
branch_sample_type to properly write the nr and hw_idx fields.

> 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;
> +		}

Since event_copy is allocated with malloc(), is it possible that
event_copy->header.misc is left uninitialized?

event_copy->header.type and event_copy->header.size are populated below, but
header.misc seems to be skipped. Could this result in garbage data being
passed for CPU mode and other flags, corrupting downstream symbol resolution?

> +
> +		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 perf inject is run with --itrace=L, last_branch is true but
add_last_branch is false, so PERF_SAMPLE_AUX is not stripped here.

Since __cmd_inject() unconditionally strips PERF_SAMPLE_AUX from the output
session header, could this lead to samples being synthesized and written with
their aux data intact, causing a mismatch between the declared header
attributes and the actual sample payload?

[ ... ]
> @@ -2434,12 +2488,25 @@ static int __cmd_inject(struct perf_inject *inject)
>  		 * synthesized hardware events, so clear the feature flag.
>  		 */
>  		if (inject->itrace_synth_opts.set) {
> +			struct evsel *evsel;
> +
>  			perf_header__clear_feat(&session->header,
>  						HEADER_AUXTRACE);
> -			if (inject->itrace_synth_opts.last_branch ||
> -			    inject->itrace_synth_opts.add_last_branch)
> +
> +			evlist__for_each_entry(session->evlist, evsel) {
> +				evsel->core.attr.sample_type &= ~PERF_SAMPLE_AUX;
> +			}
> +
> +			if (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;
> +				}
> +			}
>  		}

This block updates the attributes to reflect the new sample payload, but
it is inside an if (!inject->output.is_pipe && !inject->in_place_update)
check.

When pipe mode is used, attributes are emitted inline at the start of the
stream. If the payload is modified in perf_event__repipe_sample() without
the corresponding attribute updates in pipe mode, will downstream pipe
consumers receive initial attributes that do not match the modified payloads?

[ ... ]
> 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);
>  }

Passing 0 for branch_sample_type here hardcodes the synthesis to omit the
hw_idx field.

Since intel_pt_inject_event() is used for synthesizing pebs samples which
inherit the original evsel configuration, what happens if the original pebs
evsel had PERF_SAMPLE_BRANCH_HW_INDEX enabled? Will this cause the payload
to omit hw_idx while the attributes still expect it, misaligning all
subsequent payload reads?

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